[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.

diff --git a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
index 253a850..af4ef29 100644 (file)
--- 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 854ba9e..457b494 100644 (file)
--- 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 b7a0229..55656a8 100644 (file)
--- 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