[llvm] 2c15b9d - [MergeLoadStoreMotion] Don't require GEP for sinking

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 27 03:19:48 PST 2022


Author: Nikita Popov
Date: 2022-12-27T12:17:58+01:00
New Revision: 2c15b9d9e1a898cfd849db81b36d278eac3ef24e

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

LOG: [MergeLoadStoreMotion] Don't require GEP for sinking

Allow sinking stores where both operands are the same, don't require
them to have an identical GEP in each block.

This came up when migrating tests to opaque pointers, where
zero-index GEPs are omitted.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
    llvm/test/Transforms/MergedLoadStoreMotion/st_sink_no_gep.ll
    llvm/test/Transforms/MergedLoadStoreMotion/st_sink_split_bb.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
index 253a850d9479c..af4ef29fa26a3 100644
--- a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
+++ b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
@@ -220,27 +220,29 @@ PHINode *MergedLoadStoreMotion::getPHIOperand(BasicBlock *BB, StoreInst *S0,
 }
 
 ///
-/// Check if 2 stores can be sunk together with corresponding GEPs
+/// Check if 2 stores can be sunk, optionally together with corresponding GEPs.
 ///
 bool MergedLoadStoreMotion::canSinkStoresAndGEPs(StoreInst *S0,
                                                  StoreInst *S1) const {
-  auto *A0 = dyn_cast<Instruction>(S0->getPointerOperand());
-  auto *A1 = dyn_cast<Instruction>(S1->getPointerOperand());
-  return A0 && A1 && A0->isIdenticalTo(A1) && A0->hasOneUse() &&
-         (A0->getParent() == S0->getParent()) && A1->hasOneUse() &&
-         (A1->getParent() == S1->getParent()) && isa<GetElementPtrInst>(A0);
+  if (S0->getPointerOperand() == S1->getPointerOperand())
+    return true;
+  auto *GEP0 = dyn_cast<GetElementPtrInst>(S0->getPointerOperand());
+  auto *GEP1 = dyn_cast<GetElementPtrInst>(S1->getPointerOperand());
+  return GEP0 && GEP1 && GEP0->isIdenticalTo(GEP1) && GEP0->hasOneUse() &&
+         (GEP0->getParent() == S0->getParent()) && GEP1->hasOneUse() &&
+         (GEP1->getParent() == S1->getParent());
 }
 
 ///
 /// Merge two stores to same address and sink into \p BB
 ///
-/// Also sinks GEP instruction computing the store address
+/// Optionally also sinks GEP instruction computing the store address
 ///
 void MergedLoadStoreMotion::sinkStoresAndGEPs(BasicBlock *BB, StoreInst *S0,
                                               StoreInst *S1) {
+  Value *Ptr0 = S0->getPointerOperand();
+  Value *Ptr1 = S1->getPointerOperand();
   // Only one definition?
-  auto *A0 = dyn_cast<Instruction>(S0->getPointerOperand());
-  auto *A1 = dyn_cast<Instruction>(S1->getPointerOperand());
   LLVM_DEBUG(dbgs() << "Sink Instruction into BB \n"; BB->dump();
              dbgs() << "Instruction Left\n"; S0->dump(); dbgs() << "\n";
              dbgs() << "Instruction Right\n"; S1->dump(); dbgs() << "\n");
@@ -254,23 +256,23 @@ void MergedLoadStoreMotion::sinkStoresAndGEPs(BasicBlock *BB, StoreInst *S0,
 
   // Create the new store to be inserted at the join point.
   StoreInst *SNew = cast<StoreInst>(S0->clone());
-  Instruction *ANew = A0->clone();
   SNew->insertBefore(&*InsertPt);
-  ANew->insertBefore(SNew);
-  ANew->applyMergedLocation(A0->getDebugLoc(), A1->getDebugLoc());
-
-  assert(S0->getParent() == A0->getParent());
-  assert(S1->getParent() == A1->getParent());
-
   // New PHI operand? Use it.
   if (PHINode *NewPN = getPHIOperand(BB, S0, S1))
     SNew->setOperand(0, NewPN);
   S0->eraseFromParent();
   S1->eraseFromParent();
-  A0->replaceAllUsesWith(ANew);
-  A0->eraseFromParent();
-  A1->replaceAllUsesWith(ANew);
-  A1->eraseFromParent();
+
+  if (Ptr0 != Ptr1) {
+    auto *GEP0 = cast<GetElementPtrInst>(Ptr0);
+    auto *GEP1 = cast<GetElementPtrInst>(Ptr1);
+    Instruction *GEPNew = GEP0->clone();
+    GEPNew->insertBefore(SNew);
+    GEPNew->applyMergedLocation(GEP0->getDebugLoc(), GEP1->getDebugLoc());
+    SNew->setOperand(1, GEPNew);
+    GEP0->eraseFromParent();
+    GEP1->eraseFromParent();
+  }
 }
 
 ///

diff  --git a/llvm/test/Transforms/MergedLoadStoreMotion/st_sink_no_gep.ll b/llvm/test/Transforms/MergedLoadStoreMotion/st_sink_no_gep.ll
index 854ba9e549dec..457b4942d8121 100644
--- a/llvm/test/Transforms/MergedLoadStoreMotion/st_sink_no_gep.ll
+++ b/llvm/test/Transforms/MergedLoadStoreMotion/st_sink_no_gep.ll
@@ -7,12 +7,12 @@ define void @no_gep_same_ptr(i1 %c, ptr %p, i32 %x, i32 %y) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
 ; CHECK:       if.then:
-; CHECK-NEXT:    store i32 [[X:%.*]], ptr [[P:%.*]], align 4
 ; CHECK-NEXT:    br label [[IF_END:%.*]]
 ; CHECK:       if.else:
-; CHECK-NEXT:    store i32 [[Y:%.*]], ptr [[P]], align 4
 ; CHECK-NEXT:    br label [[IF_END]]
 ; CHECK:       if.end:
+; CHECK-NEXT:    [[Y_SINK:%.*]] = phi i32 [ [[X:%.*]], [[IF_THEN]] ], [ [[Y:%.*]], [[IF_ELSE]] ]
+; CHECK-NEXT:    store i32 [[Y_SINK]], ptr [[P:%.*]], align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:
@@ -64,12 +64,12 @@ define void @shared_gep(i1 %c, ptr %p, i32 %x, i32 %y) {
 ; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i32, ptr [[P:%.*]], i32 1
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
 ; CHECK:       if.then:
-; CHECK-NEXT:    store i32 [[X:%.*]], ptr [[GEP]], align 4
 ; CHECK-NEXT:    br label [[IF_END:%.*]]
 ; CHECK:       if.else:
-; CHECK-NEXT:    store i32 [[Y:%.*]], ptr [[GEP]], align 4
 ; CHECK-NEXT:    br label [[IF_END]]
 ; CHECK:       if.end:
+; CHECK-NEXT:    [[Y_SINK:%.*]] = phi i32 [ [[X:%.*]], [[IF_THEN]] ], [ [[Y:%.*]], [[IF_ELSE]] ]
+; CHECK-NEXT:    store i32 [[Y_SINK]], ptr [[GEP]], align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:

diff  --git a/llvm/test/Transforms/MergedLoadStoreMotion/st_sink_split_bb.ll b/llvm/test/Transforms/MergedLoadStoreMotion/st_sink_split_bb.ll
index b7a022996f3f1..55656a8ae0f02 100644
--- a/llvm/test/Transforms/MergedLoadStoreMotion/st_sink_split_bb.ll
+++ b/llvm/test/Transforms/MergedLoadStoreMotion/st_sink_split_bb.ll
@@ -48,12 +48,12 @@ define dso_local void @st_sink_split_bb(ptr nocapture %arg, ptr nocapture %arg1,
 ; CHECK-YES:       bb5:
 ; CHECK-YES-NEXT:    br i1 [[ARG3:%.*]], label [[BB6:%.*]], label [[BB7:%.*]]
 ; CHECK-YES:       bb6:
-; CHECK-YES-NEXT:    store i32 2, ptr [[ARG]], align 4
 ; CHECK-YES-NEXT:    br label [[BB9_SINK_SPLIT:%.*]]
 ; CHECK-YES:       bb7:
-; CHECK-YES-NEXT:    store i32 3, ptr [[ARG]], align 4
 ; CHECK-YES-NEXT:    br label [[BB9_SINK_SPLIT]]
 ; CHECK-YES:       bb9.sink.split:
+; CHECK-YES-NEXT:    [[DOTSINK:%.*]] = phi i32 [ 2, [[BB6]] ], [ 3, [[BB7]] ]
+; CHECK-YES-NEXT:    store i32 [[DOTSINK]], ptr [[ARG]], align 4
 ; CHECK-YES-NEXT:    [[TMP0:%.*]] = getelementptr inbounds i32, ptr [[ARG1:%.*]], i64 1
 ; CHECK-YES-NEXT:    store i32 3, ptr [[TMP0]], align 4
 ; CHECK-YES-NEXT:    [[TMP1:%.*]] = getelementptr inbounds i32, ptr [[ARG1]], i64 2


        


More information about the llvm-commits mailing list