[Qemu-devel] [PATCH] ARM NEON shift emulation fix

[Qemu-devel] [PATCH] ARM NEON shift emulation fix

[Daniel Gutson]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

Hi,
   the attached patch fixes a bug that caused some NEON shift operations 
to shift a wrong amount of bytes.

The problem was that a variable holding an immediate value representing 
the amount of bits to shift was later overwritten with another value 
(used for something different) within a loop.

Please commit this for me if approved, since I don't have write access.

Thanks!
	Daniel.

---
2009-08-21  Daniel Gutson  <dgutson@codesourcery.com>

	* target-arm/translate.c (disas_neon_data_insn): Fixed
	shift operand.

-- 
Daniel Gutson
CodeSourcery
www.codesourcery.com

[-- Attachment #2: neon_fix.patch --]
[-- Type: text/x-diff, Size: 2399 bytes --]

diff -Naurp qemu-0.10.5-orig/target-arm/translate.c qemu-0.10.5-dfg/target-arm/translate.c
--- qemu-0.10.5-orig/target-arm/translate.c	2009-05-20 13:47:00.000000000 -0700
+++ qemu-0.10.5-dfg/target-arm/translate.c	2009-08-21 13:26:08.000000000 -0700
@@ -4440,6 +4440,7 @@ static int disas_neon_data_insn(CPUState
     } else if (insn & (1 << 4)) {
         if ((insn & 0x00380080) != 0) {
             /* Two registers and shift.  */
+            uint32_t shift_imm;
             op = (insn >> 8) & 0xf;
             if (insn & (1 << 7)) {
                 /* 64-bit shift.   */
@@ -4466,17 +4467,17 @@ static int disas_neon_data_insn(CPUState
                 }
                 switch (size) {
                 case 0:
-                    imm = (uint8_t) shift;
-                    imm |= imm << 8;
-                    imm |= imm << 16;
+                    shift_imm = (uint8_t) shift;
+                    shift_imm |= shift_imm << 8;
+                    shift_imm |= shift_imm << 16;
                     break;
                 case 1:
-                    imm = (uint16_t) shift;
-                    imm |= imm << 16;
+                    shift_imm = (uint16_t) shift;
+                    shift_imm |= shift_imm << 16;
                     break;
                 case 2:
                 case 3:
-                    imm = shift;
+                    shift_imm = shift;
                     break;
                 default:
                     abort();
@@ -4485,7 +4486,7 @@ static int disas_neon_data_insn(CPUState
                 for (pass = 0; pass < count; pass++) {
                     if (size == 3) {
                         neon_load_reg64(cpu_V0, rm + pass);
-                        tcg_gen_movi_i64(cpu_V1, imm);
+                        tcg_gen_movi_i64(cpu_V1, shift_imm);
                         switch (op) {
                         case 0:  /* VSHR */
                         case 1:  /* VSRA */
@@ -4530,7 +4531,7 @@ static int disas_neon_data_insn(CPUState
                         neon_store_reg64(cpu_V0, rd + pass);
                     } else { /* size < 3 */
                         /* Operands in T0 and T1.  */
-                        gen_op_movl_T1_im(imm);
+                        gen_op_movl_T1_im(shift_imm);
                         NEON_GET_REG(T0, rm, pass);
                         switch (op) {
                         case 0:  /* VSHR */

[Qemu-devel] Re: [PATCH] ARM NEON shift emulation fix

[Daniel Gutson]

Any update on this?

Thanks,
	Daniel.


Daniel Gutson wrote:
> Hi,
>   the attached patch fixes a bug that caused some NEON shift operations 
> to shift a wrong amount of bytes.
> 
> The problem was that a variable holding an immediate value representing 
> the amount of bits to shift was later overwritten with another value 
> (used for something different) within a loop.
> 
> Please commit this for me if approved, since I don't have write access.
> 
> Thanks!
>     Daniel.
> 
> ---
> 2009-08-21  Daniel Gutson  <dgutson@codesourcery.com>
> 
>     * target-arm/translate.c (disas_neon_data_insn): Fixed
>     shift operand.
> 

-- 
Daniel Gutson
CodeSourcery
www.codesourcery.com

[Qemu-devel] Re: [PATCH] ARM NEON shift emulation fix

[Daniel Gutson]

Hi,
   friendly reminder about this patch.

Thanks,
	Daniel.

Daniel Gutson wrote:
> Any update on this?
> 
> Thanks,
>     Daniel.
> 
> 
> Daniel Gutson wrote:
>> Hi,
>>   the attached patch fixes a bug that caused some NEON shift 
>> operations to shift a wrong amount of bytes.
>>
>> The problem was that a variable holding an immediate value 
>> representing the amount of bits to shift was later overwritten with 
>> another value (used for something different) within a loop.
>>
>> Please commit this for me if approved, since I don't have write access.
>>
>> Thanks!
>>     Daniel.
>>
>> ---
>> 2009-08-21  Daniel Gutson  <dgutson@codesourcery.com>
>>
>>     * target-arm/translate.c (disas_neon_data_insn): Fixed
>>     shift operand.
>>
> 

-- 
Daniel Gutson
CodeSourcery
www.codesourcery.com