[PATCH] D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches.

[Mingming Liu via Phabricator via llvm-commits]

mingmingl updated this revision to Diff 462529.
mingmingl marked 2 inline comments as done.
mingmingl edited the summary of this revision.
mingmingl added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134152/new/

https://reviews.llvm.org/D134152

Files:
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/test/Transforms/SimplifyCFG/preserve-llvm-loop-metadata.ll


Index: llvm/test/Transforms/SimplifyCFG/preserve-llvm-loop-metadata.ll
===================================================================
--- llvm/test/Transforms/SimplifyCFG/preserve-llvm-loop-metadata.ll
+++ llvm/test/Transforms/SimplifyCFG/preserve-llvm-loop-metadata.ll
@@ -46,10 +46,10 @@
   ret void
 }
 
-; The test case is constructed based on the following C++ code,
-; as a simplified test case to show why `llvm.loop.unroll.enable`
-; could be dropped.
+; Test that empty loop latch `while.cond.loopexit` will not be folded into its successor if its
+; predecessor blocks are also loop latches.
 ;
+; The test case is constructed based on the following C++ code.
 ; While the C++ code itself might have the inner-loop unrolled (e.g., with -O3),
 ; the loss of inner-loop unroll metadata is a bug.
 ; Under some optimization pipelines (e.g., FullLoopUnroll pass is skipped in ThinLTO prelink stage),
@@ -111,4 +111,7 @@
 !5 = !{!"llvm.loop.unroll.enable"}
 ; CHECK: !0 = distinct !{!0, !1}
 ; CHECK: !1 = !{!"llvm.loop.distribute.enable", i1 true}
-; CHECK-NOT: !{!"llvm.loop.unroll.enable"}
+; CHECK: !2 = distinct !{!2, !3}
+; CHECK: !3 = !{!"llvm.loop.mustprogress"}
+; CHECK: !4 = distinct !{!4, !3, !5}
+; CHECK: !5 = !{!"llvm.loop.unroll.enable"}
Index: llvm/lib/Transforms/Utils/Local.cpp
===================================================================
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1090,6 +1090,68 @@
     }
   }
 
+  // 'BB' and 'BB->Pred' are loop latches, bail out.
+  //
+  // In this case, we expect 'BB' is the latch for outer-loop and 'BB->Pred' is
+  // the latch for inner-loop (see reason below), so bail out to prerserve
+  // inner-loop metadata rather than eliminating 'BB' and attaching its metadata
+  // to this inner-loop.
+  // - The reason we believe 'BB' and 'BB->Pred' have different inner-most
+  // loops: assuming 'BB' and 'BB->Pred' are from the same inner-most loop L,
+  // then 'BB' is the header and latch of 'L' and thereby 'L' must consist of
+  // one self-looping basic block, which is contradictory with the assumption.
+  //
+  // To illustrate how inner-loop metadata is dropped:
+  //
+  // CFG Before
+  //
+  // BB is while.cond.exit, attached with loop metdata md2.
+  // BB->Pred is for.body, attached with loop metadata md1.
+  //
+  //      entry
+  //        |
+  //        v
+  // ---> while.cond   ------------->  while.end
+  // |       |
+  // |       v
+  // |   while.body
+  // |       |
+  // |       v
+  // |    for.body <---- (md1)
+  // |       |  |______|
+  // |       v
+  // |    while.cond.exit (md2)
+  // |       |
+  // |_______|
+  //
+  // CFG After
+  //
+  // while.cond1 is the merge of while.cond.exit and while.cond above.
+  // for.body is attached with md2, and md1 is dropped.
+  // If LoopSimplify runs later (as a part of loop pass), it could create
+  // dedicated exits for inner-loop (essentially adding `while.cond.exit` basck),
+  // but won't it won't see 'md1' nor restore it for the inner-loop.
+  //
+  //       entry
+  //         |
+  //         v
+  // ---> while.cond1  ------------->  while.end
+  // |       |
+  // |       v
+  // |   while.body
+  // |       |
+  // |       v
+  // |    for.body <---- (md2)
+  // |_______|  |______|
+  if (Instruction *TI = BB->getTerminator())
+    if (MDNode *LoopMD = TI->getMetadata(LLVMContext::MD_loop)) {
+      for (BasicBlock *Pred : predecessors(BB)) {
+        if (Instruction *PredTI = Pred->getTerminator())
+          if (MDNode *PredLoopMD = PredTI->getMetadata(LLVMContext::MD_loop))
+            return false;
+      }
+    }
+
   LLVM_DEBUG(dbgs() << "Killing Trivial BB: \n" << *BB);
 
   SmallVector<DominatorTree::UpdateType, 32> Updates;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D134152.462529.patch
Type: text/x-patch
Size: 3766 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220923/ba9632f8/attachment.bin>