[PATCH] D60600: [InstCombine] Fix a vector-of-pointers instcombine undef bug.

[Neil Henning via Phabricator via llvm-commits]

sheredom created this revision.
sheredom added reviewers: majnemer, uabelho.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This commit fixes a really fun bug with inst combine where you have a
vector-of-pointers and only one element of this vector is used.
InstCombine correctly notes that an element is not used, and then will
go back through a gep into that vector-of-pointers and set all vector
elements to undef. This is fine in all cases except that the langref
(and a ton of places in the optimizer) requires that indexing into a
struct has the same index for each element of the vector-of-pointers.

To fix the bug I've just made the gep/undef propagation check that the
current type being indexed into is not a struct when doing the undef
propagation.


Repository:
  rL LLVM

https://reviews.llvm.org/D60600

Files:
  lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
  test/Transforms/InstCombine/vector-of-pointers.ll


Index: test/Transforms/InstCombine/vector-of-pointers.ll
===================================================================
--- /dev/null
+++ test/Transforms/InstCombine/vector-of-pointers.ll
@@ -0,0 +1,17 @@
+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+%foo = type { float, i8 }
+
+define void @func(float* %out, [2 x %foo]* %in) {
+  %a = insertelement <2 x [2 x %foo]*> undef, [2 x %foo]* %in, i32 0
+  %b = insertelement <2 x [2 x %foo]*> %a, [2 x %foo]* %in, i32 1
+
+; CHECK: %gep = getelementptr [2 x %foo], <2 x [2 x %foo]*> %b, <2 x i64> <i64 undef, i64 0>, <2 x i64> <i64 undef, i64 1>, <2 x i32> zeroinitializer
+  %gep = getelementptr [2 x %foo], <2 x [2 x %foo]*> %b, <2 x i32> zeroinitializer, <2 x i32> <i32 0, i32 1>, <2 x i32> zeroinitializer
+
+  %extract = extractelement <2 x float*> %gep, i64 1
+
+  %load = load float, float* %extract, align 4
+  store float %load, float* %out
+  ret void
+}
Index: lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -1173,17 +1173,38 @@
     // operands we may have.  We know there must be at least one, or we
     // wouldn't have a vector result to get here. Note that we intentionally
     // merge the undef bits here since gepping with either an undef base or
-    // index results in undef. 
-    for (unsigned i = 0; i < I->getNumOperands(); i++) {
+    // index results in undef.
+
+    auto simplifyGEPOperand = [&](unsigned i, bool isIndexStruct) -> bool {
       if (isa<UndefValue>(I->getOperand(i))) {
         // If the entire vector is undefined, just return this info.
         UndefElts = EltMask;
-        return nullptr;
+        return true;
       }
+
       if (I->getOperand(i)->getType()->isVectorTy()) {
-        APInt UndefEltsOp(VWidth, 0);
-        simplifyAndSetOp(I, i, DemandedElts, UndefEltsOp);
-        UndefElts |= UndefEltsOp;
+        // If we have a vector of indices into a struct element of the GEP, and
+        // change a single element this into an undef while preserving the
+        // others, that breaks the guarantee that each index of a
+        // vector-of-pointers into a struct will have the same index.
+        if (!isIndexStruct) {
+          APInt UndefEltsOp(VWidth, 0);
+          simplifyAndSetOp(I, i, DemandedElts, UndefEltsOp);
+          UndefElts |= UndefEltsOp;
+        }
+      }
+
+      return false;
+    };
+
+    if (simplifyGEPOperand(0, false)) {
+      return nullptr;
+    }
+
+    gep_type_iterator GTI = gep_type_begin(cast<GetElementPtrInst>(I));
+    for (unsigned i = 1; i < I->getNumOperands(); i++, GTI++) {
+      if (simplifyGEPOperand(i, GTI.isStruct())) {
+        return nullptr;
       }
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60600.194814.patch
Type: text/x-patch
Size: 2857 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190412/a93e5be1/attachment.bin>