[PATCH] D116754: Bugfix for application of trivial loop optimization in LoopIdiomRecognize(Github Issue #50308)

[Raghu R via Phabricator via llvm-commits]

razetime added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:628-636
   // We can only promote stores in this block if they are unconditionally
-  // executed in the loop.  For a block to be unconditionally executed, it has
-  // to dominate all the exit blocks of the loop.  Verify this now.
-  for (BasicBlock *ExitBlock : ExitBlocks)
-    if (!DT->dominates(BB, ExitBlock))
-      return false;
+  // executed in the loop. For a block to be unconditionally executed, it has
+  // to dominate all the exit blocks of the loop. Verify this now.
+  // On countable unrotated loops, checking the exit blocks fails.
+  // Hence, this is only run on rotated loops.
+  if (isFromRotatedLoop)
+    for (BasicBlock *ExitBlock : ExitBlocks)
----------------
lebedev.ri wrote:
> I do not understand why we can just not do this check for unrotated loops?
The check unconditionally failed on the unrotated test case I was using from https://llvm.godbolt.org/z/YrGqGhnE9.

The place where it failed was when BB =`loop.latch` and ExitBlock = `exit`. pre-rotating the loop would work, but it seemed unintuitive and costlier than skipping the check. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116754