[PATCH] D92385: [ARM] Add a pass that re-arranges blocks when there is a backwards WLS branch

[Sam Tebbs via Phabricator via llvm-commits]

samtebbs marked 3 inline comments as done.
samtebbs added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:123
+          CanMove = false;
+          break;
+        }
----------------
SjoerdMeijer wrote:
> I am a bit struggling with reading the indentation, but was wondering if we could `return false` here? In that case, do we need `CanMove`?
I don't think we can return as that would stop looking at other loops in the function.


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:130
+        // An example loop structure where the LoopExit can't be moved, since
+        // bb2's LE will become forwards once bb1 is moved after bb3 bb1: -
+        // LoopExit bb2:
----------------
SjoerdMeijer wrote:
> nit: newline after `bb3` and LoopExit? I guess you want to have: 
> 
>   // bb1:  -LoopExit
>   // bb2:
>   //   LE bb1 - Terminator
>   // bb3:
>   // ...
Ah yes. I blame clang-format.


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:136
+        // bb4:          - Header
+        for (auto It = LoopExit->getIterator(); It != Preheader->getIterator();
+             It++) {
----------------
SjoerdMeijer wrote:
> Have we checked that LoopExit < Preheader?
Yep, on line 85.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92385