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

[Richard Henderson]

On 04/24/2017 11:16 AM, Nikunj A Dadhania wrote:
> 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.

Ok, I see.  And the tcg error is because we're generating incorrect opcodes 
along this path.  I should be able to fix it quickly now.


r~