[PATCH] D67977: [InstCombine] Don't apply SimplifyDemandedVectorElts to ConstantAggregateZero GEP indices

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: reames, spatel, RKSimon.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

The test case here previously infinite looped.  Only one element from the GEP is used so SimplifyDemandedVectorElts would replace the other lanes in each index with undef leading to the first index being <0, undef, undef, undef>.  But there's a GEP transform that tries to replace an index into a 0 sized type with a  zero index. But the zero index check only works on ConstantInt 0 or ConstantAggregateZero so it would turn the index back to zeroinitializer. Resulting in a loop.

There are various ways to handle this that I could think
-Don't apply SimplifyDemandedVectorElts to all zero index. It doens't allow any instructions to be removed anyway.
-Improve the other transform to handle a mix of 0s and undef as being the same as zero. Probably requires a new loop since we can't just use isNullValue. Maybe there's something in PatternMatch we can use that allows undef?
-Block the other transform on vector indices.
-Use a scalar 0 in the other transform instant of a vector. But it might be the only vector index and is what makes it a vector GEP. We would need to detect this and make scalar GEP and a broadcast.
-Replace splat constant indices on GEPs with scalars. I think there's a FIXME for this already. But again this might scalarize the GEP and need a broadcast.

I went with SimplifyDemandedVectorElts since it was easy.


https://reviews.llvm.org/D67977

Files:
  llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
  llvm/test/Transforms/InstCombine/vec_demanded_elts.ll


Index: llvm/test/Transforms/InstCombine/vec_demanded_elts.ll
===================================================================
--- llvm/test/Transforms/InstCombine/vec_demanded_elts.ll
+++ llvm/test/Transforms/InstCombine/vec_demanded_elts.ll
@@ -651,3 +651,23 @@
   %r = extractelement <2 x i32*> %w, i32 0
   ret i32* %r
 }
+
+ at global = external global [0 x i32], align 4
+
+; The global here has size 0 so InstCombine would like the first index to be
+; zeroinitializer, but SimplifyDemandedVectorElts was trying to turn that index into
+; <i64 0, i64 undef, i64 undef, i64 undef>. SimplifyDemandedVectorElts has been told
+; to leave a zeroinitializer indices alone.
+define i32* @ham(<4 x i64> %arg, i64 %arg1) {
+; CHECK-LABEL: @ham(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[TMP:%.*]] = getelementptr inbounds [0 x i32], <4 x [0 x i32]*> <[0 x i32]* @global, [0 x i32]* undef, [0 x i32]* undef, [0 x i32]* undef>, <4 x i64> zeroinitializer, <4 x i64> [[ARG:%.*]]
+; CHECK-NEXT:    [[TMP2:%.*]] = extractelement <4 x i32*> [[TMP]], i64 0
+; CHECK-NEXT:    ret i32* [[TMP2]]
+;
+bb:
+  %tmp = getelementptr inbounds [0 x i32], <4 x [0 x i32]*> <[0 x i32]* @global, [0 x i32]* @global, [0 x i32]* @global, [0 x i32]* @global>, <4 x i64> zeroinitializer, <4 x i64> %arg
+  %tmp2 = extractelement <4 x i32*> %tmp, i64 0
+  ret i32* %tmp2
+}
+
Index: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -1192,6 +1192,12 @@
         return nullptr;
       }
       if (I->getOperand(i)->getType()->isVectorTy()) {
+        // Don't mess with all zeros constants. There are other combines
+        // that consider an all zero constant index canonical.
+        // FIXME: Should we skip all constant splats?
+        if (isa<Constant>(I->getOperand(i)) &&
+            cast<Constant>(I->getOperand(i))->isNullValue())
+          continue;
         APInt UndefEltsOp(VWidth, 0);
         simplifyAndSetOp(I, i, DemandedElts, UndefEltsOp);
         UndefElts |= UndefEltsOp;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67977.221574.patch
Type: text/x-patch
Size: 2191 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190924/ad3cf30b/attachment.bin>