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

Jian Cai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 6 16:39:23 PDT 2019


jcai19 marked an inline comment as done.
jcai19 added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:211
     unsigned ARMMoveToIntReg(MVT VT, unsigned SrcReg);
-    unsigned ARMSelectCallOp(bool UseReg);
+    unsigned ARMSelectCallOp(bool UseReg, bool PushLR = false);
     unsigned ARMLowerPICELF(const GlobalValue *GV, unsigned Align, MVT VT);
----------------
nickdesaulniers wrote:
> How many other call sites would have to be updated if this was not a default parameter?
There should be only 2 references to this function. But this argument is likely to be false in most cases.


================
Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:2417
   // Issue the call.
-  unsigned CallOpc = ARMSelectCallOp(UseReg);
+  unsigned CallOpc = ARMSelectCallOp(UseReg, Callee && Callee->getName() == "\01__gnu_mcount_nc");
   MachineInstrBuilder MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt,
----------------
nickdesaulniers wrote:
> efriedma wrote:
> > 80 cols
> also, what's up with `\01`?
I am not sure why the name is prefixed with \01 either but it was there in the original code.


================
Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:6
+; CHECK-ARM:        stmdb   sp!, {lr}
+; CHECK-ARM-NOT:    stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl      __gnu_mcount_nc
----------------
nickdesaulniers wrote:
> why does the `-NOT` case duplicate the non-`NOT` case?
They are unnecessary, I have removed them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019





More information about the cfe-commits mailing list