Skip to contentSkip to conversation

ARM NEON shift emulation fix

PATCH In review Daniel Gutson 1 participant 3 messages Aug 21, 2009 – Nov 4, 2009

lore
Share & tools
lore

Daniel Gutson

author
#0 lore raw

PATCH 1/1 ARM NEON shift emulation fix

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 <[email protected]>

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

Signature · 3 lines
Daniel Gutson
CodeSourcery
www.codesourcery.com
target-arm/translate.c +9 -8
diff -Naurp qemu-0.10.5-orig/target-arm/translate.c qemu-0.10.5-dfg/target-arm/translate.c
@@ -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 */
author
#1 lore raw

Any update on this?

Thanks,
Daniel.

Quoted 17 lines · Daniel Gutson

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 <[email protected]>

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

Signature · 3 lines
Daniel Gutson
CodeSourcery
www.codesourcery.com
author
#2 lore raw

Hi,
friendly reminder about this patch.

Thanks,
Daniel.

Quoted 25 lines · Daniel Gutson

Any update on this?

Thanks,
Daniel.

Quoted 17 lines · Daniel Gutson

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 <[email protected]>

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

Signature · 3 lines
Daniel Gutson
CodeSourcery
www.codesourcery.com