From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1Mo1Nv-0005cJ-PF
	for qemu-devel@nongnu.org; Wed, 16 Sep 2009 16:45:51 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1Mo1Nr-0005aU-J4
	for qemu-devel@nongnu.org; Wed, 16 Sep 2009 16:45:51 -0400
Received: from [199.232.76.173] (port=47241 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Mo1Nr-0005aK-DS
	for qemu-devel@nongnu.org; Wed, 16 Sep 2009 16:45:47 -0400
Received: from csl.cornell.edu ([128.84.224.10]:3099 helo=vlsi.csl.cornell.edu)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <vince@stanley.csl.cornell.edu>) id 1Mo1Nq-0004DL-RZ
	for qemu-devel@nongnu.org; Wed, 16 Sep 2009 16:45:47 -0400
Date: Wed, 16 Sep 2009 16:45:20 -0400 (EDT)
From: Vince Weaver <vince@csl.cornell.edu>
Subject: Re: [Qemu-devel] [PATCH] Fix extlh instruction on Alpha
In-Reply-To: <20090916195242.GE770@volta.aurel32.net>
Message-ID: <20090916163826.M50839@stanley.csl.cornell.edu>
References: <20090909120628.J4195@stanley.csl.cornell.edu>
	<20090916195242.GE770@volta.aurel32.net>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Aurelien Jarno <aurelien@aurel32.net>
Cc: qemu-devel@nongnu.org

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?


> >              tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1);
> > +
> > +            gen_set_label(l1);
> > +

Vince