[llvm] fdd4c19 - Revert "[InstCombine] Make indexed compare fold opaque ptr compatible"

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 25 15:34:46 PDT 2021


Author: Nikita Popov
Date: 2021-06-26T00:32:59+02:00
New Revision: fdd4c199a1ec68a231ea0086239e7d8287a40fad

URL: https://github.com/llvm/llvm-project/commit/fdd4c199a1ec68a231ea0086239e7d8287a40fad
DIFF: https://github.com/llvm/llvm-project/commit/fdd4c199a1ec68a231ea0086239e7d8287a40fad.diff

LOG: Revert "[InstCombine] Make indexed compare fold opaque ptr compatible"

This reverts commit 5cb20ef8a235c2027489a196bba27630ca21a00b.

Assertion failures with this patch were reported on
https://reviews.llvm.org/rG5cb20ef8a235, revert for now.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
    llvm/test/Transforms/InstCombine/indexed-gep-compares.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 0c1629d49bd9..e141d614851b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -502,7 +502,7 @@ static Value *evaluateGEPOffsetExpression(User *GEP, InstCombinerImpl &IC,
 /// Returns true if we can rewrite Start as a GEP with pointer Base
 /// and some integer offset. The nodes that need to be re-written
 /// for this transformation will be added to Explored.
-static bool canRewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
+static bool canRewriteGEPAsOffset(Value *Start, Value *Base,
                                   const DataLayout &DL,
                                   SetVector<Value *> &Explored) {
   SmallVector<Value *, 16> WorkList(1, Start);
@@ -550,7 +550,7 @@ static bool canRewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
         // the original pointer type. We could handle more cases in the
         // future.
         if (GEP->getNumIndices() != 1 || !GEP->isInBounds() ||
-            GEP->getSourceElementType() != ElemTy)
+            GEP->getType() != Start->getType())
           return false;
 
         if (Explored.count(GEP->getOperand(0)) == 0)
@@ -626,7 +626,7 @@ static void setInsertionPoint(IRBuilder<> &Builder, Value *V,
 
 /// Returns a re-written value of Start as an indexed GEP using Base as a
 /// pointer.
-static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
+static Value *rewriteGEPAsOffset(Value *Start, Value *Base,
                                  const DataLayout &DL,
                                  SetVector<Value *> &Explored) {
   // Perform all the substitutions. This is a bit tricky because we can
@@ -728,7 +728,8 @@ static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
                                                Start->getName() + "to.ptr");
 
     Value *GEP = Builder.CreateInBoundsGEP(
-        ElemTy, NewBase, makeArrayRef(NewInsts[Val]), Val->getName() + ".ptr");
+        Start->getType()->getPointerElementType(), NewBase,
+        makeArrayRef(NewInsts[Val]), Val->getName() + ".ptr");
 
     if (!Val->getType()->isPointerTy()) {
       Value *Cast = Builder.CreatePointerCast(GEP, Val->getType(),
@@ -745,7 +746,7 @@ static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
 /// the input Value as a constant indexed GEP. Returns a pair containing
 /// the GEPs Pointer and Index.
 static std::pair<Value *, Value *>
-getAsConstantIndexedAddress(Type *ElemTy, Value *V, const DataLayout &DL) {
+getAsConstantIndexedAddress(Value *V, const DataLayout &DL) {
   Type *IndexType = IntegerType::get(V->getContext(),
                                      DL.getIndexTypeSizeInBits(V->getType()));
 
@@ -757,7 +758,7 @@ getAsConstantIndexedAddress(Type *ElemTy, Value *V, const DataLayout &DL) {
       if (!GEP->isInBounds())
         break;
       if (GEP->hasAllConstantIndices() && GEP->getNumIndices() == 1 &&
-          GEP->getSourceElementType() == ElemTy) {
+          GEP->getType() == V->getType()) {
         V = GEP->getOperand(0);
         Constant *GEPIndex = static_cast<Constant *>(GEP->getOperand(1));
         Index = ConstantExpr::getAdd(
@@ -796,14 +797,17 @@ static Instruction *transformToIndexedCompare(GEPOperator *GEPLHS, Value *RHS,
   if (!GEPLHS->hasAllConstantIndices())
     return nullptr;
 
-  Type *ElemTy = GEPLHS->getSourceElementType();
+  // Make sure the pointers have the same type.
+  if (GEPLHS->getType() != RHS->getType())
+    return nullptr;
+
   Value *PtrBase, *Index;
-  std::tie(PtrBase, Index) = getAsConstantIndexedAddress(ElemTy, GEPLHS, DL);
+  std::tie(PtrBase, Index) = getAsConstantIndexedAddress(GEPLHS, DL);
 
   // The set of nodes that will take part in this transformation.
   SetVector<Value *> Nodes;
 
-  if (!canRewriteGEPAsOffset(ElemTy, RHS, PtrBase, DL, Nodes))
+  if (!canRewriteGEPAsOffset(RHS, PtrBase, DL, Nodes))
     return nullptr;
 
   // We know we can re-write this as
@@ -812,7 +816,7 @@ static Instruction *transformToIndexedCompare(GEPOperator *GEPLHS, Value *RHS,
   // can't have overflow on either side. We can therefore re-write
   // this as:
   //   OFFSET1 cmp OFFSET2
-  Value *NewRHS = rewriteGEPAsOffset(ElemTy, RHS, PtrBase, DL, Nodes);
+  Value *NewRHS = rewriteGEPAsOffset(RHS, PtrBase, DL, Nodes);
 
   // RewriteGEPAsOffset has replaced RHS and all of its uses with a re-written
   // GEP having PtrBase as the pointer base, and has returned in NewRHS the

diff  --git a/llvm/test/Transforms/InstCombine/indexed-gep-compares.ll b/llvm/test/Transforms/InstCombine/indexed-gep-compares.ll
index 0ec71c3f3095..2ca2a45d5c3f 100644
--- a/llvm/test/Transforms/InstCombine/indexed-gep-compares.ll
+++ b/llvm/test/Transforms/InstCombine/indexed-gep-compares.ll
@@ -29,63 +29,7 @@ bb:
 
 bb2:
   ret i32* %RHS
-}
-
-define ptr @test1_opaque_ptr(ptr %A, i32 %Offset) {
-; CHECK-LABEL: @test1_opaque_ptr(
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br label [[BB:%.*]]
-; CHECK:       bb:
-; CHECK-NEXT:    [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[OFFSET:%.*]], [[ENTRY:%.*]] ]
-; CHECK-NEXT:    [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 1
-; CHECK-NEXT:    [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 100
-; CHECK-NEXT:    br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
-; CHECK:       bb2:
-; CHECK-NEXT:    [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A:%.*]], i32 [[RHS_IDX]]
-; CHECK-NEXT:    ret ptr [[RHS_PTR]]
-;
-entry:
-  %tmp = getelementptr inbounds i32, ptr %A, i32 %Offset
-  br label %bb
-
-bb:
-  %RHS = phi ptr [ %RHS.next, %bb ], [ %tmp, %entry ]
-  %LHS = getelementptr inbounds i32, ptr %A, i32 100
-  %RHS.next = getelementptr inbounds i32, ptr %RHS, i64 1
-  %cond = icmp ult ptr %LHS, %RHS
-  br i1 %cond, label %bb2, label %bb
 
-bb2:
-  ret ptr %RHS
-}
-
-define ptr @test_opaque_ptr_
diff erent_types(ptr %A, i32 %Offset) {
-; CHECK-LABEL: @test_opaque_ptr_
diff erent_types(
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP:%.*]] = getelementptr inbounds i32, ptr [[A:%.*]], i32 [[OFFSET:%.*]]
-; CHECK-NEXT:    br label [[BB:%.*]]
-; CHECK:       bb:
-; CHECK-NEXT:    [[RHS:%.*]] = phi ptr [ [[RHS_NEXT:%.*]], [[BB]] ], [ [[TMP]], [[ENTRY:%.*]] ]
-; CHECK-NEXT:    [[LHS:%.*]] = getelementptr inbounds i64, ptr [[A]], i32 100
-; CHECK-NEXT:    [[RHS_NEXT]] = getelementptr inbounds i32, ptr [[RHS]], i32 1
-; CHECK-NEXT:    [[COND:%.*]] = icmp ult ptr [[LHS]], [[RHS]]
-; CHECK-NEXT:    br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
-; CHECK:       bb2:
-; CHECK-NEXT:    ret ptr [[RHS]]
-;
-entry:
-  %tmp = getelementptr inbounds i32, ptr %A, i32 %Offset
-  br label %bb
-
-bb:
-  %RHS = phi ptr [ %RHS.next, %bb ], [ %tmp, %entry ]
-  %LHS = getelementptr inbounds i64, ptr %A, i32 100
-  %RHS.next = getelementptr inbounds i32, ptr %RHS, i64 1
-  %cond = icmp ult ptr %LHS, %RHS
-  br i1 %cond, label %bb2, label %bb
-
-bb2:
-  ret ptr %RHS
 }
 
 define i32 *@test2(i32 %A, i32 %Offset) {


        


More information about the llvm-commits mailing list