[InstSimplify] Fix SimplifyGEPInst when GEP index type is vector type.

[huihuiz]

Summary

The following folds

"gep (gep V, C), (sub 0, V) -> C"
"gep (gep V, C), (xor V, -1) -> C-1"

are only valid when GEP index type is scalar type.

This patch bailout early when GEP index type is vector type. Explicitly call
TypeSize::getFixedSize to assert on places where scalable type doesn't make sense.

[huihuiz]

Take test.ll, run opt -S -instsimplify -o -
Then you will get the compiler warning: Compiler has made implicit assumption that TypeSize is not scalable.

define <vscale x 16 x i8*> @getelementptr_src_ty_not_vscale(<vscale x 16 x i8*> %a, <vscale x 16 x i64> %offset) {
  %v = getelementptr i8, <vscale x 16 x i8*> %a, <vscale x 16 x i64> %offset
  ret <vscale x 16 x i8*> %v
}

For this case SrcTy is i8

[efriedma]

There are two different kinds of "scalable" involved when you're dealing with GEPs: whether you're indexing over a scalable type, and whether the result is a scalable vector of pointers. Mixing the two together into a single boolean doesn't really make sense.

In this context, I'm not sure why it would matter whether the result is a vector of pointers.

[huihuiz]

My initial thought was wrong.
Spent sometime look again, we are nearly impossible to generate incorrect result without the code fixes in this patch.
But comparing a scalable size with a fixed size still doesn't make sense.

E.g., We are comparing scalable type size %offset to a fixed size 'IdxWidth' returned by DataLayout::getIndexSizeInBits

define <vscale x 1 x i8*> @gep(i8* %a, <vscale x 1 x i32> %offset) {
  %v = getelementptr i8, i8* %a, <vscale x 1 x i32> %offset
  ret <vscale x 1 x i8*> %v
}

Please help take a look, let me know what you think?

[efriedma]

llvm/lib/Analysis/InstructionSimplify.cpp

4153

This is supposed to be comparing the size of the scalar types, I think? Scalar types can't be scalable.

That said, it looks like the code isn't actually doing that at the moment.

[huihuiz]

Thanks Eli for the feedback!

Update the patch to address the folds in question, reject if GEP index type is vector type.

[efriedma]

llvm/lib/Analysis/InstructionSimplify.cpp

4143

I think the transform is still valid, it just isn't implemented.

llvm/lib/Analysis/InstructionSimplify.cpp

     // getelementptr P, 0 -> P.
     if (match(Ops[1], m_Zero()) && Ops[0]->getType() == GEPTy)
       return Ops[0];
 
     Type *Ty = SrcTy;
     if (!IsScalableVec && Ty->isSized()) {
       Value *P;
       uint64_t C;
-      uint64_t TyAllocSize = Q.DL.getTypeAllocSize(Ty);
+      uint64_t TyAllocSize = Q.DL.getTypeAllocSize(Ty).getFixedSize();
       // getelementptr P, N -> P if P points to a type of zero size.
       if (TyAllocSize == 0 && Ops[0]->getType() == GEPTy)
         return Ops[0];
 
       // The following transforms are only safe if the ptrtoint cast
       // doesn't truncate the pointers.
       if (Ops[1]->getType()->getScalarSizeInBits() ==
           Q.DL.getPointerSizeInBits(AS)) {

                   m_SDiv(m_Sub(m_Value(P), m_PtrToInt(m_Specific(Ops[0]))),
                          m_SpecificInt(TyAllocSize))))
           if (Value *R = PtrToIntOrZero(P))
             return R;
       }
     }
   }
 
-  if (!IsScalableVec && Q.DL.getTypeAllocSize(LastType) == 1 &&
+  // The following folds are only valid when GEP index types are scalar types.
+  // Bailout early if GEP index type is vector type.
+  if (!IsScalableVec && !isa<VectorType>(Ops.back()->getType()) &&
+      Q.DL.getTypeAllocSize(LastType).getFixedSize() == 1 &&
       all_of(Ops.slice(1).drop_back(1),
              [](Value *Idx) { return match(Idx, m_Zero()); })) {
     unsigned IdxWidth =
         Q.DL.getIndexSizeInBits(Ops[0]->getType()->getPointerAddressSpace());
-    if (Q.DL.getTypeSizeInBits(Ops.back()->getType()) == IdxWidth) {
+    if (Q.DL.getTypeSizeInBits(Ops.back()->getType()).getFixedSize() ==
+        IdxWidth) {
       APInt BasePtrOffset(IdxWidth, 0);
       Value *StrippedBasePtr =
           Ops[0]->stripAndAccumulateInBoundsConstantOffsets(Q.DL,
                                                             BasePtrOffset);
 
       // gep (gep V, C), (sub 0, V) -> C
       if (match(Ops.back(),
                 m_Sub(m_Zero(), m_PtrToInt(m_Specific(StrippedBasePtr))))) {
         auto *CI = ConstantInt::get(GEPTy->getContext(), BasePtrOffset);
         return ConstantExpr::getIntToPtr(CI, GEPTy);

llvm/test/Transforms/InstSimplify/cast.ll

 define i8* @test3(i8* %V) {
 entry:
   %BC = bitcast i8* %V to i8*
   ret i8* %BC
 ; CHECK-LABEL: define i8* @test3(
 ; CHECK: ret i8* %V
 }
 
+; This test checks for fold "gep (gep V, C), (sub 0, V) -> C"
 define i32 @test4() {
 ; CHECK-LABEL: @test4(
   %alloca = alloca i32, align 4                                     ; alloca + 0
   %gep = getelementptr inbounds i32, i32* %alloca, i32 1            ; alloca + 4
   %bc = bitcast i32* %gep to [4 x i8]*                              ; alloca + 4
   %pti = ptrtoint i32* %alloca to i32                               ; alloca
   %sub = sub i32 0, %pti                                            ; -alloca
   %add = getelementptr [4 x i8], [4 x i8]* %bc, i32 0, i32 %sub     ; alloca + 4 - alloca == 4
   %add_to_int = ptrtoint i8* %add to i32                            ; 4
   ret i32 %add_to_int                                               ; 4
 ; CHECK-NEXT: ret i32 4
 }
 
+; This test checks for fold "gep (gep V, C), (xor V, -1) -> C-1"
 define i32 @test5() {
 ; CHECK-LABEL: @test5(
   %alloca = alloca i32, align 4                                     ; alloca + 0
   %gep = getelementptr inbounds i32, i32* %alloca, i32 1            ; alloca + 4
   %bc = bitcast i32* %gep to [4 x i8]*                              ; alloca + 4
   %pti = ptrtoint i32* %alloca to i32                               ; alloca
   %sub = xor i32 %pti, -1                                           ; ~alloca
   %add = getelementptr [4 x i8], [4 x i8]* %bc, i32 0, i32 %sub     ; alloca + 4 - alloca - 1 == 3
-  %add_to_int = ptrtoint i8* %add to i32                            ; 4
-  ret i32 %add_to_int                                               ; 4
+  %add_to_int = ptrtoint i8* %add to i32                            ; 3
+  ret i32 %add_to_int                                               ; 3
 ; CHECK-NEXT: ret i32 3
 }
+
+; Negative tests
+
+; The above folds are only valid when gep index type is scalar type. These negative
+; tests check the folding codepath won't be executed, and assertion won't be triggered
+; that we are not trying to get fixed size on a scalable type.
+
+define <1 x i8*> @gep_index_vector(i8* %a, <1 x i32> %offset) {
+; CHECK-LABEL: @gep_index_vector
+  %v = getelementptr i8, i8* %a, <1 x i32> %offset
+  ret <1 x i8*> %v
+; CHECK: ret <1 x i8*> %v
+}
+
+define <vscale x 1 x i8*> @gep_index_vector_scalable(i8* %a, <vscale x 1 x i32> %offset) {
+; CHECK-LABEL: @gep_index_vector_scalable
+  %v = getelementptr i8, i8* %a, <vscale x 1 x i32> %offset
+  ret <vscale x 1 x i8*> %v
+; CHECK: ret <vscale x 1 x i8*> %v
+}