Re: [Qemu-devel] [PATCH] Fix extlh instruction on Alpha

[Aurelien Jarno <aurelien@aurel32.net>]

On Wed, Sep 16, 2009 at 04:45:20PM -0400, Vince Weaver wrote:
> On Wed, 16 Sep 2009, Aurelien Jarno wrote:
> 
> > On Wed, Sep 09, 2009 at 12:08:25PM -0400, Vince Weaver wrote:
> 
> > >          } else {
> > > +            int l1;
> > >              TCGv tmp1, tmp2;
> > > -            tmp1 = tcg_temp_new();
> > > +            tmp1 = tcg_temp_local_new();
> > > +            l1 = gen_new_label();
> > > +
> > >              tcg_gen_andi_i64(tmp1, cpu_ir[rb], 7);
> > >              tcg_gen_shli_i64(tmp1, tmp1, 3);
> > > +
> > > +            tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]);
> > > +            tcg_gen_brcondi_i64(TCG_COND_EQ, tmp1, 0, l1);
> > > +
> > >              tmp2 = tcg_const_i64(64);
> > >              tcg_gen_sub_i64(tmp1, tmp2, tmp1);
> > >              tcg_temp_free(tmp2);
> > 
> > Given that a test costs a lot (partly due to the fact temp local
> > variable must be used), I do wonder if doing a AND here wouldn't
> > be better:
> > 		
> >                tcg_gen_andi_i64(tmp1, tmp1, 0x3f);
> 
> I'm not sure I follow.
> 
> The code is attempting the following:
> 
>   tmp1=rb&0x7;
>   tmp1=temp1<<3;
> 
>   if (tmp1!=0) {
>      tmp1=64-tmp1;
>      rc=ra<<tmp1;
>   }
>   else {
>      rc=ra;
>   }
> 
> The problem with the original code is that in the case of tmp1 being 0,
> the shift left by 64 would result in 0, instead of the identity.
> 
> I tried to avoid the jump but couldn't.  Am I missing something?
> 

I mean the following code:

    tmp1=rb&0x7;
    tmp1=temp1<<3;
    tmp1=64-tmp1;
    tmp1=tmp1 & 0x3f;
    rc=ra<<tmp1;

In case tmp1 = 0, it becomes 64, and then 0 again after the and, so
rc=ra<<0.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net