[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

Thanks for this!

I realize I'm not a reviewer, but have a few nits in passing anyway. :)

I don't plan to stamp this because I've zero familiarity here.



================
Comment at: llvm/lib/Target/ARM/CustomCallLoweringPass.cpp:21
+
+  virtual bool runOnMachineFunction(MachineFunction &MF) {
+
----------------
nit: when overriding a virtual function,

`bool runOnMachineFunction(MachineFunction &MF) override {`

is preferred over

`virtual bool runOnMachineFunction(MachineFunction &MF) {`


================
Comment at: llvm/lib/Target/ARM/CustomCallLoweringPass.cpp:25
+    LLVM_DEBUG(dbgs() << MF.getName() << "\n");
+    for (MachineFunction::iterator FI = MF.begin(), FE = MF.end(); FI != FE; ++FI) {
+      for (MachineBasicBlock::iterator BBI = FI->begin(), BBE = FI->end(); BBI != BBE; ++BBI) {
----------------
nit: can this be iterated over via a range-for loop?

```
for (MachineBasicBlock &MBB : MF) {
  for (MachineInstruction &MI : MBB) {
    // ...
  }
}
```


================
Comment at: llvm/lib/Target/ARM/CustomCallLoweringPass.cpp:29
+          LLVM_DEBUG(dbgs() << *BBI << "\n");
+          for (MachineInstr::const_mop_iterator II = BBI->operands_begin(), IE = BBI->operands_end(); II != IE; ++II )
+            if (II->isGlobal() && II->getGlobal()->getName() == "\01__gnu_mcount_nc") {
----------------
nit: similar range-for question over something like `BBI->operands()`?


================
Comment at: llvm/lib/Target/ARM/CustomCallLoweringPass.cpp:35
+              if (ST.isThumb()) {
+              BuildMI(*FI, BBI, DL, TII.get(ARM::tPUSH))
+                  .add(predOps(ARMCC::AL))
----------------
please clang-format all patches


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019