[llvm] cb03470 - Reapply [MergeLoadStoreMotion] Don't require GEP for sinking
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 27 03:50:19 PST 2022
Author: Nikita Popov
Date: 2022-12-27T12:49:30+01:00
New Revision: cb03470aefb91dfd0d01f2651d604e9afaef5cb1
URL: https://github.com/llvm/llvm-project/commit/cb03470aefb91dfd0d01f2651d604e9afaef5cb1
DIFF: https://github.com/llvm/llvm-project/commit/cb03470aefb91dfd0d01f2651d604e9afaef5cb1.diff
LOG: Reapply [MergeLoadStoreMotion] Don't require GEP for sinking
Reapply with a fix for a failing debuginfo assignment tracking test.
-----
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..62e75d98448cf 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,25 @@ 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->replaceAllUsesWith(GEPNew);
+ GEP0->eraseFromParent();
+ GEP1->replaceAllUsesWith(GEPNew);
+ 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