[PATCH] D85598: Do not strip metadata from load of constant memory

[Yichao Yu via Phabricator via llvm-commits]

yuyichao created this revision.
yuyichao added a reviewer: sanjoy.
Herald added subscribers: llvm-commits, asbirlea, hiraditya.
Herald added a project: LLVM.
yuyichao requested review of this revision.

These load should always return the same result. It'll also be nice if there's a way to create a instruction specific list of metadata that is always safe to speculate but this should be good and simple enough for now...

Ref https://github.com/JuliaLang/julia/pull/36970


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85598

Files:
  llvm/lib/Transforms/Scalar/LICM.cpp


Index: llvm/lib/Transforms/Scalar/LICM.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LICM.cpp
+++ llvm/lib/Transforms/Scalar/LICM.cpp
@@ -141,7 +141,7 @@
 static void hoist(Instruction &I, const DominatorTree *DT, const Loop *CurLoop,
                   BasicBlock *Dest, ICFLoopSafetyInfo *SafetyInfo,
                   MemorySSAUpdater *MSSAU, ScalarEvolution *SE,
-                  OptimizationRemarkEmitter *ORE);
+                  OptimizationRemarkEmitter *ORE, AAResults *AA);
 static bool sink(Instruction &I, LoopInfo *LI, DominatorTree *DT,
                  const Loop *CurLoop, ICFLoopSafetyInfo *SafetyInfo,
                  MemorySSAUpdater *MSSAU, OptimizationRemarkEmitter *ORE);
@@ -814,7 +814,7 @@
               I, DT, CurLoop, SafetyInfo, ORE,
               CurLoop->getLoopPreheader()->getTerminator())) {
         hoist(I, DT, CurLoop, CFH.getOrCreateHoistedBlock(BB), SafetyInfo,
-              MSSAU, SE, ORE);
+              MSSAU, SE, ORE, AA);
         HoistedInstructions.push_back(&I);
         Changed = true;
         continue;
@@ -840,7 +840,7 @@
         eraseInstruction(I, *SafetyInfo, CurAST, MSSAU);
 
         hoist(*ReciprocalDivisor, DT, CurLoop, CFH.getOrCreateHoistedBlock(BB),
-              SafetyInfo, MSSAU, SE, ORE);
+              SafetyInfo, MSSAU, SE, ORE, AA);
         HoistedInstructions.push_back(ReciprocalDivisor);
         Changed = true;
         continue;
@@ -859,7 +859,7 @@
           CurLoop->hasLoopInvariantOperands(&I) &&
           MustExecuteWithoutWritesBefore(I)) {
         hoist(I, DT, CurLoop, CFH.getOrCreateHoistedBlock(BB), SafetyInfo,
-              MSSAU, SE, ORE);
+              MSSAU, SE, ORE, AA);
         HoistedInstructions.push_back(&I);
         Changed = true;
         continue;
@@ -873,7 +873,7 @@
             PN->setIncomingBlock(
                 i, CFH.getOrCreateHoistedBlock(PN->getIncomingBlock(i)));
           hoist(*PN, DT, CurLoop, CFH.getOrCreateHoistedBlock(BB), SafetyInfo,
-                MSSAU, SE, ORE);
+                MSSAU, SE, ORE, AA);
           assert(DT->dominates(PN, BB) && "Conditional PHIs not expected");
           Changed = true;
           continue;
@@ -1632,7 +1632,7 @@
 static void hoist(Instruction &I, const DominatorTree *DT, const Loop *CurLoop,
                   BasicBlock *Dest, ICFLoopSafetyInfo *SafetyInfo,
                   MemorySSAUpdater *MSSAU, ScalarEvolution *SE,
-                  OptimizationRemarkEmitter *ORE) {
+                  OptimizationRemarkEmitter *ORE, AAResults *AA) {
   LLVM_DEBUG(dbgs() << "LICM hoisting to " << Dest->getName() << ": " << I
                     << "\n");
   ORE->emit([&]() {
@@ -1644,11 +1644,13 @@
   // Conservatively strip all metadata on the instruction unless we were
   // guaranteed to execute I if we entered the loop, in which case the metadata
   // is valid in the loop preheader.
+  // This is also safe for loads on constant memory since the load result is always the same.
   if (I.hasMetadataOtherThanDebugLoc() &&
       // The check on hasMetadataOtherThanDebugLoc is to prevent us from burning
       // time in isGuaranteedToExecute if we don't actually have anything to
       // drop.  It is a compile time optimization, not required for correctness.
-      !SafetyInfo->isGuaranteedToExecute(I, DT, CurLoop))
+      (!SafetyInfo->isGuaranteedToExecute(I, DT, CurLoop) ||
+       (isa<LoadInst>(I) && AA->pointsToConstantMemory(cast<LoadInst>(I).getOperand(0)))))
     I.dropUnknownNonDebugMetadata();
 
   if (isa<PHINode>(I))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85598.284172.patch
Type: text/x-patch
Size: 3595 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200809/22b066fa/attachment.bin>