[llvm] r358299 - [InstCombine] Fix a nasty miscompile introduced w/masked.gather demanded elts

[Philip Reames via llvm-commits]

Author: reames
Date: Fri Apr 12 11:26:56 2019
New Revision: 358299

URL: http://llvm.org/viewvc/llvm-project?rev=358299&view=rev
Log:
[InstCombine] Fix a nasty miscompile introduced w/masked.gather demanded elts

This fixes a miscompile which was introduced in r356510 (https://reviews.llvm.org/D57372).

The problem is that the original patch removed pointer operands where the load results we're demanded, but without considering the legality of the load itself.  If the masked.gather had active, but undemanded, lanes, then we could end up creating a load which loaded from an undef address.  The result could be a segfault, or, in theory, an arbitrary read from a random memory location into an used register.  


Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
    llvm/trunk/test/Transforms/InstCombine/masked_intrinsics.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp?rev=358299&r1=358298&r2=358299&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp Fri Apr 12 11:26:56 2019
@@ -1437,7 +1437,11 @@ Value *InstCombiner::SimplifyDemandedVec
     switch (II->getIntrinsicID()) {
     case Intrinsic::masked_gather: // fallthrough
     case Intrinsic::masked_load: {
-      APInt DemandedPtrs(DemandedElts), DemandedPassThrough(DemandedElts);
+      // Subtlety: If we load from a pointer, the pointer must be valid
+      // regardless of whether the element is demanded.  Doing otherwise risks
+      // segfaults which didn't exist in the original program.
+      APInt DemandedPtrs(APInt::getAllOnesValue(VWidth)),
+        DemandedPassThrough(DemandedElts);
       if (auto *CV = dyn_cast<ConstantVector>(II->getOperand(2)))
         for (unsigned i = 0; i < VWidth; i++) {
           Constant *CElt = CV->getAggregateElement(i);

Modified: llvm/trunk/test/Transforms/InstCombine/masked_intrinsics.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/masked_intrinsics.ll?rev=358299&r1=358298&r2=358299&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/masked_intrinsics.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/masked_intrinsics.ll Fri Apr 12 11:26:56 2019
@@ -56,10 +56,9 @@ define <2 x double> @load_lane0(<2 x dou
   ret <2 x double> %res
 }
 
-; FIXME: the output here demonstrates a miscompile!
 define double @load_all(double* %base, double %pt)  {
 ; CHECK-LABEL: @load_all(
-; CHECK-NEXT:    [[PTRS:%.*]] = getelementptr double, double* [[BASE:%.*]], <4 x i64> <i64 undef, i64 undef, i64 2, i64 undef>
+; CHECK-NEXT:    [[PTRS:%.*]] = getelementptr double, double* [[BASE:%.*]], <4 x i64> <i64 0, i64 undef, i64 2, i64 3>
 ; CHECK-NEXT:    [[RES:%.*]] = call <4 x double> @llvm.masked.gather.v4f64.v4p0f64(<4 x double*> [[PTRS]], i32 4, <4 x i1> <i1 true, i1 false, i1 true, i1 true>, <4 x double> undef)
 ; CHECK-NEXT:    [[ELT:%.*]] = extractelement <4 x double> [[RES]], i64 2
 ; CHECK-NEXT:    ret double [[ELT]]