[PATCH] D82061: [IR] Prefer scalar type for struct indexes in GEP constant expressions.

[Eli Friedman via Phabricator via llvm-commits]

efriedma created this revision.
efriedma added reviewers: spatel, ctetreau, sdesmalen, fhahn.
Herald added subscribers: arphaman, hiraditya.
Herald added a project: LLVM.

Using a scalar constant instead of a vector constant has two advantages: one, it's simpler to understand, and two, it doesn't require complex pattern matching to work with scalable vectors.

Also includes a small fix to DataLayout to allow the scalable vector testcase to work correctly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82061

Files:
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/test/Analysis/ConstantFolding/vectorgep-crash.ll
  llvm/test/Transforms/InstSimplify/gep.ll


Index: llvm/test/Transforms/InstSimplify/gep.ll
===================================================================
--- llvm/test/Transforms/InstSimplify/gep.ll
+++ llvm/test/Transforms/InstSimplify/gep.ll
@@ -152,7 +152,7 @@
 %struct = type { double, float }
 define <4 x float*> @vector_idx_mix_scalar_vector() {
 ; CHECK-LABEL: @vector_idx_mix_scalar_vector(
-; CHECK-NEXT:    ret <4 x float*> getelementptr (%struct, <4 x %struct*> zeroinitializer, <4 x i64> zeroinitializer, <4 x i32> <i32 1, i32 1, i32 1, i32 1>)
+; CHECK-NEXT:    ret <4 x float*> getelementptr (%struct, <4 x %struct*> zeroinitializer, <4 x i64> zeroinitializer, i32 1)
 ;
   %gep = getelementptr %struct, <4 x %struct*> zeroinitializer, i32 0, <4 x i32> <i32 1, i32 1, i32 1, i32 1>
   ret <4 x float*> %gep
@@ -168,4 +168,12 @@
   ret <vscale x 4 x i32*> %gep
 }
 
+define <vscale x 4 x float*> @scalable_vector_idx_mix_scalar_vector() {
+; CHECK-LABEL: @scalable_vector_idx_mix_scalar_vector(
+; CHECK-NEXT:    ret <vscale x 4 x float*> getelementptr (%struct, <vscale x 4 x %struct*> zeroinitializer, <vscale x 4 x i64> zeroinitializer, i32 1)
+;
+  %gep = getelementptr %struct, <vscale x 4 x %struct*> zeroinitializer, i32 0, i32 1
+  ret <vscale x 4 x float*> %gep
+}
+
 ; Check ConstantExpr::getGetElementPtr() using ElementCount for size queries - end.
Index: llvm/test/Analysis/ConstantFolding/vectorgep-crash.ll
===================================================================
--- llvm/test/Analysis/ConstantFolding/vectorgep-crash.ll
+++ llvm/test/Analysis/ConstantFolding/vectorgep-crash.ll
@@ -24,7 +24,7 @@
 
 @G = internal global [65 x %struct.A] zeroinitializer, align 16
 ; CHECK-LABEL: @test
-; CHECK: ret <16 x i32*> getelementptr ([65 x %struct.A], [65 x %struct.A]* @G, <16 x i64> zeroinitializer, <16 x i64> <i64 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9, i64 10, i64 11, i64 12, i64 13, i64 14, i64 15, i64 16>, <16 x i32> zeroinitializer)
+; CHECK: ret <16 x i32*> getelementptr ([65 x %struct.A], [65 x %struct.A]* @G, <16 x i64> zeroinitializer, <16 x i64> <i64 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9, i64 10, i64 11, i64 12, i64 13, i64 14, i64 15, i64 16>, i32 0)
 define <16 x i32*> @test() {
 vector.body:
   %VectorGep = getelementptr [65 x %struct.A], [65 x %struct.A]* @G, <16 x i64> zeroinitializer, <16 x i64> <i64 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9, i64 10, i64 11, i64 12, i64 13, i64 14, i64 15, i64 16>, <16 x i32> zeroinitializer
Index: llvm/lib/IR/DataLayout.cpp
===================================================================
--- llvm/lib/IR/DataLayout.cpp
+++ llvm/lib/IR/DataLayout.cpp
@@ -814,7 +814,7 @@
   unsigned NumBits = getIndexTypeSizeInBits(Ty);
   IntegerType *IntTy = IntegerType::get(Ty->getContext(), NumBits);
   if (VectorType *VecTy = dyn_cast<VectorType>(Ty))
-    return FixedVectorType::get(IntTy, VecTy->getNumElements());
+    return VectorType::get(IntTy, VecTy);
   return IntTy;
 }
 
Index: llvm/lib/IR/Constants.cpp
===================================================================
--- llvm/lib/IR/Constants.cpp
+++ llvm/lib/IR/Constants.cpp
@@ -2175,15 +2175,22 @@
   std::vector<Constant*> ArgVec;
   ArgVec.reserve(1 + Idxs.size());
   ArgVec.push_back(C);
-  for (unsigned i = 0, e = Idxs.size(); i != e; ++i) {
+  generic_gep_type_iterator<Value* const*>
+    GTI = gep_type_begin(Ty, Idxs),
+    GTE = gep_type_end(Ty, Idxs);
+  for (; GTI != GTE; ++GTI) {
+    auto *Idx = cast<Constant>(GTI.getOperand());
     assert(
-        (!isa<VectorType>(Idxs[i]->getType()) ||
-         cast<VectorType>(Idxs[i]->getType())->getElementCount() == EltCount) &&
+        (!isa<VectorType>(Idx->getType()) ||
+         cast<VectorType>(Idx->getType())->getElementCount() == EltCount) &&
         "getelementptr index type missmatch");
 
-    Constant *Idx = cast<Constant>(Idxs[i]);
-    if (EltCount.Min != 0 && !Idxs[i]->getType()->isVectorTy())
+    if (GTI.isStruct() && Idx->getType()->isVectorTy()) {
+      Idx = Idx->getSplatValue();
+    } else if (GTI.isSequential() && EltCount.Min != 0 &&
+               !Idx->getType()->isVectorTy()) {
       Idx = ConstantVector::getSplat(EltCount, Idx);
+    }
     ArgVec.push_back(Idx);
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82061.271522.patch
Type: text/x-patch
Size: 4255 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200618/f5962803/attachment-0001.bin>