[PATCH] D52294: [InstCombine] Fix incongruous GEP type addrspace

[Ewan Crawford via Phabricator via llvm-commits]

EwanCrawford updated this revision to Diff 168459.
EwanCrawford edited the summary of this revision.
EwanCrawford added a comment.

done, thanks again


Repository:
  rL LLVM

https://reviews.llvm.org/D52294

Files:
  lib/IR/Verifier.cpp
  lib/Transforms/InstCombine/InstructionCombining.cpp
  test/Transforms/InstCombine/gep-vector.ll


Index: test/Transforms/InstCombine/gep-vector.ll
===================================================================
--- test/Transforms/InstCombine/gep-vector.ll
+++ test/Transforms/InstCombine/gep-vector.ll
@@ -47,3 +47,26 @@
   ret i32* %gep
 }
 
+define i32 addrspace(3)* @bitcast_vec_to_array_addrspace(<7 x i32>* %x, i64 %y, i64 %z) {
+; CHECK-LABEL: @bitcast_vec_to_array_addrspace(
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr <7 x i32>, <7 x i32>* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = addrspacecast i32* [[GEP]] to i32 addrspace(3)*
+; CHECK-NEXT:    ret i32 addrspace(3)* [[TMP1]]
+;
+  %arr_ptr = bitcast <7 x i32>* %x to [7 x i32]*
+  %asc = addrspacecast [7 x i32]* %arr_ptr to [7 x i32] addrspace(3)*
+  %gep = getelementptr [7 x i32], [7 x i32] addrspace(3)* %asc, i64 %y, i64 %z
+  ret i32 addrspace(3)* %gep
+}
+
+define i32 addrspace(3)* @inbounds_bitcast_vec_to_array_addrspace(<7 x i32>* %x, i64 %y, i64 %z) {
+; CHECK-LABEL: @inbounds_bitcast_vec_to_array_addrspace(
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds <7 x i32>, <7 x i32>* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = addrspacecast i32* [[GEP]] to i32 addrspace(3)*
+; CHECK-NEXT:    ret i32 addrspace(3)* [[TMP1]]
+;
+  %arr_ptr = bitcast <7 x i32>* %x to [7 x i32]*
+  %asc = addrspacecast [7 x i32]* %arr_ptr to [7 x i32] addrspace(3)*
+  %gep = getelementptr inbounds [7 x i32], [7 x i32] addrspace(3)* %asc, i64 %y, i64 %z
+  ret i32 addrspace(3)* %gep
+}
Index: lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- lib/Transforms/InstCombine/InstructionCombining.cpp
+++ lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2052,9 +2052,22 @@
           areMatchingArrayAndVecTypes(GEPEltType, SrcEltType)) ||
          (GEPEltType->isVectorTy() && SrcEltType->isArrayTy() &&
           areMatchingArrayAndVecTypes(SrcEltType, GEPEltType)))) {
-      GEP.setOperand(0, SrcOp);
-      GEP.setSourceElementType(SrcEltType);
-      return &GEP;
+
+      // Create a new GEP here, as using `setOperand()` followed by
+      // `setSourceElementType()` won't actually update the type of the
+      // existing GEP Value. Causing issues if this Value is accessed when
+      // constructing an AddrSpaceCastInst
+      Value *NGEP =
+          GEP.isInBounds()
+              ? Builder.CreateInBoundsGEP(nullptr, SrcOp, {Ops[1], Ops[2]})
+              : Builder.CreateGEP(nullptr, SrcOp, {Ops[1], Ops[2]});
+      NGEP->takeName(&GEP);
+
+      // Preserve GEP address space to satisfy users
+      if (NGEP->getType()->getPointerAddressSpace() != GEP.getAddressSpace())
+        return new AddrSpaceCastInst(NGEP, GEPType);
+
+      return replaceInstUsesWith(GEP, NGEP);
     }
 
     // See if we can simplify:
Index: lib/IR/Verifier.cpp
===================================================================
--- lib/IR/Verifier.cpp
+++ lib/IR/Verifier.cpp
@@ -3138,6 +3138,12 @@
              "All GEP indices should be of integer type");
     }
   }
+
+  if (auto *PTy = dyn_cast<PointerType>(GEP.getType())) {
+    Assert(GEP.getAddressSpace() == PTy->getAddressSpace(),
+           "GEP address space doesn't match type", &GEP);
+  }
+
   visitInstruction(GEP);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52294.168459.patch
Type: text/x-patch
Size: 3303 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181005/e3d31752/attachment.bin>