[Valgrind-developers] [valgrind] PPC64, vpermr, xxperm, xxpermr fix Iop_Perm8x16 selector field

[Carl L. <ca...@so...>]

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=856d45eb7e3661a61ace32be2cfa10bf198620c8

commit 856d45eb7e3661a61ace32be2cfa10bf198620c8
Author: Carl Love <ca...@us...>
Date:   Thu Oct 5 12:19:59 2017 -0500

    PPC64, vpermr, xxperm, xxpermr fix Iop_Perm8x16 selector field
    
    The implementation of the vpermr, xxperm, xxpermr violate this by
    using a mask of 0x1F.  Fix the code and the corresponding comments
    to met the definition for Iop_Perm8x16.  Use Iop_Dup8x16 to generate
    vector value for subtraction.
    
    Bugzilla 385334.

Diff:
---
 NEWS                      |  1 +
 VEX/priv/guest_ppc_toIR.c | 38 +++++++++++++++++++-------------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/NEWS b/NEWS
index efa1f4a..097930a 100644
--- a/NEWS
+++ b/NEWS
@@ -63,6 +63,7 @@ n-i-bz  Fix missing workq_ops operations (macOS)
 385208  PPC64, xxperm instruction exhausts temporary memory
 385210  PPC64, vpermr instruction could exhaust temporary memory
 385183  PPC64, Add support for xscmpeqdp, xscmpgtdp, xscmpgedp, xsmincdp instructions
+385334  PPC64, fix vpermr, xxperm, xxpermr mask value.
 
 Release 3.13.0 (15 June 2017)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/VEX/priv/guest_ppc_toIR.c b/VEX/priv/guest_ppc_toIR.c
index b5b0d03..f63146e 100644
--- a/VEX/priv/guest_ppc_toIR.c
+++ b/VEX/priv/guest_ppc_toIR.c
@@ -22579,6 +22579,7 @@ dis_vx_permute_misc( UInt theInstr, UInt opc2 )
          IRTemp b_perm  = newTemp(Ity_V128);
          IRTemp mask    = newTemp(Ity_V128);
          IRTemp perm_val = newTemp(Ity_V128);
+         IRTemp vB_adj = newTemp( Ity_V128 );
 
          if ( opc2 == 0x68 ) {
             DIP("xxperm v%d,v%d,v%d\n", (UInt)XT, (UInt)XA, (UInt)XB);
@@ -22591,29 +22592,27 @@ dis_vx_permute_misc( UInt theInstr, UInt opc2 )
          assign( vT, getVSReg( XT ) );
 
          if ( opc2 == 0x68 ) // xxperm
-            assign( perm_val,
-                    binop( Iop_AndV128, mkexpr( vB ),
-                           unop( Iop_Dup8x16, mkU8( 0x1F ) ) ) );
+            assign( vB_adj, mkexpr( vB ) );
 
          else                // xxpermr
-            assign( perm_val,
+            assign( vB_adj,
                     binop( Iop_Sub16x8,
-                           binop( Iop_64HLtoV128,
-                                  mkU64( 0x1F1F1F1F1F1F1F1F ),
-                                  mkU64( 0x1F1F1F1F1F1F1F1F ) ),
-                           binop( Iop_AndV128, mkexpr( vB ),
-                                  unop( Iop_Dup8x16, mkU8( 0x1F ) ) ) ) );
+                           unop( Iop_Dup8x16, mkU8( 0x1F ) ),
+                           mkexpr( vB ) ) );
 
-         /* Limit the Perm8x16 steering values to 0 .. 31 as that is what
+         /* Limit the Perm8x16 steering values to 0 .. 15 as that is what
             IR specifies, and also to hide irrelevant bits from
             memcheck.
          */
+         assign( perm_val,
+                 binop( Iop_AndV128, mkexpr( vB_adj ),
+                        unop( Iop_Dup8x16, mkU8( 0xF ) ) ) );
          assign( a_perm,
                  binop( Iop_Perm8x16, mkexpr( vA ), mkexpr( perm_val ) ) );
          assign( b_perm,
                  binop( Iop_Perm8x16, mkexpr( vT ), mkexpr( perm_val ) ) );
          assign( mask, binop( Iop_SarN8x16,
-                              binop( Iop_ShlN8x16, mkexpr( perm_val ),
+                              binop( Iop_ShlN8x16, mkexpr( vB_adj ),
                                      mkU8( 3 ) ),
                               mkU8( 7 ) ) );
          // dst = (a & ~mask) | (b & mask)
@@ -24361,28 +24360,29 @@ static Bool dis_av_permute ( UInt theInstr )
       IRTemp b_perm  = newTemp( Ity_V128 );
       IRTemp mask    = newTemp( Ity_V128 );
       IRTemp vC_andF = newTemp( Ity_V128 );
+      IRTemp vC_adj = newTemp( Ity_V128 );
 
       DIP( "vpermr v%d,v%d,v%d,v%d\n",
            vD_addr, vA_addr, vB_addr, vC_addr);
-      /* Limit the Perm8x16 steering values to 0 .. 31 as that is what
+      /* Limit the Perm8x16 steering values to 0 .. 15 as that is what
          IR specifies, and also to hide irrelevant bits from
          memcheck.
       */
 
+      assign( vC_adj,
+                    binop( Iop_Sub16x8,
+			   unop( Iop_Dup8x16, mkU8( 0x1F ) ),
+			   mkexpr( vC ) ) );
       assign( vC_andF,
-              binop( Iop_Sub16x8,
-                     binop( Iop_64HLtoV128,
-                            mkU64( 0x1F1F1F1F1F1F1F1F ),
-                            mkU64( 0x1F1F1F1F1F1F1F1F ) ),
-                     binop( Iop_AndV128, mkexpr( vC ),
-                            unop( Iop_Dup8x16, mkU8( 0x1F ) ) ) ) );
+              binop( Iop_AndV128, mkexpr( vC_adj),
+                     unop( Iop_Dup8x16, mkU8( 0xF ) ) ) );
       assign( a_perm,
               binop( Iop_Perm8x16, mkexpr( vA ), mkexpr( vC_andF ) ) );
       assign( b_perm,
               binop( Iop_Perm8x16, mkexpr( vB ), mkexpr( vC_andF ) ) );
       // mask[i8] = (vC[i8]_4 == 1) ? 0xFF : 0x0
       assign( mask, binop(Iop_SarN8x16,
-                          binop( Iop_ShlN8x16, mkexpr( vC_andF ),
+                          binop( Iop_ShlN8x16, mkexpr( vC_adj ),
                                  mkU8( 3 ) ), mkU8( 7 ) ) );
       // dst = (a & ~mask) | (b & mask)
       putVReg( vD_addr, binop( Iop_OrV128,