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


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:128
+      if (CanMove) {
+        // Make sure no LEs become forwards
+        // An example loop structure where the LoopExit can't be moved, since
----------------
Nit: some punctuation required here, and also below.


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:212
+
+  // Fix fall-through to the moved BB from the one that used to be before it
+  if (BBPrevious && BBPrevious->isSuccessor(BB))
----------------
Nit: some punctuation required in this comment and also the ones below.


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:213
+  // Fix fall-through to the moved BB from the one that used to be before it
+  if (BBPrevious && BBPrevious->isSuccessor(BB))
+    FixFallthrough(BBPrevious, BB);
----------------
I guess `BBPrevious` is a nullptr when the WLS is in the Entry block? Is this covered with a test? I guess that will never happen, so we will always fix this fall through, and never skip over which is then not covered by a test.
I am not even sure that makes sense, i.e. the WLS in the entry block, because then we would move the entry block. So, does this need to be an assert that BBprevious is not a nullptr?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92385