[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Two nits, that I wanted to submit before the meeting, but didn't get around to.



================
Comment at: llvm/lib/Target/RISCV/RISCV.td:72
 
+def FeatureSaveRestore : SubtargetFeature<"save-restore", "EnableSaveRestore",
+                                          "true", "Enable save/restore.">;
----------------
Given the clang option defaults to false, I think it should here too, to avoid confusion in other frontends.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:278
   // Add CFI directives for callee-saved registers.
-  const std::vector<CalleeSavedInfo> &CSI = MFI.getCalleeSavedInfo();
-  // Iterate over list of callee-saved registers and emit .cfi_restore
-  // directives.
-  for (const auto &Entry : CSI) {
-    Register Reg = Entry.getReg();
-    unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestore(
-        nullptr, RI->getDwarfRegNum(Reg, true)));
-    BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
-        .addCFIIndex(CFIIndex);
+  if (!CSI.empty()) {
+    // Iterate over list of callee-saved registers and emit .cfi_restore
----------------
Nit: You shouldn't need this `if` statement - the for loop just won't execute if CSI is empty, surely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62686