Re: [Qemu-ppc] MTTCG for ppc64 (removed from ppc-for-2.10)

[Nikunj A Dadhania]

Richard Henderson <address@hidden> writes:

> On 04/21/2017 02:16 AM, Nikunj A Dadhania wrote:
>> Nikunj A Dadhania <address@hidden> writes:
>>
>>> Richard Henderson <address@hidden> writes:
>>>
>>>> On 04/21/2017 12:08 AM, Nikunj A Dadhania wrote:
>>>>> David Gibson <address@hidden> writes:
>>>>>
>>>>>> [ Unknown signature status ]
>>>>>> Hi all,
>>>>>>
>>>>>> I'm afraid I've pulled the MTTCG enablement patches for target/ppc64
>>>>>> out of my ppc-for-2.10 tree, since I discovered it was causing a
>>>>>> repeatable failure on one of the Travis builds.
>>>>>>
>>>>>> Specifically, with both the LL/SC in terms of cmpxchg and
>>>>>> enable-mttcg-by-default patches applied, and qemu build with clang,
>>>>>> then make check will fail with a tcg_abort() in the powernv
>>>>>> boot-serial test.
>>>>>
>>>>> I see that in temp_load(), ts->val_type is ending up being
>>>>> TEMP_VAL_DEAD, and the tcg_abort() is because of that.
>>>>>
>>>>> Following patch gets past this error...
>>>>>
>>>>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>>>>> index 4a1f24a..9a4ea5f 100644
>>>>> --- a/target/ppc/translate.c
>>>>> +++ b/target/ppc/translate.c
>>>>> @@ -3169,6 +3169,7 @@ static void gen_conditional_store(DisasContext 
>>>>> *ctx, TCGv EA,
>>>>>      tcg_gen_brcond_tl(TCG_COND_NE, EA, cpu_reserve, l1);
>>>>>
>>>>>      t0 = tcg_temp_new();
>>>>> +    tcg_gen_movi_tl(t0, 0);
>>>>>      tcg_gen_atomic_cmpxchg_tl(t0, cpu_reserve, cpu_reserve_val,
>>>>>                                cpu_gpr[reg], ctx->mem_idx,
>>>>>                                DEF_MEMOP(memop) | MO_ALIGN);
>>>>
>>>> This *shouldn't* change anything.  The arugument that you're initializing 
>>>> is
>>>> pure output and is equivalent to
>>>>
>>>>    t0 = 0;
>>>>    t0 = foo();
>>>>
>>>> What's the test case here?
>>>
>>> With David's new branch that has PPC64 MTTCG - 
>>> https://github.com/dgibson/qemu/tree/mttcg
>>>
>>
>> BTW, only happens with clang:
>>
>>
>> ./configure --cc=clang --host-cc=clang  --target-list=ppc64-softmmu
>
> Ah!  This is good information to have.  Let's see if one of the sanitizers or 
> valgrind can spot the error or miscompilation...

Some interesting code generated for stdcx.:

    0x0000000030012624:  stdcx.  r0,0,r3

With code compiled using GCC
     ---- 0000000030012624
     [...]
     call raise_exception_err,$0x20,$0,env,tmp0,tmp1
     set_label $L1
     brcond_i64 loc4,reserve_addr,ne,$L2
     movi_i32 tmp0,$0x7b7
     call atomic_cmpxchgq_be,$0x20,$1,tmp2,env,reserve_addr,reserve_val,r0,tmp0
     setcond_i64 tmp2,tmp2,reserve_val,eq
     [...]
     
With code compiled using clang
     ---- 0000000030012624
     [...]
     call raise_exception_err,$0x20,$0,env,tmp0,tmp1
     set_label $L1
     brcond_i64 loc4,reserve_addr,ne,$L2
     call exit_atomic,$0x20,$0,env
     setcond_i64 tmp2,tmp2,reserve_val,eq
     [...]

In case of clang, movi_i32 and atomic_cmpxchgq_be is missing, and I see
a call to exit_atomic.

Regards
Nikunj

Detailed log:
With code compiled using GCC

    IN: 
    0x000000003001261c:  ori     r0,r0,1
    0x0000000030012620:  rldimi  r0,r9,32,0
    0x0000000030012624:  stdcx.  r0,0,r3
    0x0000000030012628:  bne-    0x30013620
    
    OP:
     ld_i32 tmp0,env,$0xffffffffffffffdc
     movi_i32 tmp1,$0x0
     brcond_i32 tmp0,tmp1,lt,$L0
    
     ---- 000000003001261c
     movi_i64 tmp2,$0x1
     or_i64 r0,r0,tmp2
    
     ---- 0000000030012620
     movi_i64 tmp3,$0x20
     shl_i64 tmp2,r9,tmp3
     ext32u_i64 r0,r0
     or_i64 r0,r0,tmp2
    
     ---- 0000000030012624
     mov_i64 loc4,r3
     movi_i64 tmp3,$0x7
     and_i64 tmp2,loc4,tmp3
     movi_i64 tmp3,$0x0
     brcond_i64 tmp2,tmp3,eq,$L1
     movi_i32 tmp0,$0x5
     movi_i32 tmp1,$0x0
     movi_i64 nip,$0x30012624
     call raise_exception_err,$0x20,$0,env,tmp0,tmp1
     set_label $L1
     brcond_i64 loc4,reserve_addr,ne,$L2
     movi_i32 tmp0,$0x7b7
     call atomic_cmpxchgq_be,$0x20,$1,tmp2,env,reserve_addr,reserve_val,r0,tmp0
     setcond_i64 tmp2,tmp2,reserve_val,eq
     movi_i64 tmp3,$0x1
     shl_i64 tmp2,tmp2,tmp3
     or_i64 tmp2,tmp2,SO
     mov_i32 crf0,tmp2
     br $L3
     set_label $L2
     mb $0x2f
     mov_i32 crf0,SO
     set_label $L3
     movi_i64 reserve_addr,$0xffffffffffffffff

With code compiled using clang:

    IN: 
    0x000000003001261c:  ori     r0,r0,1
    0x0000000030012620:  rldimi  r0,r9,32,0
    0x0000000030012624:  stdcx.  r0,0,r3
    0x0000000030012628:  bne-    0x30013620
    
    OP:
     ld_i32 tmp0,env,$0xffffffffffffffdc
     movi_i32 tmp1,$0x0
     brcond_i32 tmp0,tmp1,lt,$L0
    
     ---- 000000003001261c
     movi_i64 tmp2,$0x1
     or_i64 r0,r0,tmp2
    
     ---- 0000000030012620
     movi_i64 tmp3,$0x20
     shl_i64 tmp2,r9,tmp3
     ext32u_i64 r0,r0
     or_i64 r0,r0,tmp2
    
     ---- 0000000030012624
     mov_i64 loc4,r3
     movi_i64 tmp3,$0x7
     and_i64 tmp2,loc4,tmp3
     movi_i64 tmp3,$0x0
     brcond_i64 tmp2,tmp3,eq,$L1
     movi_i32 tmp0,$0x5
     movi_i32 tmp1,$0x0
     movi_i64 nip,$0x30012624
     call raise_exception_err,$0x20,$0,env,tmp0,tmp1
     set_label $L1
     brcond_i64 loc4,reserve_addr,ne,$L2
     call exit_atomic,$0x20,$0,env
     setcond_i64 tmp2,tmp2,reserve_val,eq
     movi_i64 tmp3,$0x1
     shl_i64 tmp2,tmp2,tmp3
     or_i64 tmp2,tmp2,SO
     mov_i32 crf0,tmp2
     br $L3
     set_label $L2
     mb $0x2f
     mov_i32 crf0,SO
     set_label $L3
     movi_i64 reserve_addr,$0xffffffffffffffff