Skip to content

[RISCV] Save vector registers in interrupt handler. #143808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
topperc merged 2 commits into from
Jun 19, 2025
Merged

[RISCV] Save vector registers in interrupt handler. #143808

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0af8996
[RISCV] Save vector registers in interrupt handler.
topperc Jun 11, 2025
48cd23e
fixup! Fix RVE register lists. Simplify F32_V and F64_V lists.
topperc Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions llvm/lib/Target/RISCV/RISCVCallingConv.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,44 @@ def CSR_XLEN_F32_Interrupt: CalleeSavedRegs<(add CSR_Interrupt,
def CSR_XLEN_F64_Interrupt: CalleeSavedRegs<(add CSR_Interrupt,
(sequence "F%u_D", 0, 31))>;

// Same as CSR_Interrupt, but including all vector registers.
def CSR_XLEN_V_Interrupt: CalleeSavedRegs<(add CSR_Interrupt,
(sequence "V%u", 0, 31))>;

// Same as CSR_Interrupt, but including all 32-bit FP registers and all vector
// registers.
def CSR_XLEN_F32_V_Interrupt: CalleeSavedRegs<(add CSR_Interrupt,
(sequence "F%u_F", 0, 31),
(sequence "V%u", 0, 31))>;

// Same as CSR_Interrupt, but including all 64-bit FP registers and all vector
// registers.
def CSR_XLEN_F64_V_Interrupt: CalleeSavedRegs<(add CSR_Interrupt,
(sequence "F%u_D", 0, 31),
(sequence "V%u", 0, 31))>;
Copy link
Contributor

@wangpc-pp wangpc-pp Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use LMUL8 registers?

Copy link
Collaborator Author

@topperc topperc Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried, but it caused us to spill an LMUL=8 register even if only 1 LMUL=1 register was used. Not sure if we should be optimizing for number of instructions or amount of stack space required.

Copy link
Collaborator Author

@topperc topperc Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have questions about what we do for the callee saved registers for vector calling convention now.

Copy link
Contributor

@wangpc-pp wangpc-pp Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @4vtomat

Copy link
Member

@4vtomat 4vtomat Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In vector callee saved registers we just put every combination of vector register class we didn't do anything special lol

Copy link
Member

@4vtomat 4vtomat Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think callee-saved register also have this issue when I change

llvm-project/llvm/test/CodeGen/RISCV/rvv/callee-saved-regs.ll

Lines 89 to 90 in 4f60321

call void asm sideeffect "",
"~{v0},~{v1},~{v2},~{v3},~{v4},~{v5},~{v6},~{v7},~{v8},~{v9},~{v10},~{v11},~{v12},~{v13},~{v14},~{v15},~{v16},~{v17},~{v18},~{v19},~{v20},~{v21},~{v22},~{v23},~{v24},~{v25},~{v26},~{v27},~{v28},~{v29},~{v30},~{v31}"()
to only use v24, it uses vs8r.

Copy link
Member

@4vtomat 4vtomat Jun 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the register is set if any of it's alias(including sub-registers and super-registers) is used, so in this case if v24 is clobbered, all of super-registers of v24 in callee-saved lists including v24, v24m2, v24m4 and v24m8 is set in SavedRegs.
https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/TargetFrameLoweringImpl.cpp#L145-L146


// Same as CSR_Interrupt, but excluding X16-X31.
def CSR_Interrupt_RVE : CalleeSavedRegs<(sub CSR_Interrupt,
(sequence "X%u", 16, 31))>;

// Same as CSR_XLEN_F32_Interrupt, but excluding X16-X31.
def CSR_XLEN_F32_Interrupt_RVE: CalleeSavedRegs<(sub CSR_XLEN_F32_Interrupt,
(sequence "X%u", 16, 31))>;
(sequence "X%u", 16, 31))>;

// Same as CSR_XLEN_F64_Interrupt, but excluding X16-X31.
def CSR_XLEN_F64_Interrupt_RVE: CalleeSavedRegs<(sub CSR_XLEN_F64_Interrupt,
(sequence "X%u", 16, 31))>;
(sequence "X%u", 16, 31))>;

// Same as CSR_XLEN_V_Interrupt, but excluding X16-X31.
def CSR_XLEN_V_Interrupt_RVE: CalleeSavedRegs<(add CSR_Interrupt,
(sequence "V%u", 0, 31))>;
Copy link
Member

@lenary lenary Jun 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look correct? I think it should be this (and similar changes in the 2 instances below)

Suggested change
def CSR_XLEN_V_Interrupt_RVE: CalleeSavedRegs<(add CSR_Interrupt,
(sequence "V%u", 0, 31))>;
def CSR_XLEN_V_Interrupt_RVE: CalleeSavedRegs<(add CSR_Interrupt_RVE,
(sequence "V%u", 0, 31))>;

Copy link
Collaborator Author

@topperc topperc Jun 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. Unfortunately, vectors+RVE crashes so I couldn't test it.


// Same as CSR_XLEN_F32_V_Interrupt, but excluding X16-X31.
def CSR_XLEN_F32_V_Interrupt_RVE: CalleeSavedRegs<(add CSR_Interrupt,
(sequence "F%u_F", 0, 31),
(sequence "V%u", 0, 31))>;

// Same as CSR_XLEN_F64_V_Interrupt, but excluding X16-X31.
def CSR_XLEN_F64_V_Interrupt_RVE: CalleeSavedRegs<(add CSR_Interrupt,
(sequence "F%u_D", 0, 31),
(sequence "V%u", 0, 31))>;
10 changes: 10 additions & 0 deletions llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ RISCVRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
if (MF->getFunction().getCallingConv() == CallingConv::GHC)
return CSR_NoRegs_SaveList;
if (MF->getFunction().hasFnAttribute("interrupt")) {
if (Subtarget.hasVInstructions()) {
if (Subtarget.hasStdExtD())
return Subtarget.hasStdExtE() ? CSR_XLEN_F64_V_Interrupt_RVE_SaveList
: CSR_XLEN_F64_V_Interrupt_SaveList;
if (Subtarget.hasStdExtF())
return Subtarget.hasStdExtE() ? CSR_XLEN_F32_V_Interrupt_RVE_SaveList
: CSR_XLEN_F32_V_Interrupt_SaveList;
return Subtarget.hasStdExtE() ? CSR_XLEN_V_Interrupt_RVE_SaveList
: CSR_XLEN_V_Interrupt_SaveList;
}
if (Subtarget.hasStdExtD())
return Subtarget.hasStdExtE() ? CSR_XLEN_F64_Interrupt_RVE_SaveList
: CSR_XLEN_F64_Interrupt_SaveList;
Expand Down
Loading
Loading