nickdesaulniers 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);
----------------
jcai19 wrote:
> 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.
Then I'd just make it an explicit arg and update the 2 call sites. If there
were many call sites, then the default param would cut down on code churn, but
I don't think 2 call sites is unreasonable to just be explicit about such
arguments.
================
Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:2418
+ bool PushLR = IntrinsicName &&
+ !memcmp(IntrinsicName, "\01__gnu_mcount_nc",
sizeof("\01__gnu_mcount_nc"));
+ unsigned CallOpc = ARMSelectCallOp(UseReg, PushLR);
----------------
`memcmp` is a code smell in a C++ codebase, but I see that `IntrinsicName` is a
C style string. Is there a reason why `strncmp` isn't used?
================
Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:2587
}
+ case Intrinsic::arm_gnu_eabi_mcount: {
+ return SelectCall(&I, "\01__gnu_mcount_nc");
----------------
`{}` are not needed here since you're not introducing a new scope for variables.
================
Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:11
+entry:
+ %cmp = icmp slt i32 %n, 2
+ br i1 %cmp, label %return, label %if.end
----------------
This test case can probably be simplified to just a call and ret void.
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
