commit e221eca26be6b2396e3fcbf4117e630fc22e79f6

Author: Julian Seward <jseward@acm.org>

Date:   Tue Nov 20 11:28:42 2018 +0100

    Add Memcheck support for IROps added in 42719898.

    memcheck/mc_translate.c:

    Add mkRight{32,64} as right-travelling analogues to mkLeft{32,64}.

    doCmpORD: for the cases of a signed comparison against zero, compute

    definedness of the 3 result bits (lt,gt,eq) separately, and, for the lt and eq

    bits, do it exactly accurately.

    expensiveCountTrailingZeroes: no functional change.  Re-analyse/verify and add

    comments.

    expensiveCountLeadingZeroes: add.  Very similar to

    expensiveCountTrailingZeroes.

    Add some comments to mark unary ops which are self-shadowing.

    Route Iop_Ctz{,Nat}{32,64} through expensiveCountTrailingZeroes.

    Route Iop_Clz{,Nat}{32,64} through expensiveCountLeadingZeroes.

    Add instrumentation for Iop_PopCount{32,64} and Iop_Reverse8sIn32_x1.

    memcheck/tests/vbit-test/irops.c

    Add dummy new entries for all new IROps, just enough to make it compile and

    run.

diff --git a/memcheck/mc_translate.c b/memcheck/mc_translate.c

index 68a2ab3..c24db91 100644

--- a/memcheck/mc_translate.c

+++ b/memcheck/mc_translate.c

@@ -737,6 +737,34 @@ static IRAtom* mkLeft64 ( MCEnv* mce, IRAtom* a1 ) {

    return assignNew('V', mce, Ity_I64, unop(Iop_Left64, a1));

 }

+/* --------- The Right-family of operations. --------- */

+

+/* Unfortunately these are a lot more expensive then their Left

+   counterparts.  Fortunately they are only very rarely used -- only for

+   count-leading-zeroes instrumentation. */

+

+static IRAtom* mkRight32 ( MCEnv* mce, IRAtom* a1 )

+{

+   for (Int i = 1; i <= 16; i *= 2) {

+      // a1 |= (a1 >>u i)

+      IRAtom* tmp

+         = assignNew('V', mce, Ity_I32, binop(Iop_Shr32, a1, mkU8(i)));

+      a1 = assignNew('V', mce, Ity_I32, binop(Iop_Or32, a1, tmp));

+   }

+   return a1;

+}

+

+static IRAtom* mkRight64 ( MCEnv* mce, IRAtom* a1 )

+{

+   for (Int i = 1; i <= 32; i *= 2) {

+      // a1 |= (a1 >>u i)

+      IRAtom* tmp

+         = assignNew('V', mce, Ity_I64, binop(Iop_Shr64, a1, mkU8(i)));

+      a1 = assignNew('V', mce, Ity_I64, binop(Iop_Or64, a1, tmp));

+   }

+   return a1;

+}

+

 /* --------- 'Improvement' functions for AND/OR. --------- */

 /* ImproveAND(data, vbits) = data OR vbits.  Defined (0) data 0s give

@@ -1280,20 +1308,18 @@ static IRAtom* doCmpORD ( MCEnv*  mce,

                           IRAtom* xxhash, IRAtom* yyhash, 

                           IRAtom* xx,     IRAtom* yy )

 {

-   Bool   m64    = cmp_op == Iop_CmpORD64S || cmp_op == Iop_CmpORD64U;

-   Bool   syned  = cmp_op == Iop_CmpORD64S || cmp_op == Iop_CmpORD32S;

-   IROp   opOR   = m64 ? Iop_Or64  : Iop_Or32;

-   IROp   opAND  = m64 ? Iop_And64 : Iop_And32;

-   IROp   opSHL  = m64 ? Iop_Shl64 : Iop_Shl32;

-   IROp   opSHR  = m64 ? Iop_Shr64 : Iop_Shr32;

-   IRType ty     = m64 ? Ity_I64   : Ity_I32;

-   Int    width  = m64 ? 64        : 32;

+   Bool   m64      = cmp_op == Iop_CmpORD64S || cmp_op == Iop_CmpORD64U;

+   Bool   syned    = cmp_op == Iop_CmpORD64S || cmp_op == Iop_CmpORD32S;

+   IROp   opOR     = m64 ? Iop_Or64   : Iop_Or32;

+   IROp   opAND    = m64 ? Iop_And64  : Iop_And32;

+   IROp   opSHL    = m64 ? Iop_Shl64  : Iop_Shl32;

+   IROp   opSHR    = m64 ? Iop_Shr64  : Iop_Shr32;

+   IROp   op1UtoWS = m64 ? Iop_1Uto64 : Iop_1Uto32;

+   IRType ty       = m64 ? Ity_I64    : Ity_I32;

+   Int    width    = m64 ? 64         : 32;

    Bool (*isZero)(IRAtom*) = m64 ? isZeroU64 : isZeroU32;

-   IRAtom* threeLeft1 = NULL;

-   IRAtom* sevenLeft1 = NULL;

-

    tl_assert(isShadowAtom(mce,xxhash));

    tl_assert(isShadowAtom(mce,yyhash));

    tl_assert(isOriginalAtom(mce,xx));

@@ -1312,30 +1338,55 @@ static IRAtom* doCmpORD ( MCEnv*  mce,

       /* fancy interpretation */

       /* if yy is zero, then it must be fully defined (zero#). */

       tl_assert(isZero(yyhash));

-      threeLeft1 = m64 ? mkU64(3<<1) : mkU32(3<<1);

+      // This is still inaccurate, but I don't think it matters, since

+      // nobody writes code of the form

+      // "is <partially-undefined-value> signedly greater than zero?".

+      // We therefore simply declare "x >s 0" to be undefined if any bit in

+      // x is undefined.  That's clearly suboptimal in some cases.  Eg, if

+      // the highest order bit is a defined 1 then x is negative so it

+      // doesn't matter whether the remaining bits are defined or not.

+      IRAtom* t_0_gt_0_0

+         = assignNew(

+              'V', mce,ty,

+              binop(

+                 opAND,

+                 mkPCastTo(mce,ty, xxhash),

+                 m64 ? mkU64(1<<2) : mkU32(1<<2)

+              ));

+      // For "x 

+      // and we have a precise result.

+      IRAtom* t_lt_0_0_0

+         = assignNew(

+              'V', mce,ty,

+              binop(

+                 opSHL,

+                 assignNew(

+                    'V', mce,ty,

+                    binop(opSHR, xxhash, mkU8(width-1))),

+                 mkU8(3)

+              ));

+      // For "x == 0" we can hand the problem off to expensiveCmpEQorNE.

+      IRAtom* t_0_0_eq_0

+         = assignNew(

+              'V', mce,ty,

+              binop(

+                 opSHL,

+                 assignNew('V', mce,ty,

+                    unop(

+                    op1UtoWS,

+                    expensiveCmpEQorNE(mce, ty, xxhash, yyhash, xx, yy))

+                 ),

+                 mkU8(1)

+              ));

       return

          binop(

             opOR,

-            assignNew(

-               'V', mce,ty,

-               binop(

-                  opAND,

-                  mkPCastTo(mce,ty, xxhash), 

-                  threeLeft1

-               )),

-            assignNew(

-               'V', mce,ty,

-               binop(

-                  opSHL,

-                  assignNew(

-                     'V', mce,ty,

-                     binop(opSHR, xxhash, mkU8(width-1))),

-                  mkU8(3)

-               ))

-	 );

+            assignNew('V', mce,ty, binop(opOR, t_lt_0_0_0, t_0_gt_0_0)),

+            t_0_0_eq_0

+         );

    } else {

       /* standard interpretation */

-      sevenLeft1 = m64 ? mkU64(7<<1) : mkU32(7<<1);

+      IRAtom* sevenLeft1 = m64 ? mkU64(7<<1) : mkU32(7<<1);

       return 

          binop( 

             opAND, 

@@ -2211,14 +2262,14 @@ IRAtom* expensiveCountTrailingZeroes ( MCEnv* mce, IROp czop,

    tl_assert(sameKindedAtoms(atom,vatom));

    switch (czop) {

-      case Iop_Ctz32:

+      case Iop_Ctz32: case Iop_CtzNat32:

          ty = Ity_I32;

          xorOp = Iop_Xor32;

          subOp = Iop_Sub32;

          andOp = Iop_And32;

          one = mkU32(1);

          break;

-      case Iop_Ctz64:

+      case Iop_Ctz64: case Iop_CtzNat64:

          ty = Ity_I64;

          xorOp = Iop_Xor64;

          subOp = Iop_Sub64;

@@ -2232,8 +2283,30 @@ IRAtom* expensiveCountTrailingZeroes ( MCEnv* mce, IROp czop,

    // improver = atom ^ (atom - 1)

    //

-   // That is, improver has its low ctz(atom) bits equal to one;

-   // higher bits (if any) equal to zero.

+   // That is, improver has its low ctz(atom)+1 bits equal to one;

+   // higher bits (if any) equal to zero.  So it's exactly the right

+   // mask to use to remove the irrelevant undefined input bits.

+   /* Here are some examples:

+         atom   = U...U 1 0...0

+         atom-1 = U...U 0 1...1

+         ^ed    = 0...0 1 11111, which correctly describes which bits of |atom|

+                                 actually influence the result

+      A boundary case

+         atom   = 0...0

+         atom-1 = 1...1

+         ^ed    = 11111, also a correct mask for the input: all input bits

+                         are relevant

+      Another boundary case

+         atom   = 1..1 1

+         atom-1 = 1..1 0

+         ^ed    = 0..0 1, also a correct mask: only the rightmost input bit

+                          is relevant

+      Now with misc U bits interspersed:

+         atom   = U...U 1 0 U...U 0 1 0...0

+         atom-1 = U...U 1 0 U...U 0 0 1...1

+         ^ed    = 0...0 0 0 0...0 0 1 1...1, also correct

+      (Per re-check/analysis of 14 Nov 2018)

+   */

    improver = assignNew('V', mce,ty,

                         binop(xorOp,

                               atom,

@@ -2242,8 +2315,96 @@ IRAtom* expensiveCountTrailingZeroes ( MCEnv* mce, IROp czop,

    // improved = vatom & improver

    //

-   // That is, treat any V bits above the first ctz(atom) bits as

-   // "defined".

+   // That is, treat any V bits to the left of the rightmost ctz(atom)+1

+   // bits as "defined".

+   improved = assignNew('V', mce, ty,

+                        binop(andOp, vatom, improver));

+

+   // Return pessimizing cast of improved.

+   return mkPCastTo(mce, ty, improved);

+}

+

+static

+IRAtom* expensiveCountLeadingZeroes ( MCEnv* mce, IROp czop,

+                                      IRAtom* atom, IRAtom* vatom )

+{

+   IRType ty;

+   IROp shrOp, notOp, andOp;

+   IRAtom* (*mkRight)(MCEnv*, IRAtom*);

+   IRAtom *improver, *improved;

+   tl_assert(isShadowAtom(mce,vatom));

+   tl_assert(isOriginalAtom(mce,atom));

+   tl_assert(sameKindedAtoms(atom,vatom));

+

+   switch (czop) {

+      case Iop_Clz32: case Iop_ClzNat32:

+         ty = Ity_I32;

+         shrOp = Iop_Shr32;

+         notOp = Iop_Not32;

+         andOp = Iop_And32;

+         mkRight = mkRight32;

+         break;

+      case Iop_Clz64: case Iop_ClzNat64:

+         ty = Ity_I64;

+         shrOp = Iop_Shr64;

+         notOp = Iop_Not64;

+         andOp = Iop_And64;

+         mkRight = mkRight64;

+         break;

+      default:

+         ppIROp(czop);

+         VG_(tool_panic)("memcheck:expensiveCountLeadingZeroes");

+   }

+

+   // This is in principle very similar to how expensiveCountTrailingZeroes

+   // works.  That function computed an "improver", which it used to mask

+   // off all but the rightmost 1-bit and the zeroes to the right of it,

+   // hence removing irrelevant bits from the input.  Here, we play the

+   // exact same game but with the left-vs-right roles interchanged.

+   // Unfortunately calculation of the improver in this case is

+   // significantly more expensive.

+   //

+   // improver = ~(RIGHT(atom) >>u 1)

+   //

+   // That is, improver has its upper clz(atom)+1 bits equal to one;

+   // lower bits (if any) equal to zero.  So it's exactly the right

+   // mask to use to remove the irrelevant undefined input bits.

+   /* Here are some examples:

+         atom             = 0...0 1 U...U

+         R(atom)          = 0...0 1 1...1

+         R(atom) >>u 1    = 0...0 0 1...1

+         ~(R(atom) >>u 1) = 1...1 1 0...0

+                            which correctly describes which bits of |atom|

+                            actually influence the result

+      A boundary case

+         atom             = 0...0

+         R(atom)          = 0...0

+         R(atom) >>u 1    = 0...0

+         ~(R(atom) >>u 1) = 1...1

+                            also a correct mask for the input: all input bits

+                            are relevant

+      Another boundary case

+         atom             = 1 1..1

+         R(atom)          = 1 1..1

+         R(atom) >>u 1    = 0 1..1

+         ~(R(atom) >>u 1) = 1 0..0

+                            also a correct mask: only the leftmost input bit

+                            is relevant

+      Now with misc U bits interspersed:

+         atom             = 0...0 1 U...U 0 1 U...U

+         R(atom)          = 0...0 1 1...1 1 1 1...1

+         R(atom) >>u 1    = 0...0 0 1...1 1 1 1...1

+         ~(R(atom) >>u 1) = 1...1 1 0...0 0 0 0...0, also correct

+      (Per initial implementation of 15 Nov 2018)

+   */

+   improver = mkRight(mce, atom);

+   improver = assignNew('V', mce, ty, binop(shrOp, improver, mkU8(1)));

+   improver = assignNew('V', mce, ty, unop(notOp, improver));

+

+   // improved = vatom & improver

+   //

+   // That is, treat any V bits to the right of the leftmost clz(atom)+1

+   // bits as "defined".

    improved = assignNew('V', mce, ty,

                         binop(andOp, vatom, improver));

@@ -4705,6 +4866,7 @@ IRExpr* expr2vbits_Unop ( MCEnv* mce, IROp op, IRAtom* atom )

       case Iop_RecipEst32F0x4:

          return unary32F0x4(mce, vatom);

+      // These are self-shadowing.

       case Iop_32UtoV128:

       case Iop_64UtoV128:

       case Iop_Dup8x16:

@@ -4745,6 +4907,7 @@ IRExpr* expr2vbits_Unop ( MCEnv* mce, IROp op, IRAtom* atom )

       case Iop_MulI128by10Carry:

       case Iop_F16toF64x2:

       case Iop_F64toF16x2:

+         // FIXME JRS 2018-Nov-15.  This is surely not correct!

          return vatom;

       case Iop_I32StoF128: /* signed I32 -> F128 */

@@ -4770,7 +4933,6 @@ IRExpr* expr2vbits_Unop ( MCEnv* mce, IROp op, IRAtom* atom )

       case Iop_RoundF64toF64_NegINF:

       case Iop_RoundF64toF64_PosINF:

       case Iop_RoundF64toF64_ZERO:

-      case Iop_Clz64:

       case Iop_D32toD64:

       case Iop_I32StoD64:

       case Iop_I32UtoD64:

@@ -4785,17 +4947,32 @@ IRExpr* expr2vbits_Unop ( MCEnv* mce, IROp op, IRAtom* atom )

       case Iop_D64toD128:

          return mkPCastTo(mce, Ity_I128, vatom);

-      case Iop_Clz32:

       case Iop_TruncF64asF32:

       case Iop_NegF32:

       case Iop_AbsF32:

       case Iop_F16toF32: 

          return mkPCastTo(mce, Ity_I32, vatom);

-      case Iop_Ctz32:

-      case Iop_Ctz64:

+      case Iop_Ctz32: case Iop_CtzNat32:

+      case Iop_Ctz64: case Iop_CtzNat64:

          return expensiveCountTrailingZeroes(mce, op, atom, vatom);

+      case Iop_Clz32: case Iop_ClzNat32:

+      case Iop_Clz64: case Iop_ClzNat64:

+         return expensiveCountLeadingZeroes(mce, op, atom, vatom);

+

+      // PopCount32: this is slightly pessimistic.  It is true that the

+      // result depends on all input bits, so that aspect of the PCast is

+      // correct.  However, regardless of the input, only the lowest 5 bits

+      // out of the output can ever be undefined.  So we could actually

+      // "improve" the results here by marking the top 27 bits of output as

+      // defined.  A similar comment applies for PopCount64.

+      case Iop_PopCount32:

+         return mkPCastTo(mce, Ity_I32, vatom);

+      case Iop_PopCount64:

+         return mkPCastTo(mce, Ity_I64, vatom);

+

+      // These are self-shadowing.

       case Iop_1Uto64:

       case Iop_1Sto64:

       case Iop_8Uto64:

@@ -4821,6 +4998,7 @@ IRExpr* expr2vbits_Unop ( MCEnv* mce, IROp op, IRAtom* atom )

       case Iop_V256to64_2: case Iop_V256to64_3:

          return assignNew('V', mce, Ity_I64, unop(op, vatom));

+      // These are self-shadowing.

       case Iop_64to32:

       case Iop_64HIto32:

       case Iop_1Uto32:

@@ -4830,8 +5008,10 @@ IRExpr* expr2vbits_Unop ( MCEnv* mce, IROp op, IRAtom* atom )

       case Iop_16Sto32:

       case Iop_8Sto32:

       case Iop_V128to32:

+      case Iop_Reverse8sIn32_x1:

          return assignNew('V', mce, Ity_I32, unop(op, vatom));

+      // These are self-shadowing.

       case Iop_8Sto16:

       case Iop_8Uto16:

       case Iop_32to16:

@@ -4840,6 +5020,7 @@ IRExpr* expr2vbits_Unop ( MCEnv* mce, IROp op, IRAtom* atom )

       case Iop_GetMSBs8x16:

          return assignNew('V', mce, Ity_I16, unop(op, vatom));

+      // These are self-shadowing.

       case Iop_1Uto8:

       case Iop_1Sto8:

       case Iop_16to8:

@@ -4868,6 +5049,7 @@ IRExpr* expr2vbits_Unop ( MCEnv* mce, IROp op, IRAtom* atom )

       case Iop_Not16:

       case Iop_Not8:

       case Iop_Not1:

+         // FIXME JRS 2018-Nov-15.  This is surely not correct!

          return vatom;

       case Iop_CmpNEZ8x8:

@@ -4929,6 +5111,7 @@ IRExpr* expr2vbits_Unop ( MCEnv* mce, IROp op, IRAtom* atom )

       case Iop_Ctz64x2:

          return mkPCast64x2(mce, vatom);

+      // This is self-shadowing.

       case Iop_PwBitMtxXpose64x2:

          return assignNew('V', mce, Ity_V128, unop(op, vatom));

diff --git a/memcheck/tests/vbit-test/irops.c b/memcheck/tests/vbit-test/irops.c

index bfd82fc..e8bf67d 100644

--- a/memcheck/tests/vbit-test/irops.c

+++ b/memcheck/tests/vbit-test/irops.c

@@ -111,6 +111,12 @@ static irop_t irops[] = {

   { DEFOP(Iop_Clz32,      UNDEF_ALL),  .s390x = 0, .amd64 = 0, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 =1, .mips64 = 1 },

   { DEFOP(Iop_Ctz64,      UNDEF_ALL),  .s390x = 0, .amd64 = 1, .x86 = 0, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 =0, .mips64 = 0 },

   { DEFOP(Iop_Ctz32,      UNDEF_ALL),  .s390x = 0, .amd64 = 0, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 =0, .mips64 = 0 },

+  { DEFOP(Iop_ClzNat64,   UNDEF_ALL),  .s390x = 0, .amd64 = 0, .x86 = 0, .arm = 0, .ppc64 = 1, .ppc32 = 0, .mips32 =0, .mips64 = 0 }, // ppc32 asserts

+  { DEFOP(Iop_ClzNat32,   UNDEF_ALL),  .s390x = 0, .amd64 = 0, .x86 = 0, .arm = 0, .ppc64 = 1, .ppc32 = 1, .mips32 =0, .mips64 = 0 },

+  { DEFOP(Iop_CtzNat64,   UNDEF_ALL),  .s390x = 0, .amd64 = 0, .x86 = 0, .arm = 0, .ppc64 = 1, .ppc32 = 0, .mips32 =0, .mips64 = 0 },

+  { DEFOP(Iop_CtzNat32,   UNDEF_ALL),  .s390x = 0, .amd64 = 0, .x86 = 0, .arm = 0, .ppc64 = 0, .ppc32 = 1, .mips32 =0, .mips64 = 0 },

+  { DEFOP(Iop_PopCount64, UNDEF_ALL),  .s390x = 0, .amd64 = 0, .x86 = 0, .arm = 0, .ppc64 = 1, .ppc32 = 0, .mips32 =0, .mips64 = 0 },

+  { DEFOP(Iop_PopCount32, UNDEF_ALL),  .s390x = 0, .amd64 = 0, .x86 = 0, .arm = 0, .ppc64 = 1, .ppc32 = 1, .mips32 =0, .mips64 = 0 },

   { DEFOP(Iop_CmpLT32S,   UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 =1, .mips64 = 1 },

   { DEFOP(Iop_CmpLT64S,   UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 0, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 =0, .mips64 = 1 }, // ppc, mips assert

   { DEFOP(Iop_CmpLE32S,   UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 =1, .mips64 = 1 },

@@ -336,6 +342,7 @@ static irop_t irops[] = {

   { DEFOP(Iop_Sad8Ux4, UNDEF_UNKNOWN), },

   { DEFOP(Iop_CmpNEZ16x2, UNDEF_UNKNOWN), },

   { DEFOP(Iop_CmpNEZ8x4, UNDEF_UNKNOWN), },

+  { DEFOP(Iop_Reverse8sIn32_x1, UNDEF_UNKNOWN) },

   /* ------------------ 64-bit SIMD FP ------------------------ */

   { DEFOP(Iop_I32UtoFx2, UNDEF_UNKNOWN), },

   { DEFOP(Iop_I32StoFx2, UNDEF_UNKNOWN), },