[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 added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:77
+      // Make sure that moving this block would not cause a WLS in the target BB
+      // to branch backwards
+      bool CanMove = true;
----------------
SjoerdMeijer wrote:
> Are we here actually not analysing a loop nest? I.e., look for t2WhileLoopStart within a t2WhileLoopStart?
> 
> If so, I am not sure that is necessary, because as far as I know that's not something we support for performance reasons? Then, we could perhaps just add an assert on getLoopDepth on the MachineLoop, and just don't need the loop on 79. Maybe I'm overthinking this, but it's just the repetition that makes me wonder. Also, this case doesn't seem to be covered in the tests?
Indeed it is checking for a nested while loop. If that situation won't occur then I don't mind removing it, or replacing it with an assertion.

It is tested by the `backwards_branch_nested` function in the mir test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92385