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

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoopBlockPlacement.cpp:49
+        continue;
+      moveBasicBlock(Target, &BB);
+      Changed = true;
----------------
I think moving a block with invalidate your BBUtils info, but I'm hoping adjustBBOffsetsAfter can be used instead of recomputing everthing. We should also have a test which exercises multiple block rearrangements. 


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoopBlockPlacement.cpp:61
+
+bool ARMLowOverheadLoopBlockPlacement::canMoveBasicBlock(
+    MachineBasicBlock *BB, MachineBasicBlock *To) {
----------------
Looks like you're duplicating work here:
- iterating through BB twice to find t2WhileLoopStart
- extra call to blockIsBefore?


================
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
----------------
Keep the code together and have fixFallThrough as a lambda?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92385