[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.

qemu-system-alpha (was: Re: [Qemu-devel] [PATCH] Fix extlh instruction on Alpha)

[Brian Wheeler]

On Sun, 2009-09-20 at 21:20 -0500, Rob Landley wrote:
> 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.  

Yep.  The ES40 used the ALI1543 chipset and was pretty pc-ish is most
respects.  There were a couple of DEC specific chips on board (such as
an IOMMU).  

But the hardware isn't the real problem, I don't think.  I think the
real problems (for QEMU, at least) are the PAL and SRM.

The PAL adds several os-specific pseudo-instructions to the instruction
set.  MILO has a linux palcode, but no other open source ones are
available that I know of.  Without VMS or Tru64 Palcode you're limited
to linux.  I think the BSDs use the Tru64 Palcode.  I suppose its
possible to have the palcode routines hard-coded into qemu, though you'd
need several sets of them (one for each supported OS type)

There doesn't seem to be an open source SRM which can be used to boot
the machine and provide the initial user interface.  MILO also performs
this function, but its hard to build and pretty old.  

I believe that charon axp solves this problem by having their own
mini-implementation of the srm (and presumably the PAL) which only
supports setting/viewing the environment and booting.  The real SRM has
lots of features that aren't really needed on emulated hardware.

> 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 were patches last year for an es40, but they never made it to the
main tree...and weren't quite finished.

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-system-alpha (was: Re: [Qemu-devel] [PATCH] Fix extlh instruction on Alpha)

[Rob Landley]

On Monday 21 September 2009 08:31:49 Brian Wheeler wrote:
> On Sun, 2009-09-20 at 21:20 -0500, Rob Landley wrote:
> > 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.
>
> Yep.  The ES40 used the ALI1543 chipset and was pretty pc-ish is most
> respects.  There were a couple of DEC specific chips on board (such as
> an IOMMU).
>
> But the hardware isn't the real problem, I don't think.  I think the
> real problems (for QEMU, at least) are the PAL and SRM.
>
> The PAL adds several os-specific pseudo-instructions to the instruction
> set.  MILO has a linux palcode, but no other open source ones are
> available that I know of.  Without VMS or Tru64 Palcode you're limited
> to linux.  I think the BSDs use the Tru64 Palcode.  I suppose its
> possible to have the palcode routines hard-coded into qemu, though you'd
> need several sets of them (one for each supported OS type)

What I'd really like to use is the -kernel option to boot Linux.

I have a project (http://impactlinux.com/fwl) that's trying to build a simple 
uclibc/busybox Linux system image for each qemu-system target, plus a native 
compiler (gcc,binutils,make,bash), and then using distcc to call out to the 
host's cross compiler through the virtual network to speed up native builds 
without all the other complexity of cross compiling (lying to ./configure in 
three dozen different ways while keeping two different toolchains with their own 
sets of headers and libraries from leaking into each other).

It's also a fun way to do nightly regression tests of busybox, uClibc, and the 
linux kernel on various different targets, so things like 2.6.30's mips bug or 
the current sh4 weirdness in 2.6.31 get caught early enough that the 
developers remember what changed.

It boots a kernel via -kernel and feeds in a root filesystem as /dev/hda or 
/dev/sda (or initramfs in a pinch, but that seldom gives you enough space 11 
megabytes of compressed toolchain before the memory mappings go "boing" during 
initramfs extract).  The way I'm booting uses a serial console, emulated hard 
drive (ide or scsi), emulated network card, 256 megs of ram, and optionally a 
battery-backed clock (I can fake it with rdate if the network card works, make 
gets really unhappy otherwise), the processor emulation in question, and a 
kernel configured to boot on that particular qemu setup.

So far I've got ppc, arm, mips, sh4, x86, x86-64, and a very buggy sparc 
booting to a shell prompt, and all of 'em except sparc can build and run 
"hello world" natively.  (uClibc's sparc support is not happy, hopefully 
current -git sucks less.)

I whipped up an Alpha config back when I started the project, but there's never 
been a way to test it, so it got lost.  Similar with m68k and cris.  (Last I 
checked the emulated cris boards were toys with a fixed small amount of memory, 
no hard drive, no way to add a network card, and I don't remember if they even 
emulated a battery-backed clock.)

> There doesn't seem to be an open source SRM which can be used to boot
> the machine and provide the initial user interface.  MILO also performs
> this function, but its hard to build and pretty old.
>
> I believe that charon axp solves this problem by having their own
> mini-implementation of the srm (and presumably the PAL) which only
> supports setting/viewing the environment and booting.  The real SRM has
> lots of features that aren't really needed on emulated hardware.

Reminds me of the nightmare getting openbios/openhackware on ppc to feed in a 
coherent device tree.  I just want enough setup to make -kernel work, but 
that's often nontrivial. :(

> > 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 were patches last year for an es40, but they never made it to the
> main tree...and weren't quite finished.

I'm happy to test (and document) something that almost works.  But if it 
doesn't almost work, I'm unlikely to be much help. :(

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.