[InstCombine] Use m_Zero instead of isNullValue() when checking if a GEP index is all zeroes to prevent an infinite loop.

[craig.topper]

Summary

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.

The fix is to use m_Zero() to allow a vector of zeroes and undefs.

[reames]

I'm fine with this landing as a workaround, but it really feels like the right fix is on the other transform. <0, undef> is effectively a zero index (or can be chosen as such), and our transforms should be aware of that.

More generally, I'm increasingly thinking the implicit broadcast feature of vector geps is a mistake. I'm tempted to just change the LangRef and require a separate explicit broadcast via a shuffle.

[spatel]

Maybe there's something in PatternMatch we can use that allows undef?

I haven't stepped through the example, but m_Zero() and m_ZeroInt() allow undefs.

[spatel]

LGTM

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Type *NewIndexType =
         IndexTy->isVectorTy()
             ? VectorType::get(NewScalarIndexTy, IndexTy->getVectorNumElements())
             : NewScalarIndexTy;
 
     // If the element type has zero size then any index over it is equivalent
     // to an index of zero, so replace it with zero if it is not zero already.
     Type *EltTy = GTI.getIndexedType();
     if (EltTy->isSized() && DL.getTypeAllocSize(EltTy) == 0)
-      if (!isa<Constant>(*I) || !cast<Constant>(*I)->isNullValue()) {
+      if (!isa<Constant>(*I) || !match(I->get(), m_Zero())) {
         *I = Constant::getNullValue(NewIndexType);
         MadeChange = true;
       }
 
     if (IndexTy != NewIndexType) {
       // If we are using a wider index than needed for this platform, shrink
       // it to what we need.  If narrower, sign-extend it to what we need.
       // This explicit cast can make subsequent optimizations more obvious.

llvm/test/Transforms/InstCombine/vec_demanded_elts.ll

 ; CHECK-LABEL: @PR41624(
 ; CHECK-NEXT:   %w = getelementptr { i32, i32 }, <2 x { i32, i32 }*> %a, <2 x i64> <i64 5, i64 5>, <2 x i32> zeroinitializer
 ; CHECK-NEXT:   %r = extractelement <2 x i32*> %w, i32 0
 ; CHECK-NEXT:   ret i32* %r
   %w = getelementptr { i32, i32 }, <2 x { i32, i32 }*> %a, <2 x i64> <i64 5, i64 5>, <2 x i32> zeroinitializer
   %r = extractelement <2 x i32*> %w, i32 0
   ret i32* %r
 }
+
+@global = external global [0 x i32], align 4
+
+; Make sure we don't get stuck in a loop turning the zeroinitializer into
+; <0, undef, undef, undef> and then changing it back.
+define i32* @zero_sized_type_extract(<4 x i64> %arg, i64 %arg1) {
+; CHECK-LABEL: @zero_sized_type_extract(
+; 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> <i64 0, i64 undef, i64 undef, i64 undef>, <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
+}