https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/221318
>From 3d68c396cb421db24087426ad7cbb3934d284ddf Mon Sep 17 00:00:00 2001 From: Zeyi Xu <[email protected]> Date: Sat, 5 Sep 2026 02:48:58 +0800 Subject: [PATCH 1/2] [RISCV] Support frame pointers in SiFive CLIC preemptible handlers --- llvm/lib/Target/RISCV/RISCVFrameLowering.cpp | 184 ++++++++++++------ llvm/lib/Target/RISCV/RISCVISelLowering.cpp | 6 - .../RISCV/sifive-interrupt-attr-err.ll | 12 -- .../RISCV/sifive-interrupt-frame-pointer.ll | 155 +++++++++++++++ 4 files changed, 280 insertions(+), 77 deletions(-) delete mode 100644 llvm/test/CodeGen/RISCV/sifive-interrupt-attr-err.ll create mode 100644 llvm/test/CodeGen/RISCV/sifive-interrupt-frame-pointer.ll diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp index 458cf32bf8c1b..3ba4ef969c4ed 100644 --- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp @@ -265,9 +265,8 @@ createSiFivePreemptibleInterruptFrameEntries(MachineFunction &MF, *MF.getSubtarget<RISCVSubtarget>().getRegisterInfo(); MachineFrameInfo &MFI = MF.getFrameInfo(); - // Create two frame objects for spilling X8 and X9, which will be done in - // `emitSiFiveCLICPreemptibleSaves`. This is in addition to any other stack - // objects we might have for X8 and X9, as they might be saved twice. + // Create two frame objects for preserving X8 and X9 or saving mcause and + // mepc, depending on whether the function uses a frame pointer. for (int I = 0; I < 2; ++I) { int FI = MFI.CreateStackObject(TRI.getSpillSize(RC), TRI.getSpillAlign(RC), true); @@ -289,35 +288,60 @@ static void emitSiFiveCLICPreemptibleSaves(MachineFunction &MF, // FIXME: CFI Information here is nonexistent/wrong. - // X8 and X9 might be stored into the stack twice, initially into the - // `interruptCSRFrameIndex` here, and then maybe again into their CSI frame - // index. - // - // This is done instead of telling the register allocator that we need two - // VRegs to store the value of `mcause` and `mepc` through the instruction, - // which affects other passes. - TII->storeRegToStackSlot(MBB, MBBI, RISCV::X8, /* IsKill=*/true, - RVFI->getInterruptCSRFrameIndex(0), - &RISCV::GPRRegClass, Register(), - MachineInstr::FrameSetup); - TII->storeRegToStackSlot(MBB, MBBI, RISCV::X9, /* IsKill=*/true, - RVFI->getInterruptCSRFrameIndex(1), - &RISCV::GPRRegClass, Register(), - MachineInstr::FrameSetup); - - // Put `mcause` into X8 (s0), and `mepc` into X9 (s1). If either of these are - // used in the function, then they will appear in `getUnmanagedCSI` and will - // be saved again. - BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRS)) - .addReg(RISCV::X8, RegState::Define) - .addImm(RISCVSysReg::mcause) - .addReg(RISCV::X0) - .setMIFlag(MachineInstr::FrameSetup); - BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRS)) - .addReg(RISCV::X9, RegState::Define) - .addImm(RISCVSysReg::mepc) - .addReg(RISCV::X0) - .setMIFlag(MachineInstr::FrameSetup); + if (STI.getFrameLowering()->hasFP(MF)) { + // With a frame pointer, use X5 as a temporary to save mcause and mepc to + // the stack. `determineCalleeSaves` adds X5 to the callee-saved set, so + // X5's original value has already been preserved. + BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRS)) + .addReg(RISCV::X5, RegState::Define) + .addImm(RISCVSysReg::mcause) + .addReg(RISCV::X0) + .setMIFlag(MachineInstr::FrameSetup); + TII->storeRegToStackSlot(MBB, MBBI, RISCV::X5, /* IsKill=*/true, + RVFI->getInterruptCSRFrameIndex(0), + &RISCV::GPRRegClass, Register(), + MachineInstr::FrameSetup); + + BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRS)) + .addReg(RISCV::X5, RegState::Define) + .addImm(RISCVSysReg::mepc) + .addReg(RISCV::X0) + .setMIFlag(MachineInstr::FrameSetup); + TII->storeRegToStackSlot(MBB, MBBI, RISCV::X5, /* IsKill=*/true, + RVFI->getInterruptCSRFrameIndex(1), + &RISCV::GPRRegClass, Register(), + MachineInstr::FrameSetup); + } else { + // X8 and X9 might be stored into the stack twice, initially into the + // `interruptCSRFrameIndex` here, and then maybe again into their CSI frame + // index. + // + // This is done instead of telling the register allocator that we need two + // VRegs to store the value of `mcause` and `mepc` through the instruction, + // which affects other passes. + TII->storeRegToStackSlot(MBB, MBBI, RISCV::X8, /* IsKill=*/true, + RVFI->getInterruptCSRFrameIndex(0), + &RISCV::GPRRegClass, Register(), + MachineInstr::FrameSetup); + TII->storeRegToStackSlot(MBB, MBBI, RISCV::X9, /* IsKill=*/true, + RVFI->getInterruptCSRFrameIndex(1), + &RISCV::GPRRegClass, Register(), + MachineInstr::FrameSetup); + + // Put `mcause` into X8 (s0), and `mepc` into X9 (s1). If either of these + // are used in the function, then they will appear in `getUnmanagedCSI` and + // will be saved again. + BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRS)) + .addReg(RISCV::X8, RegState::Define) + .addImm(RISCVSysReg::mcause) + .addReg(RISCV::X0) + .setMIFlag(MachineInstr::FrameSetup); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRS)) + .addReg(RISCV::X9, RegState::Define) + .addImm(RISCVSysReg::mepc) + .addReg(RISCV::X0) + .setMIFlag(MachineInstr::FrameSetup); + } // Enable interrupts. BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRSI)) @@ -348,30 +372,54 @@ static void emitSiFiveCLICPreemptibleRestores(MachineFunction &MF, .addImm(8) .setMIFlag(MachineInstr::FrameDestroy); - // Restore `mepc` from x9 (s1), and `mcause` from x8 (s0). If either were used - // in the function, they have already been restored once, so now have the - // value stored in `emitSiFiveCLICPreemptibleSaves`. - BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRW)) - .addReg(RISCV::X0, RegState::Define) - .addImm(RISCVSysReg::mepc) - .addReg(RISCV::X9, RegState::Kill) - .setMIFlag(MachineInstr::FrameDestroy); - BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRW)) - .addReg(RISCV::X0, RegState::Define) - .addImm(RISCVSysReg::mcause) - .addReg(RISCV::X8, RegState::Kill) - .setMIFlag(MachineInstr::FrameDestroy); + if (STI.getFrameLowering()->hasFP(MF)) { + // Restore `mepc` and `mcause` through X5. The normal callee-saved register + // handling will restore the value X5 held on entry to the handler later. + TII->loadRegFromStackSlot(MBB, MBBI, RISCV::X5, + RVFI->getInterruptCSRFrameIndex(1), + &RISCV::GPRRegClass, Register(), + RISCV::NoSubRegister, MachineInstr::FrameDestroy); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRW)) + .addReg(RISCV::X0, RegState::Define) + .addImm(RISCVSysReg::mepc) + .addReg(RISCV::X5, RegState::Kill) + .setMIFlag(MachineInstr::FrameDestroy); - // X8 and X9 need to be restored to their values on function entry, which we - // saved onto the stack in `emitSiFiveCLICPreemptibleSaves`. - TII->loadRegFromStackSlot(MBB, MBBI, RISCV::X9, - RVFI->getInterruptCSRFrameIndex(1), - &RISCV::GPRRegClass, Register(), - RISCV::NoSubRegister, MachineInstr::FrameDestroy); - TII->loadRegFromStackSlot(MBB, MBBI, RISCV::X8, - RVFI->getInterruptCSRFrameIndex(0), - &RISCV::GPRRegClass, Register(), - RISCV::NoSubRegister, MachineInstr::FrameDestroy); + TII->loadRegFromStackSlot(MBB, MBBI, RISCV::X5, + RVFI->getInterruptCSRFrameIndex(0), + &RISCV::GPRRegClass, Register(), + RISCV::NoSubRegister, MachineInstr::FrameDestroy); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRW)) + .addReg(RISCV::X0, RegState::Define) + .addImm(RISCVSysReg::mcause) + .addReg(RISCV::X5, RegState::Kill) + .setMIFlag(MachineInstr::FrameDestroy); + } else { + // Restore `mepc` from x9 (s1), and `mcause` from x8 (s0). If either were + // used in the function, they have already been restored once, so now have + // the value stored in `emitSiFiveCLICPreemptibleSaves`. + BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRW)) + .addReg(RISCV::X0, RegState::Define) + .addImm(RISCVSysReg::mepc) + .addReg(RISCV::X9, RegState::Kill) + .setMIFlag(MachineInstr::FrameDestroy); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRW)) + .addReg(RISCV::X0, RegState::Define) + .addImm(RISCVSysReg::mcause) + .addReg(RISCV::X8, RegState::Kill) + .setMIFlag(MachineInstr::FrameDestroy); + + // X8 and X9 need to be restored to their values on function entry, which we + // saved onto the stack in `emitSiFiveCLICPreemptibleSaves`. + TII->loadRegFromStackSlot(MBB, MBBI, RISCV::X9, + RVFI->getInterruptCSRFrameIndex(1), + &RISCV::GPRRegClass, Register(), + RISCV::NoSubRegister, MachineInstr::FrameDestroy); + TII->loadRegFromStackSlot(MBB, MBBI, RISCV::X8, + RVFI->getInterruptCSRFrameIndex(0), + &RISCV::GPRRegClass, Register(), + RISCV::NoSubRegister, MachineInstr::FrameDestroy); + } } // Get the ID of the libcall used for spilling and restoring callee saved @@ -1138,8 +1186,10 @@ void RISCVFrameLowering::emitPrologue(MachineFunction &MF, NeedProbe, ProbeSize, DynAllocation, MachineInstr::FrameSetup); - // Save SiFive CLIC CSRs into Stack - emitSiFiveCLICPreemptibleSaves(MF, MBB, MBBI, DL); + // Without a frame pointer, `mcause` and `mepc` remain in X8 and X9 across + // the handler. + if (!hasFP(MF)) + emitSiFiveCLICPreemptibleSaves(MF, MBB, MBBI, DL); // The frame pointer is callee-saved, and code has been generated for us to // save it to the stack. We need to skip over the storing of callee-saved @@ -1191,6 +1241,11 @@ void RISCVFrameLowering::emitPrologue(MachineFunction &MF, CFIBuilder.buildDefCFA(FPReg, RVFI->getVarArgsSaveSize()); } + // With a frame pointer, X5 is used as a temporary, so save the CSRs after + // the normal callee-saved spill has preserved X5's original value. + if (hasFP(MF)) + emitSiFiveCLICPreemptibleSaves(MF, MBB, MBBI, DL); + uint64_t SecondSPAdjustAmount = 0; // Emit the second SP adjustment after saving callee saved registers. if (FirstSPAdjustAmount) { @@ -1401,6 +1456,10 @@ void RISCVFrameLowering::emitEpilogue(MachineFunction &MF, if (NeedsDwarfCFI && hasFP(MF)) CFIBuilder.buildDefCFA(SPReg, RealStackSize); + // With a frame pointer, restore the SiFive CLIC CSRs through X5. + if (hasFP(MF)) + emitSiFiveCLICPreemptibleRestores(MF, MBB, FirstScalarCSRRestoreInsn, DL); + // Skip to after the restores of scalar callee-saved registers // FIXME: assumes exactly one instruction is used to restore each // callee-saved register. @@ -1469,7 +1528,9 @@ void RISCVFrameLowering::emitEpilogue(MachineFunction &MF, } } - emitSiFiveCLICPreemptibleRestores(MF, MBB, MBBI, DL); + // Without a frame pointer, X8 and X9 hold mcause and mepc here. + if (!hasFP(MF)) + emitSiFiveCLICPreemptibleRestores(MF, MBB, MBBI, DL); // Deallocate stack if StackSize isn't a zero yet. If this is a QCI interrupt // function, there will be a leftover offset which is deallocated by @@ -1869,8 +1930,13 @@ void RISCVFrameLowering::determineCalleeSaves(MachineFunction &MF, if (hasBP(MF)) SavedRegs.set(RISCVABI::getBPReg()); - // When using cm.push/pop we must save X27 if we save X26. auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>(); + // X5 is used as a temporary for saving and restoring mcause and mepc when + // X8 is occupied by the frame pointer. + if (RVFI->isSiFivePreemptibleInterrupt(MF) && hasFP(MF)) + SavedRegs.set(RISCV::X5); + + // When using cm.push/pop we must save X27 if we save X26. if (RVFI->isPushable(MF) && SavedRegs.test(RISCV::X26)) SavedRegs.set(RISCV::X27); diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp index b2f684952dd90..bec9f87ff2a19 100644 --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp @@ -27073,12 +27073,6 @@ SDValue RISCVTargetLowering::LowerFormalArguments( if (Kind == "rnmi" && !Subtarget.hasStdExtSmrnmi()) reportFatalUsageError("'rnmi' interrupt kind requires Srnmi extension"); - const TargetFrameLowering *TFI = Subtarget.getFrameLowering(); - if (Kind.starts_with("SiFive-CLIC-preemptible") && TFI->hasFP(MF)) - Func.getContext().diagnose(DiagnosticInfoUnsupported{ - Func, - "'SiFive-CLIC-preemptible' interrupt functions cannot have a frame " - "pointer"}); } EVT PtrVT = getPointerTy(DAG.getDataLayout()); diff --git a/llvm/test/CodeGen/RISCV/sifive-interrupt-attr-err.ll b/llvm/test/CodeGen/RISCV/sifive-interrupt-attr-err.ll deleted file mode 100644 index f0e1f16cf6e54..0000000000000 --- a/llvm/test/CodeGen/RISCV/sifive-interrupt-attr-err.ll +++ /dev/null @@ -1,12 +0,0 @@ -; RUN: not llc -mtriple riscv32-unknown-elf -mattr=+experimental-xsfmclic -o - %s 2>&1 \ -; RUN: | FileCheck %s -; RUN: not llc -mtriple riscv64-unknown-elf -mattr=+experimental-xsfmclic -o - %s 2>&1 \ -; RUN: | FileCheck %s - -;; Test that these report regular errors. - -; CHECK: error: <unknown>:0:0: in function preemptible void (): 'SiFive-CLIC-preemptible' interrupt functions cannot have a frame pointer - -define void @preemptible() "interrupt"="SiFive-CLIC-preemptible" "frame-pointer"="all" { - ret void -} diff --git a/llvm/test/CodeGen/RISCV/sifive-interrupt-frame-pointer.ll b/llvm/test/CodeGen/RISCV/sifive-interrupt-frame-pointer.ll new file mode 100644 index 0000000000000..da025cf86e65e --- /dev/null +++ b/llvm/test/CodeGen/RISCV/sifive-interrupt-frame-pointer.ll @@ -0,0 +1,155 @@ +; RUN: llc -mtriple riscv32-unknown-elf -mattr=+experimental-xsfmclic \ +; RUN: -verify-machineinstrs < %s | FileCheck %s --check-prefix=RV32 +; RUN: llc -mtriple riscv64-unknown-elf -mattr=+experimental-xsfmclic \ +; RUN: -verify-machineinstrs < %s | FileCheck %s --check-prefix=RV64 + +@size = external global i32 + +declare void @use(ptr) + +define void @preemptible() "interrupt"="SiFive-CLIC-preemptible" "frame-pointer"="all" { +; RV32-LABEL: preemptible: +; RV32: sw t0, 16(sp) +; RV32: sw s0, 12(sp) +; RV32: addi s0, sp, 32 +; RV32: csrr t0, mcause +; RV32-NEXT: sw t0, 28(sp) +; RV32-NEXT: csrr t0, mepc +; RV32-NEXT: sw t0, 24(sp) +; RV32-NEXT: csrsi mstatus, 8 +; RV32: csrci mstatus, 8 +; RV32-NEXT: lw t0, 24(sp) +; RV32-NEXT: csrw mepc, t0 +; RV32-NEXT: lw t0, 28(sp) +; RV32-NEXT: csrw mcause, t0 +; RV32: lw t0, 16(sp) +; RV32: lw s0, 12(sp) +; RV32: mret +; +; RV64-LABEL: preemptible: +; RV64: sd t0, 16(sp) +; RV64: sd s0, 8(sp) +; RV64: addi s0, sp, 48 +; RV64: csrr t0, mcause +; RV64-NEXT: sd t0, 40(sp) +; RV64-NEXT: csrr t0, mepc +; RV64-NEXT: sd t0, 32(sp) +; RV64-NEXT: csrsi mstatus, 8 +; RV64: csrci mstatus, 8 +; RV64-NEXT: ld t0, 32(sp) +; RV64-NEXT: csrw mepc, t0 +; RV64-NEXT: ld t0, 40(sp) +; RV64-NEXT: csrw mcause, t0 +; RV64: ld t0, 16(sp) +; RV64: ld s0, 8(sp) +; RV64: mret + ret void +} + +define void @preemptible_stack_swap() "interrupt"="SiFive-CLIC-preemptible-stack-swap" "frame-pointer"="all" { +; RV32-LABEL: preemptible_stack_swap: +; RV32: csrrw sp, sf.mscratchcsw, sp +; RV32: addi s0, sp, 32 +; RV32: csrr t0, mcause +; RV32: csrsi mstatus, 8 +; RV32: csrci mstatus, 8 +; RV32: csrw mepc, t0 +; RV32: csrw mcause, t0 +; RV32: lw s0, 12(sp) +; RV32: csrrw sp, sf.mscratchcsw, sp +; RV32-NEXT: mret +; +; RV64-LABEL: preemptible_stack_swap: +; RV64: csrrw sp, sf.mscratchcsw, sp +; RV64: addi s0, sp, 48 +; RV64: csrr t0, mcause +; RV64: csrsi mstatus, 8 +; RV64: csrci mstatus, 8 +; RV64: csrw mepc, t0 +; RV64: csrw mcause, t0 +; RV64: ld s0, 8(sp) +; RV64: csrrw sp, sf.mscratchcsw, sp +; RV64-NEXT: mret + ret void +} + +define void @preemptible_var_alloca() "interrupt"="SiFive-CLIC-preemptible" "frame-pointer"="none" { +; RV32-LABEL: preemptible_var_alloca: +; RV32: addi s0, sp, 80 +; RV32: csrr t0, mcause +; RV32-NEXT: sw t0, -4(s0) +; RV32-NEXT: csrr t0, mepc +; RV32-NEXT: sw t0, -8(s0) +; RV32-NEXT: csrsi mstatus, 8 +; RV32: sub a0, sp, a0 +; RV32: mv sp, a0 +; RV32: call use +; RV32: addi sp, s0, -80 +; RV32: csrci mstatus, 8 +; RV32-NEXT: lw t0, -8(s0) +; RV32-NEXT: csrw mepc, t0 +; RV32-NEXT: lw t0, -4(s0) +; RV32-NEXT: csrw mcause, t0 +; RV32: mret +; +; RV64-LABEL: preemptible_var_alloca: +; RV64: addi s0, sp, 160 +; RV64: csrr t0, mcause +; RV64-NEXT: sd t0, -8(s0) +; RV64-NEXT: csrr t0, mepc +; RV64-NEXT: sd t0, -16(s0) +; RV64-NEXT: csrsi mstatus, 8 +; RV64: sub a0, sp, a0 +; RV64: mv sp, a0 +; RV64: call use +; RV64: addi sp, s0, -160 +; RV64: csrci mstatus, 8 +; RV64-NEXT: ld t0, -16(s0) +; RV64-NEXT: csrw mepc, t0 +; RV64-NEXT: ld t0, -8(s0) +; RV64-NEXT: csrw mcause, t0 +; RV64: mret + %count = load volatile i32, ptr @size + %object = alloca i8, i32 %count, align 16 + call void @use(ptr %object) + ret void +} + +define void @preemptible_realign() "interrupt"="SiFive-CLIC-preemptible" "frame-pointer"="none" { +; RV32-LABEL: preemptible_realign: +; RV32: addi s0, sp, +; RV32: csrr t0, mcause +; RV32-NEXT: sw t0, +; RV32-NEXT: csrr t0, mepc +; RV32-NEXT: sw t0, +; RV32-NEXT: csrsi mstatus, 8 +; RV32: andi sp, sp, -64 +; RV32: call use +; RV32: addi sp, s0, +; RV32: csrci mstatus, 8 +; RV32-NEXT: lw t0, 184(sp) +; RV32-NEXT: csrw mepc, t0 +; RV32-NEXT: lw t0, 188(sp) +; RV32-NEXT: csrw mcause, t0 +; RV32: mret +; +; RV64-LABEL: preemptible_realign: +; RV64: addi s0, sp, +; RV64: csrr t0, mcause +; RV64-NEXT: sd t0, +; RV64-NEXT: csrr t0, mepc +; RV64-NEXT: sd t0, +; RV64-NEXT: csrsi mstatus, 8 +; RV64: andi sp, sp, -64 +; RV64: call use +; RV64: addi sp, s0, +; RV64: csrci mstatus, 8 +; RV64-NEXT: ld t0, 240(sp) +; RV64-NEXT: csrw mepc, t0 +; RV64-NEXT: ld t0, 248(sp) +; RV64-NEXT: csrw mcause, t0 +; RV64: mret + %object = alloca [64 x i8], align 64 + call void @use(ptr %object) + ret void +} >From 440de9e8c904a0664b9742db118b831f3bfdbc55 Mon Sep 17 00:00:00 2001 From: Zeyi Xu <[email protected]> Date: Sat, 5 Sep 2026 02:50:00 +0800 Subject: [PATCH 2/2] forgot to add release notes, oops --- clang/docs/ReleaseNotes.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index edf74404c4925..fd84d4b28ec66 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -722,8 +722,7 @@ features cannot lower the translation-unit ABI level; - Added a new warning when the same interrupt type is specified more than once in a RISC-V `interrupt` attribute. -- SiFive CLIC preemptible interrupt handlers now diagnose unsupported frame - pointers instead of producing a backend fatal error. +- SiFive CLIC preemptible interrupt handlers now support frame pointers. - Added `-march=native` for better compatibility with ARM, AArch64, and X86. This option will be treated like `-mcpu=native` if `-mcpu` is not present. If _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
