[PATCH] D99649: [ARM] Updates to arm-block-placement pass

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D99649#2663558 <https://reviews.llvm.org/D99649#2663558>, @malharJ wrote:

> addressed comments,
> renamed some functions,
> added comments to test (and updated) 
> updated some incorrect code:
>
> - adjustBBOffsetsAfter() is called with BBPrevious as input since BB is moved, which would cause change in offsets after it.
> - code checking for LE to loopExit now starts search from the MBB after loopExit. Updated the test accordingly.

This seems to have grown from a patch that already fixing 2 things, to a patch that does many more. Can we try and keep it simple, fixing only the problems it was fixing and separate any other issues to another patch?



================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:80
+    WlsInstr = findWLSInBlock(Predecessor);
+    if (!WlsInstr && (Predecessor->pred_size() == 1))
+      WlsInstr = findWLSInBlock(*Predecessor->pred_begin());
----------------
This doesn't need brackets around `(Predecessor->pred_size() == 1)`

I would also change this code to be something more like:
```
  MachineBasicBlock *Predecessor = ML->getLoopPreheader();
  if (!Predecessor)
    return nullptr;
  MachineInstr *WlsInstr = findWLSInBlock(Predecessor);
  if (WlsInstr)
    return WlsInstr;
  if (Predecessor->pred_size() != 1)
    return nullptr;
  return findWLSInBlock(*Predecessor->pred_begin());
```

LLVM prefers those early exits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99649