[Qemu-devel] [patch] Fix extlh instruction on Alpha

[Qemu-devel] [patch] Fix extlh instruction on Alpha

[Vince Weaver]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1781 bytes --]

Hello

The extlh instruction on Alpha currently doesn't work properly.
It's a combination of a cut/paste bug (16 where it should be 32) as well 
as a "shift by 64" bug.

Below is a patch that fixes the problem, and attached is a test case that 
exhibits the bug.  The program should print a 4-char wide sliding window 
across the test string; without the patch this fails.

Vince

Signed-off-by: Vince Weaver <vince@csl.cornell.edu>

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 1fc5119..2a681b0 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -526,14 +526,24 @@ static always_inline void gen_ext_h(void (*tcg_gen_ext_i64)(TCGv t0, TCGv t1),
             else
                 tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]);
         } else {
+            int l1;
             TCGv tmp1, tmp2;
             tmp1 = tcg_temp_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);
             tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1);
+
+            gen_set_label(l1);
+
             tcg_temp_free(tmp1);
         }
         if (tcg_gen_ext_i64)
@@ -1320,7 +1330,7 @@ static always_inline int translate_one (DisasContext *ctx, uint32_t insn)
             break;
         case 0x6A:
             /* EXTLH */
-            gen_ext_h(&tcg_gen_ext16u_i64, ra, rb, rc, islit, lit);
+            gen_ext_h(&tcg_gen_ext32u_i64, ra, rb, rc, islit, lit);
             break;
         case 0x72:
             /* MSKQH */

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2444 bytes --]

# uldl.s by Vince Weaver
# This shows a bug with Qemu in handling of the extlh instruction
#  assemble with     "as -o uldl.o uldl.s"
#  link with         "ld -o uldl uldl.o"



# syscall numbers

.equ SYSCALL_EXIT,1	
.equ SYSCALL_WRITE,4
		
.equ STDIN,0
.equ STDOUT,1
.equ STDERR,2	
	
	
	.globl _start
_start:
	
	br      $27,0           # fake branch, to grab the location
	                        # of our entry point
	ldgp    $gp,0($27)      # load the GP proper for our entry point
				# this does automagic stuff...
				# gp is used for 64-bit jumps and constants
				# so if you use "la" and the like it will
				# load from gp for you. 
	

	lda     $17,title	  # load title
	br	$26,write_stdout  # print it
	
	lda	$17,test_string	  # load test string
	br	$26,write_stdout  # print it

	lda	$13,four_bytes	  # point $13 to our 32-bit wide
				  # test location
				  
	lda	$11,test_string	  # point $11 to beginning of test string
	addq	$11,20,$14	  # repeat 20 times
	
load_loop:	
	# uldl	$12,0($11)	  # load 32-bits from it
	# This expands to the following

.set noat
	lda     $28,0($11)
	ldq_u   $23,0($28)
	ldq_u   $24,3($28)
	extll   $23,$28,$23	
	extlh   $24,$28,$24	
	or      $23,$24,$12
	sextl   $12,$12
.set at

	stl	$12,0($13)	  # store to 4-byte location
	lda	$17,four_bytes	  # point to it
	br	$26,write_stdout  # print 4 chars

	addq	$11,1,$11
	
	cmpeq	$11,$14,$1
	beq	$1,load_loop

	#================================
	# Exit
	#================================
exit:		
        clr	$16			# 0 exit value
        mov	SYSCALL_EXIT,$0		# put the exit syscall number in v0
        callsys				# and exit
	
	#================================
	# WRITE_STDOUT
	#================================
	# $17 has string
	# $1 is trashed
	
write_stdout:	
	ldil	$0,SYSCALL_WRITE	# Write syscall in $0
	ldil	$16,STDOUT		# 1 in $16 (stdout)
	clr	$18			# 0 (count) in $18
	
str_loop1:
	addq	$17,$18,$1		# offset in $1
	ldbu    $1,0($1)		# load byte
	addq	$18,1,$18		# increment pointer
	bne	$1,str_loop1		# if not nul, repeat
	
	subq	$18,1,$18		# correct count
	callsys				# Make syscall
	
	ret	$26			# return
			
.data

.align 3

four_bytes: .ascii "RPLC\n\0"
.align 3
eight_bytes:	.ascii "REPLACE!\n\0"

title:	.ascii "ULDL Test\n\0"
linefeed: .ascii "\n\0"
.align 3
test_string:  .ascii "The quick brown fox jumped over the lazy dog\n\0"

[-- Attachment #3: Type: APPLICATION/octet-stream, Size: 1623 bytes --]

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

[Filip Navara]

On Wed, Aug 5, 2009 at 5:26 AM, Vince Weaver<vince@csl.cornell.edu> wrote:
> Hello
>
> The extlh instruction on Alpha currently doesn't work properly.
> It's a combination of a cut/paste bug (16 where it should be 32) as well
> as a "shift by 64" bug.
>
> Below is a patch that fixes the problem, and attached is a test case that
> exhibits the bug.  The program should print a 4-char wide sliding window
> across the test string; without the patch this fails.
>
> Vince
>
> Signed-off-by: Vince Weaver <vince@csl.cornell.edu>
>
> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index 1fc5119..2a681b0 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -526,14 +526,24 @@ static always_inline void gen_ext_h(void (*tcg_gen_ext_i64)(TCGv t0, TCGv t1),
>             else
>                 tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]);
>         } else {
> +            int l1;
>             TCGv tmp1, tmp2;
>             tmp1 = tcg_temp_new();

This should be tcg_temp_local_new since you added a branch instruction
and normal temporary variables are live in a basic block (eg. not
across a branch).

> +            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);
>             tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1);
> +
> +            gen_set_label(l1);
> +
>             tcg_temp_free(tmp1);
>         }
>         if (tcg_gen_ext_i64)
> @@ -1320,7 +1330,7 @@ static always_inline int translate_one (DisasContext *ctx, uint32_t insn)
>             break;
>         case 0x6A:
>             /* EXTLH */
> -            gen_ext_h(&tcg_gen_ext16u_i64, ra, rb, rc, islit, lit);
> +            gen_ext_h(&tcg_gen_ext32u_i64, ra, rb, rc, islit, lit);
>             break;
>         case 0x72:
>             /* MSKQH */

Best regards,
Filip Navara

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

[Vince Weaver]

(re-sending)

The extlh instruction on Alpha currently doesn't work properly.
It's a combination of a cut/paste bug (16 where it should be 32) as well 
as a "shift by 64" bug.

Below is a patch that fixes the problem.  The previous e-mails on this 
problem have test cases that exhibit the bug.

This patch uses tcg_temp_local_new() at the suggestion of Filip Navara.

Vince

Signed-off-by: Vince Weaver <vince@csl.cornell.edu>

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 1fc5119..4219916 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -526,14 +526,24 @@ static always_inline void gen_ext_h(void (*tcg_gen_ext_i64)(TCGv t0, TCGv t1),
             else
                 tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]);
         } 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);
             tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1);
+
+            gen_set_label(l1);
+
             tcg_temp_free(tmp1);
         }
         if (tcg_gen_ext_i64)
@@ -1320,7 +1330,7 @@ static always_inline int translate_one (DisasContext *ctx, uint32_t insn)
             break;
         case 0x6A:
             /* EXTLH */
-            gen_ext_h(&tcg_gen_ext16u_i64, ra, rb, rc, islit, lit);
+            gen_ext_h(&tcg_gen_ext32u_i64, ra, rb, rc, islit, lit);
             break;
         case 0x72:
             /* MSKQH */

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

[Aurelien Jarno]

On Wed, Sep 09, 2009 at 12:08:25PM -0400, Vince Weaver wrote:
> 
> (re-sending)
> 
> The extlh instruction on Alpha currently doesn't work properly.
> It's a combination of a cut/paste bug (16 where it should be 32) as well 
> as a "shift by 64" bug.
> 
> Below is a patch that fixes the problem.  The previous e-mails on this 
> problem have test cases that exhibit the bug.
> 
> This patch uses tcg_temp_local_new() at the suggestion of Filip Navara.
> 
> Vince
> 
> Signed-off-by: Vince Weaver <vince@csl.cornell.edu>
> 
> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index 1fc5119..4219916 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -526,14 +526,24 @@ static always_inline void gen_ext_h(void (*tcg_gen_ext_i64)(TCGv t0, TCGv t1),
>              else
>                  tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]);
>          } 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);

>              tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1);
> +
> +            gen_set_label(l1);
> +
>              tcg_temp_free(tmp1);
>          }
>          if (tcg_gen_ext_i64)
> @@ -1320,7 +1330,7 @@ static always_inline int translate_one (DisasContext *ctx, uint32_t insn)
>              break;
>          case 0x6A:
>              /* EXTLH */
> -            gen_ext_h(&tcg_gen_ext16u_i64, ra, rb, rc, islit, lit);
> +            gen_ext_h(&tcg_gen_ext32u_i64, ra, rb, rc, islit, lit);
>              break;
>          case 0x72:
>              /* MSKQH */
> 
> 
> 

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

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

[Vince Weaver]

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

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

[Aurelien Jarno]

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

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

[Vince Weaver]

On Wed, 16 Sep 2009, Aurelien Jarno wrote:

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

Ah, I see.  I completely missed that optimization.

How does this updated patch look?  I removed one of the TCGv variables 
too.  Does that help performance?  What would be nice is a tcg 
subtract-from instruction, which I know some architectures have.  Maybe 
tcg does have it and I should look harder.



diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 9d2bc45..af2a43c 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -524,14 +524,16 @@ static inline void gen_ext_h(void(*tcg_gen_ext_i64)(TCGv t0, TCGv t1),
             else
                 tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]);
         } else {
-            TCGv tmp1, tmp2;
+            TCGv tmp1;
             tmp1 = tcg_temp_new();
+
             tcg_gen_andi_i64(tmp1, cpu_ir[rb], 7);
             tcg_gen_shli_i64(tmp1, tmp1, 3);
-            tmp2 = tcg_const_i64(64);
-            tcg_gen_sub_i64(tmp1, tmp2, tmp1);
-            tcg_temp_free(tmp2);
+            tcg_gen_andi_i64(tmp1, tmp1, 0x3f);
+            tcg_gen_neg_i64(tmp1, tmp1);
+            tcg_gen_addi_i64(tmp1, tmp1, 64);
             tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1);
+
             tcg_temp_free(tmp1);
         }
         if (tcg_gen_ext_i64)
@@ -1316,7 +1318,7 @@ static inline int translate_one(DisasContext *ctx, uint32_t insn)
             break;
         case 0x6A:
             /* EXTLH */
-            gen_ext_h(&tcg_gen_ext16u_i64, ra, rb, rc, islit, lit);
+            gen_ext_h(&tcg_gen_ext32u_i64, ra, rb, rc, islit, lit);
             break;
         case 0x72:
             /* MSKQH */

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

[Laurent Desnogues]

On Thu, Sep 17, 2009 at 6:07 PM, Vince Weaver <vince@csl.cornell.edu> wrote:
>
> What would be nice is a tcg
> subtract-from instruction, which I know some architectures have.  Maybe
> tcg does have it and I should look harder.

There is tcg_gen_subfi.


Laurent

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

[Aurelien Jarno]

On Thu, Sep 17, 2009 at 12:07:23PM -0400, Vince Weaver wrote:
> On Wed, 16 Sep 2009, Aurelien Jarno wrote:
> 
> > In case tmp1 = 0, it becomes 64, and then 0 again after the and, so
> > rc=ra<<0.
> 
> Ah, I see.  I completely missed that optimization.
> 
> How does this updated patch look?  I removed one of the TCGv variables 
> too.  Does that help performance?  What would be nice is a tcg 

Yes it looks ok. Removing one normal TCGv variable doesn't really help.
What helps is removing a tcg temp_local variable or a branch.

> subtract-from instruction, which I know some architectures have.  Maybe 
> tcg does have it and I should look harder.

You can use tcg_gen_subfi_i64 (tcg result, immediate, tcg arg).
 
> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index 9d2bc45..af2a43c 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -524,14 +524,16 @@ static inline void gen_ext_h(void(*tcg_gen_ext_i64)(TCGv t0, TCGv t1),
>              else
>                  tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]);
>          } else {
> -            TCGv tmp1, tmp2;
ap> +            TCGv tmp1;
>              tmp1 = tcg_temp_new();
> +
>              tcg_gen_andi_i64(tmp1, cpu_ir[rb], 7);
>              tcg_gen_shli_i64(tmp1, tmp1, 3);
> -            tmp2 = tcg_const_i64(64);
> -            tcg_gen_sub_i64(tmp1, tmp2, tmp1);
> -            tcg_temp_free(tmp2);
> +            tcg_gen_andi_i64(tmp1, tmp1, 0x3f);
> +            tcg_gen_neg_i64(tmp1, tmp1);
> +            tcg_gen_addi_i64(tmp1, tmp1, 64);
>              tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1);
> +
>              tcg_temp_free(tmp1);
>          }
>          if (tcg_gen_ext_i64)
> @@ -1316,7 +1318,7 @@ static inline int translate_one(DisasContext *ctx, uint32_t insn)
>              break;
>          case 0x6A:
>              /* EXTLH */
> -            gen_ext_h(&tcg_gen_ext16u_i64, ra, rb, rc, islit, lit);
> +            gen_ext_h(&tcg_gen_ext32u_i64, ra, rb, rc, islit, lit);
>              break;
>          case 0x72:
>              /* MSKQH */
> 

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

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

[Vince Weaver]

The extlh instruction on Alpha currently doesn't work properly.
It's a combination of a cut/paste bug (16 where it should be 32) as well 
as a "shift by 64" bug.

This improves on an earlier patch that used labels, conditional jumps, 
and local variables.  Thanks go especially to Aurelien Jarno and Andreas 
Schwab who have a much better eye for bit-wise TCG optimization than I do.

Vince

Signed-off-by: Vince Weaver <vince@csl.cornell.edu>

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 9d2bc45..9e7e9b2 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -524,14 +524,15 @@ static inline void gen_ext_h(void(*tcg_gen_ext_i64)(TCGv t0, TCGv t1),
             else
                 tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]);
         } else {
-            TCGv tmp1, tmp2;
+            TCGv tmp1;
             tmp1 = tcg_temp_new();
+
             tcg_gen_andi_i64(tmp1, cpu_ir[rb], 7);
             tcg_gen_shli_i64(tmp1, tmp1, 3);
-            tmp2 = tcg_const_i64(64);
-            tcg_gen_sub_i64(tmp1, tmp2, tmp1);
-            tcg_temp_free(tmp2);
+            tcg_gen_neg_i64(tmp1, tmp1);
+            tcg_gen_andi_i64(tmp1, tmp1, 0x3f);
             tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1);
+
             tcg_temp_free(tmp1);
         }
         if (tcg_gen_ext_i64)
@@ -1316,7 +1317,7 @@ static inline int translate_one(DisasContext *ctx, uint32_t insn)
             break;
         case 0x6A:
             /* EXTLH */
-            gen_ext_h(&tcg_gen_ext16u_i64, ra, rb, rc, islit, lit);
+            gen_ext_h(&tcg_gen_ext32u_i64, ra, rb, rc, islit, lit);
             break;
         case 0x72:
             /* MSKQH */

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

[Aurelien Jarno]

On Thu, Sep 17, 2009 at 03:28:52PM -0400, Vince Weaver wrote:
> 
> The extlh instruction on Alpha currently doesn't work properly.
> It's a combination of a cut/paste bug (16 where it should be 32) as well 
> as a "shift by 64" bug.
> 
> This improves on an earlier patch that used labels, conditional jumps, 
> and local variables.  Thanks go especially to Aurelien Jarno and Andreas 
> Schwab who have a much better eye for bit-wise TCG optimization than I do.
> 
> Vince
> 
> Signed-off-by: Vince Weaver <vince@csl.cornell.edu>

Thanks, applied to both master and stable-0.11.

> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index 9d2bc45..9e7e9b2 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -524,14 +524,15 @@ static inline void gen_ext_h(void(*tcg_gen_ext_i64)(TCGv t0, TCGv t1),
>              else
>                  tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]);
>          } else {
> -            TCGv tmp1, tmp2;
> +            TCGv tmp1;
>              tmp1 = tcg_temp_new();
> +
>              tcg_gen_andi_i64(tmp1, cpu_ir[rb], 7);
>              tcg_gen_shli_i64(tmp1, tmp1, 3);
> -            tmp2 = tcg_const_i64(64);
> -            tcg_gen_sub_i64(tmp1, tmp2, tmp1);
> -            tcg_temp_free(tmp2);
> +            tcg_gen_neg_i64(tmp1, tmp1);
> +            tcg_gen_andi_i64(tmp1, tmp1, 0x3f);
>              tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1);
> +
>              tcg_temp_free(tmp1);
>          }
>          if (tcg_gen_ext_i64)
> @@ -1316,7 +1317,7 @@ static inline int translate_one(DisasContext *ctx, uint32_t insn)
>              break;
>          case 0x6A:
>              /* EXTLH */
> -            gen_ext_h(&tcg_gen_ext16u_i64, ra, rb, rc, islit, lit);
> +            gen_ext_h(&tcg_gen_ext32u_i64, ra, rb, rc, islit, lit);
>              break;
>          case 0x72:
>              /* MSKQH */
> 
> 
> 

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

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

[Rob Landley]

On Thursday 17 September 2009 14:28:52 Vince Weaver wrote:
> The extlh instruction on Alpha currently doesn't work properly.
> It's a combination of a cut/paste bug (16 where it should be 32) as well
> as a "shift by 64" bug.
>
> This improves on an earlier patch that used labels, conditional jumps,
> and local variables.  Thanks go especially to Aurelien Jarno and Andreas
> Schwab who have a much better eye for bit-wise TCG optimization than I do.
>
> Vince

Any idea how hard it would be to whip up a qemu-system-alpha emulation?  I 
note that several real-world alpha boards were essentially just a PC with a 
different processor.  (In fact the original Athlon used the Alpha EV6 bus and 
was pin-compatible, so you could drop-in replace the processor with an Alpha 
if you could somehow reflash the bios with alpha instructions instead of x86.)

I'd like to boot Alpha Linux on qemu, and it doesn't seem like there's _that_ 
much more to do.  But last I asked (a couple years ago) I was told the Alpha 
protected mode stuff wasn't implemented yet...

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds

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

[Laurent Desnogues]

On Mon, Sep 21, 2009 at 4:20 AM, Rob Landley <rob@landley.net> wrote:
>
> Any idea how hard it would be to whip up a qemu-system-alpha emulation?  I
> note that several real-world alpha boards were essentially just a PC with a
> different processor.  (In fact the original Athlon used the Alpha EV6 bus and
> was pin-compatible, so you could drop-in replace the processor with an Alpha
> if you could somehow reflash the bios with alpha instructions instead of x86.)
>
> I'd like to boot Alpha Linux on qemu, and it doesn't seem like there's _that_
> much more to do.  But last I asked (a couple years ago) I was told the Alpha
> protected mode stuff wasn't implemented yet...

There's been a series of patch to add ES40 proposed by Tristan Gingold
last March.  Look here:

http://www.archivum.info/qemu-devel@nongnu.org/2009-03/00723/%5BQemu-devel%5D_%5BPATCH_0_24%5D:_add_alpha_es40_system_emulation


Laurent

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

[Tristan Gingold]

On Sep 21, 2009, at 8:23 AM, Laurent Desnogues wrote:

> On Mon, Sep 21, 2009 at 4:20 AM, Rob Landley <rob@landley.net> wrote:
>> I'd like to boot Alpha Linux on qemu, and it doesn't seem like  
>> there's _that_
>> much more to do.  But last I asked (a couple years ago) I was told  
>> the Alpha
>> protected mode stuff wasn't implemented yet...
>
> There's been a series of patch to add ES40 proposed by Tristan Gingold
> last March.  Look here:
>
> http://www.archivum.info/qemu-devel@nongnu.org/2009-03/00723/%5BQemu-devel%5D_%5BPATCH_0_24%5D:_add_alpha_es40_system_emulation

I haven't had time to continue to work on it.  It was able to boot and  
run linux and partially boot
tru64.  Both AlphaBios and SRM were ok.

The main issue was speed and IO-TLB (necessary for Tru64 and VMS).

Tristan.

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

[Rob Landley]

On Monday 21 September 2009 01:23:50 Laurent Desnogues wrote:
> On Mon, Sep 21, 2009 at 4:20 AM, Rob Landley <rob@landley.net> wrote:
> > Any idea how hard it would be to whip up a qemu-system-alpha emulation?
> >  I note that several real-world alpha boards were essentially just a PC
> > with a different processor.  (In fact the original Athlon used the Alpha
> > EV6 bus and was pin-compatible, so you could drop-in replace the
> > processor with an Alpha if you could somehow reflash the bios with alpha
> > instructions instead of x86.)
> >
> > I'd like to boot Alpha Linux on qemu, and it doesn't seem like there's
> > _that_ much more to do.  But last I asked (a couple years ago) I was told
> > the Alpha protected mode stuff wasn't implemented yet...
>
> There's been a series of patch to add ES40 proposed by Tristan Gingold
> last March.

Ooh, thanks.

> Look here:
>
> http://www.archivum.info/qemu-devel@nongnu.org/2009-03/00723/%5BQemu-devel%
>5D_%5BPATCH_0_24%5D:_add_alpha_es40_system_emulation

That's the March 19th posting, he actually posted a newer series on March 30.  
Still just boots the Bios, not Linux, but it seems a good thing to poke at

Looks promising.  Has there been any progress since then?  (Did any of the 
series get merged?  Is it worth trying to apply this series to current -git, 
feeding it a linux -kernel, and seeing what happens?)

> Laurent

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds

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

[Rob Landley]

On Monday 21 September 2009 06:37:24 Tristan Gingold wrote:
> On Sep 21, 2009, at 8:23 AM, Laurent Desnogues wrote:
> > On Mon, Sep 21, 2009 at 4:20 AM, Rob Landley <rob@landley.net> wrote:
> >> I'd like to boot Alpha Linux on qemu, and it doesn't seem like
> >> there's _that_
> >> much more to do.  But last I asked (a couple years ago) I was told
> >> the Alpha
> >> protected mode stuff wasn't implemented yet...
> >
> > There's been a series of patch to add ES40 proposed by Tristan Gingold
> > last March.  Look here:
> >
> > http://www.archivum.info/qemu-devel@nongnu.org/2009-03/00723/%5BQemu-deve
> >l%5D_%5BPATCH_0_24%5D:_add_alpha_es40_system_emulation
>
> I haven't had time to continue to work on it.  It was able to boot and
> run linux and partially boot
> tru64.  Both AlphaBios and SRM were ok.
>
> The main issue was speed and IO-TLB (necessary for Tru64 and VMS).

I'm only worrying about Linux at the moment.  If the patch series boots Linux, 
I'm happy to test it.

Did you boot Linux with -kernel?  Do you have the Linux .config you used?  And 
what gcc/binutils tuple were you using?  (Just straight alpha-unknown-linux?)

Thanks,

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds

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

[Tristan Gingold]

On Sep 21, 2009, at 8:48 PM, Rob Landley wrote:

> On Monday 21 September 2009 06:37:24 Tristan Gingold wrote:
>> On Sep 21, 2009, at 8:23 AM, Laurent Desnogues wrote:
>>> On Mon, Sep 21, 2009 at 4:20 AM, Rob Landley <rob@landley.net>  
>>> wrote:
>>>> I'd like to boot Alpha Linux on qemu, and it doesn't seem like
>>>> there's _that_
>>>> much more to do.  But last I asked (a couple years ago) I was told
>>>> the Alpha
>>>> protected mode stuff wasn't implemented yet...
>>>
>>> There's been a series of patch to add ES40 proposed by Tristan  
>>> Gingold
>>> last March.  Look here:
>>>
>>> http://www.archivum.info/qemu-devel@nongnu.org/2009-03/00723/%5BQemu-deve
>>> l%5D_%5BPATCH_0_24%5D:_add_alpha_es40_system_emulation
>>
>> I haven't had time to continue to work on it.  It was able to boot  
>> and
>> run linux and partially boot
>> tru64.  Both AlphaBios and SRM were ok.
>>
>> The main issue was speed and IO-TLB (necessary for Tru64 and VMS).
>
> I'm only worrying about Linux at the moment.  If the patch series  
> boots Linux,
> I'm happy to test it.

Yes, I got a linux shell.

> Did you boot Linux with -kernel?

No.

>  Do you have the Linux .config you used?  And
> what gcc/binutils tuple were you using?  (Just straight alpha- 
> unknown-linux?)

Yes.

Tristan.