luismarques added a comment.
The priority for this patch is to address the issues reported by @apazos but
after that please check the clang-format output. There are some cases in this
patch where it might make sense to use a different formatting than clang-format
indicates, but the remaining should be addressed.
@apazos Have you considered tweaking the patch code to not do a tail call, just
to check if that's what's causing the remaining failures? I'm not sure if
that's too hard, but it could eventually be easier than drilling into the
failing cases.
================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:27
+// registers.
+static int getLibCallID(const MachineFunction &MF,
+ const std::vector<CalleeSavedInfo> &CSI) {
----------------
The return value isn't used as just an opaque index, it also reflects the frame
size and is used for that purpose. The function comment should probably reflect
that.
================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:34
+
+ unsigned MaxReg = 0;
+ for (auto &CS : CSI)
----------------
Use `Register` and `RISCV::NoRegister`. (You'll have to use `MaxReg.id()`
instead in the call to `max`).
================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:36
+ for (auto &CS : CSI)
+ if (CS.getFrameIdx() < 0)
+ MaxReg = std::max(MaxReg, CS.getReg());
----------------
Might be worth adding a small comment explaining how this serves as a filters
for the registers we are interested in. Or point to a later relevant comment?
================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:39
+
+ if (MaxReg == 0)
+ return -1;
----------------
Ditto `NoRegister`.
================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:66
+ const std::vector<CalleeSavedInfo> &CSI) {
+ static const char *const spillLibCalls[] = {
+ "__riscv_save_0",
----------------
Check LLVM naming convention capitalization. Ditto other vars here.
================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:93
+ const std::vector<CalleeSavedInfo> &CSI) {
+ static const char *const restoreLibCalls[] = {
+ "__riscv_restore_0",
----------------
Check LLVM naming convention capitalization. Ditto other vars here.
================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:190
+static std::vector<CalleeSavedInfo>
+getNonLibcallCSI(const std::vector<CalleeSavedInfo> &CSI) {
----------------
This could probably use `SmallVector`.
================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:706
+ for (auto &CS : reverse(NonLibcallCSI)) {
+ unsigned Reg = CS.getReg();
+ const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
----------------
Ditto `Register`.
================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:95
+// by save/restore libcalls.
+static const std::map<unsigned, int> FixedCSRFIMap = {
+ {/*ra*/ RISCV::X1, -1},
----------------
Use `IndexedMap` instead?
================
Comment at: llvm/test/CodeGen/RISCV/saverestore.ll:348
+
+; Check that functions with varargs do not use save/restore code
+
----------------
Maybe for these tests just put a -NOT check that __riscv_save_ isn't called?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62686/new/
https://reviews.llvm.org/D62686
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
