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

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

Looking good in general. I think we would need some perf numbers to see if we haven't missed something.
Some more remarks inlined.



================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:1
+#include "ARM.h"
+#include "ARMBaseInstrInfo.h"
----------------
Think we need the LLVM copyright header here.


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:65
+      bool CanMove = true;
+      for (auto &MI : Target->terminators()) {
+        if (MI.getOpcode() != ARM::t2WhileLoopStart)
----------------
I think we are duplicating work here. I.e., we are visiting a block and terminators that we will visit later or have seen already. It shouldn't be a big deal, but the code is also a bit of a copy-paste of the loop starting at line 53.
Can we not do one scan of the blocks and record a map of WLSTP to blocks or something like that?


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:105
+           "'To' is expected to be a successor of 'From'");
+    MachineInstr &Terminator = *(--From->terminators().end());
+    if (!Terminator.isUnconditionalBranch()) {
----------------
Can there be multiple terminators?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92385