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

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:194
+  bool Changed = false;
+  for (auto *InnerML : *ML) {
+    Changed |= processPostOrderLoops(InnerML);
----------------
Nit: We don't need brackets around a single statement.


================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:95
+  MachineInstr *wlsInstr = nullptr;
+  for (auto &Terminator : Predecessor->terminators()) {
+    if (Terminator.getOpcode() == ARM::t2WhileLoopStartLR)
----------------
malharJ wrote:
> dmgreen wrote:
> > It still probably doesn't need to search for it. If we've found it once, we can just return the WLS instruction.
> > 
> > Also LLVM capitalizes variable names.
> yeah, I thought I'd keep the interface a little clean by not returning a pair.
> I've made the update now though.
Maybe just return the MachineInstr and use Predecessor = WlsInstr->getParent()? I agree not using the pair is a little cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99649