[PATCH] D81225: Correctly report modified status for LoopDistribute

serge via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 8 08:48:24 PDT 2020


serge-sans-paille updated this revision to Diff 269245.
serge-sans-paille added a comment.

Move code that unconditionally modifiy the loop *after* the structural check, to avoid returning false while some changes happened.

It passes validation, and I did a quick check, the change looks fine, but I'd be happy to take an extra review from someone more familiar with the codebase.


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

https://reviews.llvm.org/D81225

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


Index: llvm/lib/Transforms/Scalar/LoopDistribute.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopDistribute.cpp
+++ llvm/lib/Transforms/Scalar/LoopDistribute.cpp
@@ -789,12 +789,6 @@
     // instructions to partitions.
     Partitions.setupPartitionIdOnInstructions();
 
-    // To keep things simple have an empty preheader before we version or clone
-    // the loop.  (Also split if this has no predecessor, i.e. entry, because we
-    // rely on PH having a predecessor.)
-    if (!PH->getSinglePredecessor() || &*PH->begin() != PH->getTerminator())
-      SplitBlock(PH, PH->getTerminator(), DT, LI);
-
     // If we need run-time checks, version the loop now.
     auto PtrToPartition = Partitions.computePartitionSetForPointers(*LAI);
     const auto *RtPtrChecking = LAI->getRuntimePointerChecking();
@@ -807,6 +801,13 @@
                   "may not insert runtime check with convergent operation");
     }
 
+    // To keep things simple have an empty preheader before we version or clone
+    // the loop.  (Also split if this has no predecessor, i.e. entry, because we
+    // rely on PH having a predecessor.)
+    if (!PH->getSinglePredecessor() || &*PH->begin() != PH->getTerminator()) {
+      SplitBlock(PH, PH->getTerminator(), DT, LI);
+    }
+
     if (!Pred.isAlwaysTrue() || !Checks.empty()) {
       assert(!LAI->hasConvergentOp() && "inserting illegal loop versioning");
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81225.269245.patch
Type: text/x-patch
Size: 1455 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200608/3a39317f/attachment.bin>


More information about the llvm-commits mailing list