[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/ARMLowOverheadLoopBlockPlacement.cpp:61
+
+bool ARMLowOverheadLoopBlockPlacement::canMoveBasicBlock(
+    MachineBasicBlock *BB, MachineBasicBlock *To) {
----------------
samparker wrote:
> Looks like you're duplicating work here:
> - iterating through BB twice to find t2WhileLoopStart
> - extra call to blockIsBefore?
> Looks like you're duplicating work here:
> - iterating through BB twice to find t2WhileLoopStart
Reckon I should be caching where I find a t2WhileLoopStart to avoid looping where possible? I assume you're referring to the single loop in blockIsBefore and the single loop in runOnMachineFunction.
> - extra call to blockIsBefore?
The reason why there are two calls to blockIsBefore is because we don't care if moving the block results in a backwards branch if it is a backwards branch even without the move.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoopBlockPlacement.cpp:88
+  if (BBPrevious && BBPrevious->isSuccessor(BB))
+    fixFallthrough(BBPrevious, BB);
+  // Fix fall through from the destination BB to the one that used to follow
----------------
samparker wrote:
> Keep the code together and have fixFallThrough as a lambda?
Sounds good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92385