[PATCH] D48411: [RISCV] Add support for _interrupt attribute

[Ana Pazos via Phabricator via llvm-commits]

apazos added inline comments.


================
Comment at: lib/Target/RISCV/RISCVCallingConv.td:32
+// Same as CSR_Interrupt, but including all 32-bit FP registers.
+def CSR_X32_F32_Interrupt: CalleeSavedRegs<(add X1,
+    (sequence "X%u", 3, 9),
----------------
asb wrote:
> Would be more accurately called `CSR_XLEN_F32` or `CSR_GPR_F32` as the X registers are a variable-length register class. i.e. we won't need to define `CSR_X64_F32`. Same for `CSR_X32_F64`.
Sure, will change the  name to XLEN.


================
Comment at: lib/Target/RISCV/RISCVFrameLowering.cpp:217
+  // If interrupt is enabled and there are calls in the handler,
+  // unconditionally save all Caller-saved registers and
+  // all FP registers, regardless whether they are used.
----------------
asb wrote:
> callee-saved
This code is forcing the saving of all arguments and temporaries plus all the floating point registers, even if they are not used.
Arguments and temporaries are ra, t0-2, a0-a7, t3-t6 (which are all Caller-saved registers).
So the comment is correct (includes Caller and floating point registers only).

Here is the logic I am enforcing:

- If interrupt is enabled:
---- If no calls
       Save all registers if they are used. That includes callee saved, caller saved, and FP registers.
      When you call getCalleeSavedRegs and getCallPreservedMask, it returns the data for the lists 
     CSR_XLEN_F32  and CSR_XLEN_F64.
     In my last patch I missed getCallPreservedMask. I will update that function.
 ----If has calls
       Force saving all arguments and temporaries (these are the Caller saved registers), even if not used.
       Force saving all FP registers, even if not used.
       RISCVFrameLowering::determineCalleeSaves is the place to force saving these registers.

I checked GCC and it seems to implement this behavior from what I could test.

Let me know if you agree.




https://reviews.llvm.org/D48411