[ConstantFolding] Don't try to cast vectors to pointer if they have different size

[davide]

Summary

An attempt to fix the bug reported by Michael on D26014

[filcab]

Using undef in tests can hide some things.
Can you tell me what you get with your test case for this?

%struct.foo = type { [1 x i64] }

define <4 x i64*> @patatino3(%struct.foo*) {
  %2 = getelementptr inbounds %struct.foo, %struct.foo* %0, i32 0,
                               i32 0, <4 x i8> zeroinitializer
  ret <4 x i64*> %2
}

clang 3.9 gives me this:

define <4 x i64*> @patatino3(%struct.foo* readnone) local_unnamed_addr #0 {
  %2 = getelementptr inbounds %struct.foo, %struct.foo* %0, i64 0, i32 0, <4 x i64> zeroinitializer
  ret <4 x i64*> %2
}

Where the first i32 and the i8 vector have been cast to i64 and i64 vector.

lib/Analysis/ConstantFolding.cpp

731

Did you mean Ops[i]->getType()->isVectorTy()?

From DataLayout.h:

/// \brief Returns an integer type with size at least as big as that of a
/// pointer in the given address space.
IntegerType *getIntPtrType(LLVMContext &C, unsigned AddressSpace = 0) const;

So IntPtrTy will never be a vector type.
It seems with your initial patch, we're not casting at all. But I might be misunderstanding something here.

[davide]

lib/Analysis/ConstantFolding.cpp

731

There's a different overload which can actually return a VectorTy.

/// \brief Returns an integer (vector of integer) type with size at least as
/// big as that of a pointer of the given pointer (vector of pointer) type.
Type *getIntPtrType(Type *) const;

I also double checked and in my example IntPtrTy is a VectorTy

[davide]

In D26060#582443, @filcab wrote:

Using undef in tests can hide some things.
Can you tell me what you get with your test case for this?

%struct.foo = type { [1 x i64] }

define <4 x i64*> @patatino3(%struct.foo*) {
  %2 = getelementptr inbounds %struct.foo, %struct.foo* %0, i32 0,
                               i32 0, <4 x i8> zeroinitializer
  ret <4 x i64*> %2
}

clang 3.9 gives me this:

define <4 x i64*> @patatino3(%struct.foo* readnone) local_unnamed_addr #0 {
  %2 = getelementptr inbounds %struct.foo, %struct.foo* %0, i64 0, i32 0, <4 x i64> zeroinitializer
  ret <4 x i64*> %2
}

Where the first i32 and the i8 vector have been cast to i64 and i64 vector.

This is what I get

[davide@localhost bin]$ ./opt -instcombine filcab.ll -o - -S
; ModuleID = 'filcab.ll'
source_filename = "filcab.ll"

%struct.foo = type { [1 x i64] }

define <4 x i64*> @patatino3(%struct.foo*) {
  %2 = getelementptr inbounds %struct.foo, %struct.foo* %0, i64 0, i32 0, <4 x i64> zeroinitializer
  ret <4 x i64*> %2
}
[davide@localhost bin]$ cat filcab.ll
%struct.foo = type { [1 x i64] }

define <4 x i64*> @patatino3(%struct.foo*) {
  %2 = getelementptr inbounds %struct.foo, %struct.foo* %0, i32 0,
                               i32 0, <4 x i8> zeroinitializer
  ret <4 x i64*> %2
}

which I think is what is expected, but another pair of eyes wouldn't hurt

[davide]

On a second thought, the check for the size should be enough in this case.

[RKSimon]

What is happening with this?

[davide]

A similar fix is landing (https://reviews.llvm.org/D27389).

lib/Analysis/ConstantFolding.cpp

                          Type *ResultTy, const DataLayout &DL,
                          const TargetLibraryInfo *TLI) {
   Type *IntPtrTy = DL.getIntPtrType(ResultTy);
 
   bool Any = false;
   SmallVector<Constant*, 32> NewIdxs;
   for (unsigned i = 1, e = Ops.size(); i != e; ++i) {
     if ((i == 1 ||
-         !isa<StructType>(GetElementPtrInst::getIndexedType(SrcElemTy,
-             Ops.slice(1, i - 1)))) &&
-        Ops[i]->getType() != IntPtrTy) {
+         !isa<StructType>(GetElementPtrInst::getIndexedType(
+             SrcElemTy, Ops.slice(1, i - 1)))) &&
+        Ops[i]->getType() != IntPtrTy &&
+        Ops[i]->getType()->getPrimitiveSizeInBits() ==
+            IntPtrTy->getPrimitiveSizeInBits()) {
       Any = true;
       NewIdxs.push_back(ConstantExpr::getCast(CastInst::getCastOpcode(Ops[i],
                                                                       true,
                                                                       IntPtrTy,
                                                                       true),
                                               Ops[i], IntPtrTy));
     } else
       NewIdxs.push_back(Ops[i]);

test/Transforms/InstCombine/gep-vector.ll

 ; RUN: opt -instcombine %s -S | FileCheck %s
 
 ; CHECK-LABEL: patatino
 ; CHECK-NEXT: ret <8 x i64*> undef
 define <8 x i64*> @patatino() {
   %el = getelementptr i64, <8 x i64*> undef, <8 x i64> undef
   ret <8 x i64*> %el
 }
 
 ; CHECK-LABEL: patatino2
 ; CHECK-NEXT: ret <8 x i64*> undef
 define <8 x i64*> @patatino2() {
   %el = getelementptr inbounds i64, i64* undef, <8 x i64> undef
   ret <8 x i64*> %el
 }
+
+%struct.foo = type { [0 x i64] }
+
+; CHECK-LABEL: patatino3
+; CHECK-NEXT: ret <8 x i64*> undef
+define <8 x i64*> @patatino3(%struct.foo*) {
+  %2 = getelementptr inbounds %struct.foo, %struct.foo* undef, i64 0,
+                               i32 0, <8 x i64> undef
+  ret <8 x i64*> %2
+}