nickdesaulniers added a comment. Thanks so much for this patch! I look forward to support for `__gnu_mcount` for the arm32 Linux kernel.
> What a horrible function. AAPCS? Who cares about that? haha ================ Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1944 + + // Replace the pseudo instruction with a call instruction + MachineInstrBuilder MIB = BuildMI(MBB, MBBI, MI.getDebugLoc(), ---------------- Here down seems to match the previous case? Maybe you could "Dont Repeat Yourself (DRY)" up the code by creating a shared function? ================ Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1950 + MIB.add(MO); + } + MI.eraseFromParent(); ---------------- No need for `{}` for single statement blocks. ================ 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); ---------------- How many other call sites would have to be updated if this was not a default parameter? ================ 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, ---------------- efriedma wrote: > 80 cols also, what's up with `\01`? ================ Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:2298 + } + } + ---------------- Ditto on single statement bodies. `{}` ================ Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:5 +define dso_local i32 @foo(i64) local_unnamed_addr #0 { +; CHECK-ARM: stmdb sp!, {lr} +; CHECK-ARM-NOT: stmdb sp!, {lr} ---------------- Don't we want to check that these occur in a parent function before a call to a child function? ================ 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 ---------------- why does the `-NOT` case duplicate the non-`NOT` case? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits