Power PC Fix V bit error in 128-bit BCD add and subtract instructions

The original code was using the bcdadd / bcdsub instruction on the operand
shadow bits to calculate the shadow bits for the result.  This introduced
non-zero bits shadow bits in the result.   The shadow bits for these
instructions should be set to all valid or all invalid.  If one of the
argument shadow bits was one, then all of the shadow bits of the result should
be one.  Otherwise the result shadow bits should be zero.

This patch fixes the above bug in memcheck/mc_translate.c

Fixing the above bug broke the v-bit test.  The issue is the v-bit tester
assumes the shadow bits for the operands of a given Iop can be set to one
for testing purposes.  The implementation of the bcdadd and bcdsub was passing
a constant value for the variable ps.  The ps value is an argument to the
instruction that specifies how to set the sign code of the result.  The
implementation of the instructions was changed to issue the instruction with
ps=0.  Then the result of the instruction is updated in the VEX code if ps=1.
This changed also results in cleaning up the vbit test code.

This patch also fixes the issues with the v-bit test program.

Valgrind commit 3218

Bugzilla 360035

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15871

diff --git a/memcheck/mc_translate.c b/memcheck/mc_translate.c
index c239e4665db61755e8d2582f847dbd19a8659686..d50b53d49edaec9935cf281bc8641731ab087b39 100644 (file)
--- a/memcheck/mc_translate.c
+++ b/memcheck/mc_translate.c
@@ -852,6 +852,17 @@ static IRAtom* mkPCastTo( MCEnv* mce, IRType dst_ty, IRAtom* vbits )
                                        unop(Iop_CmpNEZ64, tmp4));
          break;
       }
+      case Ity_V128: {
+         /* Chop it in half, OR the halves together, and compare that
+          * with zero.
+          */
+         IRAtom* tmp2 = assignNew('V', mce, Ity_I64, unop(Iop_V128HIto64, vbits));
+         IRAtom* tmp3 = assignNew('V', mce, Ity_I64, unop(Iop_V128to64, vbits));
+         IRAtom* tmp4 = assignNew('V', mce, Ity_I64, binop(Iop_Or64, tmp2, tmp3));
+         tmp1         = assignNew('V', mce, Ity_I1,
+                                       unop(Iop_CmpNEZ64, tmp4));
+         break;
+      }
       default:
          ppIRType(src_ty);
          VG_(tool_panic)("mkPCastTo(1)");
@@ -2888,11 +2899,6 @@ IRAtom* expr2vbits_Triop ( MCEnv* mce,
       case Iop_SetElem32x2:
          complainIfUndefined(mce, atom2, NULL);
          return assignNew('V', mce, Ity_I64, triop(op, vatom1, atom2, vatom3));
-      /* BCDIops */
-      case Iop_BCDAdd:
-      case Iop_BCDSub:
-         complainIfUndefined(mce, atom3, NULL);
-         return assignNew('V', mce, Ity_V128, triop(op, vatom1, vatom2, atom3));
 
       /* Vector FP with rounding mode as the first arg */
       case Iop_Add64Fx2:
@@ -3723,6 +3729,10 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce,
          complainIfUndefined(mce, atom2, NULL);
          return assignNew('V', mce, Ity_V128, binop(op, vatom1, atom2));
 
+      case Iop_BCDAdd:
+      case Iop_BCDSub:
+         return mkLazy2(mce, Ity_V128, vatom1, vatom2);
+
       /* SHA Iops */
       case Iop_SHA256:
       case Iop_SHA512:

diff --git a/memcheck/tests/vbit-test/irops.c b/memcheck/tests/vbit-test/irops.c
index 83fcd02f8e09151724a8513a068a976a0b0ba51c..75dda6050fc9cb92c905da7e11b0ef7ec4e4d56e 100644 (file)
--- a/memcheck/tests/vbit-test/irops.c
+++ b/memcheck/tests/vbit-test/irops.c
@@ -1066,8 +1066,8 @@ static irop_t irops[] = {
   { DEFOP(Iop_Min32Fx8, UNDEF_UNKNOWN), },
   { DEFOP(Iop_Max64Fx4, UNDEF_UNKNOWN), },
   { DEFOP(Iop_Min64Fx4, UNDEF_UNKNOWN), },
-  { DEFOP(Iop_BCDAdd, UNDEF_SOME), .ppc64 = 1, .ppc32 = 1 },
-  { DEFOP(Iop_BCDSub, UNDEF_SOME), .ppc64 = 1, .ppc32 = 1 },
+  { DEFOP(Iop_BCDAdd, UNDEF_ALL), .ppc64 = 1, .ppc32 = 1 },
+  { DEFOP(Iop_BCDSub, UNDEF_ALL), .ppc64 = 1, .ppc32 = 1 },
   { DEFOP(Iop_PolynomialMulAdd8x16, UNDEF_ALL_8x16), .ppc64 = 1, .ppc32 = 1 },
   { DEFOP(Iop_PolynomialMulAdd16x8, UNDEF_ALL_16x8), .ppc64 = 1, .ppc32 = 1 },
   { DEFOP(Iop_PolynomialMulAdd32x4, UNDEF_ALL_32x4), .ppc64 = 1, .ppc32 = 1 },

diff --git a/memcheck/tests/vbit-test/main.c b/memcheck/tests/vbit-test/main.c
index 0cac780f7d238ab467f3d20bdfbb129f4b12e4c0..d6bd72af3e207e3b2bd4e720b810a01ee708d22b 100644 (file)
--- a/memcheck/tests/vbit-test/main.c
+++ b/memcheck/tests/vbit-test/main.c
@@ -119,18 +119,6 @@ fixup_irops(void)
       tmp->immediate_index = 2;
       tmp->immediate_type = Ity_I8;
    }
-
-   tmp = get_irop(Iop_BCDAdd);
-   if (tmp) {
-      tmp->immediate_index = 3;
-      tmp->immediate_type = Ity_I8;
-   }
-
-   tmp = get_irop(Iop_BCDSub);
-   if (tmp) {
-      tmp->immediate_index = 3;
-      tmp->immediate_type = Ity_I8;
-   }
 #endif
 }
 

diff --git a/memcheck/tests/vbit-test/ternary.c b/memcheck/tests/vbit-test/ternary.c
index b9061a41610fadadcb1ab0de554e746a668855c0..c84570c80bc2d54e8ca3fc5aa9e0016c04f0362b 100644 (file)
--- a/memcheck/tests/vbit-test/ternary.c
+++ b/memcheck/tests/vbit-test/ternary.c
@@ -51,15 +51,6 @@ check_result_for_ternary(const irop_t *op, const test_data_t *data)
       break;
 
    case UNDEF_SOME:
-      /* The result of the Iop_BCDAdd and the Iop_BCDSub has some result
-       * vbits set.  Not sure how the vbit propagation works on these Iops.
-       * for now, just make sure there are some vbits set in the result.
-       *
-       * The Iop_BCDAdd and Iop_BCDSub iops have one immediate value in the
-       * third operand.
-       *
-       * TODO, figure out details of vbit propagation for these Iops.
-       */
       expected_vbits.num_bits = result->vbits.num_bits;
 
       if ((result->vbits.bits.u128[0] != 0) ||