[PATCH] D123473: [LICM] Only create load in pre-header when promoting load.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: nikic, nlopes, reames.
Herald added subscribers: asbirlea, hiraditya.
Herald added a project: All.
fhahn requested review of this revision.
Herald added a project: LLVM.

When only a store is sunk, there is no need to create a load in the
pre-header, as the result of the load will never get used.

The dead load can can introduce UB, if the function is marked as
writeonly.

Fixes #51248.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123473

Files:
  llvm/lib/Transforms/Scalar/LICM.cpp
  llvm/test/Transforms/LICM/scalar-promote.ll


Index: llvm/test/Transforms/LICM/scalar-promote.ll
===================================================================
--- llvm/test/Transforms/LICM/scalar-promote.ll
+++ llvm/test/Transforms/LICM/scalar-promote.ll
@@ -607,10 +607,9 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[DIV:%.*]] = sdiv i8 [[VAR:%.*]], 3
 ; CHECK-NEXT:    [[CMP2:%.*]] = icmp slt i8 [[DIV]], 0
-; CHECK-NEXT:    [[GLB_PROMOTED:%.*]] = load i8, i8* @glb, align 1
 ; CHECK-NEXT:    br label [[LOOP_HEADER:%.*]]
 ; CHECK:       loop.header:
-; CHECK-NEXT:    [[MERGE1:%.*]] = phi i8 [ [[GLB_PROMOTED]], [[ENTRY:%.*]] ], [ [[MERGE:%.*]], [[LOOP_LATCH:%.*]] ]
+; CHECK-NEXT:    [[MERGE1:%.*]] = phi i8 [ poison, [[ENTRY:%.*]] ], [ [[MERGE:%.*]], [[LOOP_LATCH:%.*]] ]
 ; CHECK-NEXT:    [[I:%.*]] = phi i64 [ 0, [[ENTRY]] ], [ [[ADD:%.*]], [[LOOP_LATCH]] ]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i64 [[I]], 4
 ; CHECK-NEXT:    br i1 [[CMP]], label [[LOOP_BODY:%.*]], label [[FOR_END:%.*]]
Index: llvm/lib/Transforms/Scalar/LICM.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LICM.cpp
+++ llvm/lib/Transforms/Scalar/LICM.cpp
@@ -2156,21 +2156,25 @@
 
   // Set up the preheader to have a definition of the value.  It is the live-out
   // value from the preheader that uses in the loop will use.
-  LoadInst *PreheaderLoad = new LoadInst(
-      AccessTy, SomePtr, SomePtr->getName() + ".promoted",
-      Preheader->getTerminator());
-  if (SawUnorderedAtomic)
-    PreheaderLoad->setOrdering(AtomicOrdering::Unordered);
-  PreheaderLoad->setAlignment(Alignment);
-  PreheaderLoad->setDebugLoc(DebugLoc());
-  if (AATags)
-    PreheaderLoad->setAAMetadata(AATags);
-  SSA.AddAvailableValue(Preheader, PreheaderLoad);
-
-  MemoryAccess *PreheaderLoadMemoryAccess = MSSAU.createMemoryAccessInBB(
-      PreheaderLoad, nullptr, PreheaderLoad->getParent(), MemorySSA::End);
-  MemoryUse *NewMemUse = cast<MemoryUse>(PreheaderLoadMemoryAccess);
-  MSSAU.insertUse(NewMemUse, /*RenameUses=*/true);
+  Value *InitialValue = PoisonValue::get(AccessTy);
+  if (FoundLoadToPromote) {
+    LoadInst *PreheaderLoad =
+        new LoadInst(AccessTy, SomePtr, SomePtr->getName() + ".promoted",
+                     Preheader->getTerminator());
+    if (SawUnorderedAtomic)
+      PreheaderLoad->setOrdering(AtomicOrdering::Unordered);
+    PreheaderLoad->setAlignment(Alignment);
+    PreheaderLoad->setDebugLoc(DebugLoc());
+    if (AATags)
+      PreheaderLoad->setAAMetadata(AATags);
+
+    MemoryAccess *PreheaderLoadMemoryAccess = MSSAU.createMemoryAccessInBB(
+        PreheaderLoad, nullptr, PreheaderLoad->getParent(), MemorySSA::End);
+    MemoryUse *NewMemUse = cast<MemoryUse>(PreheaderLoadMemoryAccess);
+    MSSAU.insertUse(NewMemUse, /*RenameUses=*/true);
+    InitialValue = PreheaderLoad;
+  }
+  SSA.AddAvailableValue(Preheader, InitialValue);
 
   if (VerifyMemorySSA)
     MSSAU.getMemorySSA()->verifyMemorySSA();
@@ -2181,8 +2185,8 @@
   if (VerifyMemorySSA)
     MSSAU.getMemorySSA()->verifyMemorySSA();
   // If the SSAUpdater didn't use the load in the preheader, just zap it now.
-  if (PreheaderLoad->use_empty())
-    eraseInstruction(*PreheaderLoad, *SafetyInfo, MSSAU);
+  if (isa<LoadInst>(InitialValue) && InitialValue->use_empty())
+    eraseInstruction(*cast<Instruction>(InitialValue), *SafetyInfo, MSSAU);
 
   return true;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D123473.421812.patch
Type: text/x-patch
Size: 3375 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220410/b0399ce0/attachment.bin>