[PATCH] D119760: [DSE] Fall back to CFG scan for unreachable terminators.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: nikic, asbirlea, dmgreen.
Herald added a subscriber: hiraditya.
fhahn requested review of this revision.
Herald added a project: LLVM.

Blocks with UnreachableInst terminators are considered as root nodes in
the PDT. This pessimize DSE, if there are no aliasing reads from the
potentially dead store and the block with the unreachable terminator.

If the PDT has a virtual root node and any of the root nodes has
UnreachableInst as terminator, fall back to the CFG scan, even the
common dominator of all killing blocks does not post-dominate the block
with potentially dead store.

It looks like the compile-time impact for the extra scans is negligible.
https://llvm-compile-time-tracker.com/compare.php?from=779bbbf27fe631154bdfaac7a443f198d4654688&to=ac59945f1bec1c6a7d7f5590c8c69fd9c5369c53&stat=instructions

Fixes #53800.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119760

Files:
  llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
  llvm/test/Transforms/DeadStoreElimination/multiblock-unreachable.ll


Index: llvm/test/Transforms/DeadStoreElimination/multiblock-unreachable.ll
===================================================================
--- llvm/test/Transforms/DeadStoreElimination/multiblock-unreachable.ll
+++ llvm/test/Transforms/DeadStoreElimination/multiblock-unreachable.ll
@@ -63,12 +63,11 @@
 define void @unreachable_exit_with_no_call(i64* noalias %ptr, i1 %c.1) {
 ; CHECK-LABEL: @unreachable_exit_with_no_call(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    store i64 1, i64* [[PTR:%.*]], align 8
 ; CHECK-NEXT:    br i1 [[C_1:%.*]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
 ; CHECK:       if.then:
 ; CHECK-NEXT:    unreachable
 ; CHECK:       if.end:
-; CHECK-NEXT:    store i64 0, i64* [[PTR]], align 8
+; CHECK-NEXT:    store i64 0, i64* [[PTR:%.*]], align 8
 ; CHECK-NEXT:    ret void
 ;
 entry:
@@ -87,13 +86,12 @@
 define void @unreachable_exit_with_nounwind_call_pr53800(i64* noalias %ptr, i1 %c.1) {
 ; CHECK-LABEL: @unreachable_exit_with_nounwind_call_pr53800(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    store i64 1, i64* [[PTR:%.*]], align 8
 ; CHECK-NEXT:    br i1 [[C_1:%.*]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
 ; CHECK:       if.then:
 ; CHECK-NEXT:    tail call void @exit() #[[ATTR0:[0-9]+]]
 ; CHECK-NEXT:    unreachable
 ; CHECK:       if.end:
-; CHECK-NEXT:    store i64 0, i64* [[PTR]], align 8
+; CHECK-NEXT:    store i64 0, i64* [[PTR:%.*]], align 8
 ; CHECK-NEXT:    ret void
 ;
 entry:
Index: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -770,6 +770,10 @@
   /// Keep track of instructions (partly) overlapping with killing MemoryDefs per
   /// basic block.
   MapVector<BasicBlock *, InstOverlapIntervalsTy> IOLs;
+  // Check if there are root nodes that are terminated by UnreachableInst.
+  // Those roots pessimize post-dominance queries. If there are such roots,
+  // fall back to CFG scan starting from all non-unreachable roots.
+  bool AnyUnreachableExit;
 
   // Class contains self-reference, make sure it's not copied/moved.
   DSEState(const DSEState &) = delete;
@@ -805,6 +809,11 @@
 
     // Collect whether there is any irreducible control flow in the function.
     ContainsIrreducibleLoops = mayContainIrreducibleControl(F, &LI);
+
+    AnyUnreachableExit = !PDT.getRootNode()->getBlock() &&
+                         any_of(PDT.roots(), [](const BasicBlock *E) {
+                           return isa<UnreachableInst>(E->getTerminator());
+                         });
   }
 
   /// Return 'OW_Complete' if a store to the 'KillingLoc' location (by \p
@@ -1513,22 +1522,32 @@
       if (KillingBlocks.count(CommonPred)) {
         if (PDT.dominates(CommonPred, MaybeDeadAccess->getBlock()))
           return {MaybeDeadAccess};
-        return None;
+
+        if (!AnyUnreachableExit)
+          return None;
       }
 
       // If the common post-dominator does not post-dominate MaybeDeadAccess,
       // there is a path from MaybeDeadAccess to an exit not going through a
       // killing block.
-      if (PDT.dominates(CommonPred, MaybeDeadAccess->getBlock())) {
-        SetVector<BasicBlock *> WorkList;
+      if (AnyUnreachableExit ||
+          PDT.dominates(CommonPred, MaybeDeadAccess->getBlock())) {
+        if (AnyUnreachableExit &&
+            !PDT.dominates(CommonPred, MaybeDeadAccess->getBlock())) {
+          // Fall back to CFG scan starting at all non-unreachable roots.
+          CommonPred = nullptr;
+        }
 
+        SetVector<BasicBlock *> WorkList;
         // If CommonPred is null, there are multiple exits from the function.
         // They all have to be added to the worklist.
         if (CommonPred)
           WorkList.insert(CommonPred);
         else
-          for (BasicBlock *R : PDT.roots())
-            WorkList.insert(R);
+          for (BasicBlock *R : PDT.roots()) {
+            if (!isa<UnreachableInst>(R->getTerminator()))
+              WorkList.insert(R);
+          }
 
         NumCFGTries++;
         // Check if all paths starting from an exit node go through one of the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D119760.408544.patch
Type: text/x-patch
Size: 4199 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220214/57673a89/attachment.bin>