[RISCV] SiFive CLIC Support #132481
Merged
[RISCV] SiFive CLIC Support #132481
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This Change adds support for two SiFive vendor attributes in clang: - "SiFive-CLIC-preemptible" - "SiFive-CLIC-stack-swap" These can be given together, and can be combined with "machine", but cannot be combined with any other interrupt attribute values. These are handled primarily in RISCVFrameLowering: - "SiFive-CLIC-stack-swap" entails swapping `sp` with `mscratchcsw` at function entry and exit, which holds the trap stack pointer. - "SiFive-CLIC-preemptible" entails saving `mcause` and `mepc` before re-enabling interrupts using `mstatus`. To save these, `s0` and `s1` are first spilled to the stack, and then the values are read into these registers. If these registers are used in the function, their values will be spilled a second time onto the stack with the generic callee-saved-register handling. At the end of the function interrupts are disabled again before `mepc` and `mcause` are restored. The CFI information for this implementation is not correct. Co-authored-by: Ana Pazos <[email protected]>
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Sam Elliott (lenary) ChangesThis Change adds support for two SiFive vendor attributes in clang:
These can be given together, and can be combined with "machine", but These are handled primarily in RISCVFrameLowering:
This Change also adds support for the following two experimental extensions, which only contain CSRs:
The latter is needed for interrupt support. The CFI information for this implementation is not correct, but I'd prefer to correct this in a follow-up. While it's unlikely anyone wants to unwind through a handler, the CFI information is also used by debuggers so it would be good to get it right. Patch is 96.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132481.diff 22 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index b8cd3475bb88a..8f1f5baef24a2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2224,10 +2224,23 @@ def NoMicroMips : InheritableAttr, TargetSpecificAttr<TargetMips32> {
def RISCVInterrupt : InheritableAttr, TargetSpecificAttr<TargetRISCV> {
let Spellings = [GCC<"interrupt">];
let Subjects = SubjectList<[Function]>;
- let Args = [EnumArgument<"Interrupt", "InterruptType", /*is_string=*/true,
- ["supervisor", "machine", "qci-nest", "qci-nonest"],
- ["supervisor", "machine", "qcinest", "qcinonest"],
- 1>];
+ let Args = [VariadicEnumArgument<"Interrupt", "InterruptType", /*is_string=*/true,
+ [
+ "supervisor",
+ "machine",
+ "qci-nest",
+ "qci-nonest",
+ "SiFive-CLIC-preemptible",
+ "SiFive-CLIC-stack-swap",
+ ],
+ [
+ "supervisor",
+ "machine",
+ "qcinest",
+ "qcinonest",
+ "SiFiveCLICPreemptible",
+ "SiFiveCLICStackSwap",
+ ]>];
let ParseKind = "Interrupt";
let Documentation = [RISCVInterruptDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 34e7ff9612859..552cb7f45a7d2 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2863,8 +2863,9 @@ targets. This attribute may be attached to a function definition and instructs
the backend to generate appropriate function entry/exit code so that it can be
used directly as an interrupt service routine.
-Permissible values for this parameter are ``supervisor``, ``machine``,
-``qci-nest`` and ``qci-nonest``. If there is no parameter, then it defaults to
+Permissible values for this parameter are ``machine``, ``supervisor``,
+``qci-nest``, ``qci-nonest``, ``SiFive-CLIC-preemptible``, and
+``SiFive-CLIC-stack-swap``. If there is no parameter, then it defaults to
``machine``.
The ``qci-nest`` and ``qci-nonest`` values require Qualcomm's Xqciint extension
@@ -2875,6 +2876,15 @@ restore interrupt state to the stack -- the ``qci-nest`` value will use
begin the interrupt handler. Both of these will use ``qc.c.mileaveret`` to
restore the state and return to the previous context.
+The ``SiFive-CLIC-preemptible`` and ``SiFive-CLIC-stack-swap`` values are used
+for machine-mode interrupts. For ``SiFive-CLIC-preemptible`` interrupts, the
+values of ``mcause`` and ``mepc`` are saved onto the stack, and interrupts are
+re-enabled. For ``SiFive-CLIC-stack-swap`` interrupts, the stack pointer is
+swapped with ``mscratch`` before its first use and after its last use.
+
+The SiFive CLIC values may be combined with each other and with the ``machine``
+attribute value. Any other combination of different values is not allowed.
+
Repeated interrupt attribute on the same declaration will cause a warning
to be emitted. In case of repeated declarations, the last one prevails.
@@ -2884,6 +2894,7 @@ https://riscv.org/specifications/privileged-isa/
The RISC-V Instruction Set Manual Volume II: Privileged Architecture
Version 1.10.
https://github.com/quic/riscv-unified-db/releases/tag/Xqci-0.7
+https://sifive.cdn.prismic.io/sifive/d1984d2b-c9b9-4c91-8de0-d68a5e64fa0f_sifive-interrupt-cookbook-v1p2.pdf
}];
}
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1536a3b8c920a..0c587d8b677d5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12705,7 +12705,9 @@ def err_riscv_builtin_invalid_lmul : Error<
def err_riscv_type_requires_extension : Error<
"RISC-V type %0 requires the '%1' extension">;
def err_riscv_attribute_interrupt_requires_extension : Error<
- "RISC-V interrupt attribute '%0' requires extension '%1'">;
+ "RISC-V 'interrupt' attribute '%0' requires extension '%1'">;
+def err_riscv_attribute_interrupt_invalid_combination : Error<
+ "RISC-V 'interrupt' attribute contains invalid combination of interrupt types">;
def err_std_source_location_impl_not_found : Error<
"'std::source_location::__impl' was not found; it must be defined before '__builtin_source_location' is called">;
diff --git a/clang/lib/CodeGen/Targets/RISCV.cpp b/clang/lib/CodeGen/Targets/RISCV.cpp
index 5aa10ba41f5ed..14d4cee7c61d3 100644
--- a/clang/lib/CodeGen/Targets/RISCV.cpp
+++ b/clang/lib/CodeGen/Targets/RISCV.cpp
@@ -829,16 +829,39 @@ class RISCVTargetCodeGenInfo : public TargetCodeGenInfo {
if (!Attr)
return;
- const char *Kind;
- switch (Attr->getInterrupt()) {
- case RISCVInterruptAttr::supervisor: Kind = "supervisor"; break;
- case RISCVInterruptAttr::machine: Kind = "machine"; break;
- case RISCVInterruptAttr::qcinest:
- Kind = "qci-nest";
- break;
- case RISCVInterruptAttr::qcinonest:
- Kind = "qci-nonest";
- break;
+ StringRef Kind = "machine";
+ bool HasSiFiveCLICPreemptible = false;
+ bool HasSiFiveCLICStackSwap = false;
+ for (RISCVInterruptAttr::InterruptType type : Attr->interrupt()) {
+ switch (type) {
+ case RISCVInterruptAttr::machine:
+ // Do not update `Kind` because `Kind` is already "machine", or the
+ // kinds also contains SiFive types which need to be applied.
+ break;
+ case RISCVInterruptAttr::supervisor:
+ Kind = "supervisor";
+ break;
+ case RISCVInterruptAttr::qcinest:
+ Kind = "qci-nest";
+ break;
+ case RISCVInterruptAttr::qcinonest:
+ Kind = "qci-nonest";
+ break;
+ // There are three different LLVM IR attribute values for SiFive CLIC
+ // interrupt kinds, one for each kind and one extra for their combination.
+ case RISCVInterruptAttr::SiFiveCLICPreemptible: {
+ HasSiFiveCLICPreemptible = true;
+ Kind = HasSiFiveCLICStackSwap ? "SiFive-CLIC-preemptible-stack-swap"
+ : "SiFive-CLIC-preemptible";
+ break;
+ }
+ case RISCVInterruptAttr::SiFiveCLICStackSwap: {
+ HasSiFiveCLICStackSwap = true;
+ Kind = HasSiFiveCLICPreemptible ? "SiFive-CLIC-preemptible-stack-swap"
+ : "SiFive-CLIC-stack-swap";
+ break;
+ }
+ }
}
Fn->addFnAttr("interrupt", Kind);
diff --git a/clang/lib/Sema/SemaRISCV.cpp b/clang/lib/Sema/SemaRISCV.cpp
index f23827d566610..25ff1e99498ec 100644
--- a/clang/lib/Sema/SemaRISCV.cpp
+++ b/clang/lib/Sema/SemaRISCV.cpp
@@ -13,6 +13,7 @@
#include "clang/Sema/SemaRISCV.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
+#include "clang/AST/Attrs.inc"
#include "clang/AST/Decl.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/TargetBuiltins.h"
@@ -1453,25 +1454,14 @@ void SemaRISCV::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
return;
}
- // Check the attribute argument. Argument is optional.
- if (!AL.checkAtMostNumArgs(SemaRef, 1))
- return;
-
- StringRef Str;
- SourceLocation ArgLoc;
-
- // 'machine'is the default interrupt mode.
- if (AL.getNumArgs() == 0)
- Str = "machine";
- else if (!SemaRef.checkStringLiteralArgumentAttr(AL, 0, Str, &ArgLoc))
- return;
-
// Semantic checks for a function with the 'interrupt' attribute:
// - Must be a function.
// - Must have no parameters.
// - Must have the 'void' return type.
- // - The attribute itself must either have no argument or one of the
- // valid interrupt types, see [RISCVInterruptDocs].
+ // - The attribute itself must have at most 2 arguments
+ // - The attribute arguments must be string literals, and valid choices.
+ // - The attribute arguments must be a valid combination
+ // - The current target must support the right extensions for the combination.
if (D->getFunctionType() == nullptr) {
Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
@@ -1491,35 +1481,105 @@ void SemaRISCV::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
return;
}
- RISCVInterruptAttr::InterruptType Kind;
- if (!RISCVInterruptAttr::ConvertStrToInterruptType(Str, Kind)) {
- Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
- << AL << Str << ArgLoc;
+ if (!AL.checkAtMostNumArgs(SemaRef, 2))
return;
- }
- switch (Kind) {
- default:
- break;
- case RISCVInterruptAttr::InterruptType::qcinest:
- case RISCVInterruptAttr::InterruptType::qcinonest: {
- const TargetInfo &TI = getASTContext().getTargetInfo();
- llvm::StringMap<bool> FunctionFeatureMap;
- getASTContext().getFunctionFeatureMap(FunctionFeatureMap,
- dyn_cast<FunctionDecl>(D));
+ bool HasSiFiveCLICType = false;
+ bool HasUnaryType = false;
+
+ SmallSet<RISCVInterruptAttr::InterruptType, 2> Types;
+ for (unsigned ArgIndex = 0; ArgIndex < AL.getNumArgs(); ++ArgIndex) {
+ RISCVInterruptAttr::InterruptType Type;
+ StringRef TypeString;
+ SourceLocation Loc;
- if (!TI.hasFeature("experimental-xqciint") &&
- !FunctionFeatureMap.lookup("experimental-xqciint")) {
- Diag(AL.getLoc(), diag::err_riscv_attribute_interrupt_requires_extension)
- << Str << "Xqciint";
+ if (!SemaRef.checkStringLiteralArgumentAttr(AL, ArgIndex, TypeString, &Loc))
+ return;
+
+ if (!RISCVInterruptAttr::ConvertStrToInterruptType(TypeString, Type)) {
+ std::string TypeLiteral = ("\"" + TypeString + "\"").str();
+ Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
+ << AL << TypeLiteral << Loc;
return;
}
- break;
+
+ switch (Type) {
+ case RISCVInterruptAttr::machine:
+ // "machine" could be combined with the SiFive CLIC types, or could be
+ // just "machine".
+ break;
+ case RISCVInterruptAttr::SiFiveCLICPreemptible:
+ case RISCVInterruptAttr::SiFiveCLICStackSwap:
+ // SiFive-CLIC types can be combined with each other and "machine"
+ HasSiFiveCLICType = true;
+ break;
+ case RISCVInterruptAttr::supervisor:
+ case RISCVInterruptAttr::qcinest:
+ case RISCVInterruptAttr::qcinonest:
+ // "supervisor" and "qci-(no)nest" cannot be combined with any other types
+ HasUnaryType = true;
+ break;
+ }
+
+ Types.insert(Type);
+ }
+
+ if (HasUnaryType && Types.size() > 1) {
+ Diag(AL.getLoc(), diag::err_riscv_attribute_interrupt_invalid_combination);
+ return;
}
+
+ if (HasUnaryType && HasSiFiveCLICType) {
+ Diag(AL.getLoc(), diag::err_riscv_attribute_interrupt_invalid_combination);
+ return;
+ }
+
+ // "machine" is the default, if nothing is specified.
+ if (AL.getNumArgs() == 0)
+ Types.insert(RISCVInterruptAttr::machine);
+
+ const TargetInfo &TI = getASTContext().getTargetInfo();
+ llvm::StringMap<bool> FunctionFeatureMap;
+ getASTContext().getFunctionFeatureMap(FunctionFeatureMap,
+ dyn_cast<FunctionDecl>(D));
+
+ auto HasFeature = [&](StringRef FeatureName) -> bool {
+ return TI.hasFeature(FeatureName) || FunctionFeatureMap.lookup(FeatureName);
};
- D->addAttr(::new (getASTContext())
- RISCVInterruptAttr(getASTContext(), AL, Kind));
+ for (RISCVInterruptAttr::InterruptType Type : Types) {
+ switch (Type) {
+ // The QCI interrupt types require Xqciint
+ case RISCVInterruptAttr::qcinest:
+ case RISCVInterruptAttr::qcinonest: {
+ if (!HasFeature("experimental-xqciint")) {
+ Diag(AL.getLoc(),
+ diag::err_riscv_attribute_interrupt_requires_extension)
+ << RISCVInterruptAttr::ConvertInterruptTypeToStr(Type) << "Xqciint";
+ return;
+ }
+ } break;
+ // The SiFive CLIC interrupt types require Xsfmclic
+ case RISCVInterruptAttr::SiFiveCLICPreemptible:
+ case RISCVInterruptAttr::SiFiveCLICStackSwap: {
+ if (!HasFeature("experimental-xsfmclic")) {
+ Diag(AL.getLoc(),
+ diag::err_riscv_attribute_interrupt_requires_extension)
+ << RISCVInterruptAttr::ConvertInterruptTypeToStr(Type)
+ << "XSfmclic";
+ return;
+ }
+ } break;
+ default:
+ break;
+ }
+ }
+
+ SmallVector<RISCVInterruptAttr::InterruptType, 2> TypesVec(Types.begin(),
+ Types.end());
+
+ D->addAttr(::new (getASTContext()) RISCVInterruptAttr(
+ getASTContext(), AL, TypesVec.data(), TypesVec.size()));
}
bool SemaRISCV::isAliasValid(unsigned BuiltinID, StringRef AliasName) {
diff --git a/clang/test/Driver/print-supported-extensions-riscv.c b/clang/test/Driver/print-supported-extensions-riscv.c
index 35de2820ef84f..ca695221dee9d 100644
--- a/clang/test/Driver/print-supported-extensions-riscv.c
+++ b/clang/test/Driver/print-supported-extensions-riscv.c
@@ -213,6 +213,8 @@
// CHECK-NEXT: xqcisls 0.2 'Xqcisls' (Qualcomm uC Scaled Load Store Extension)
// CHECK-NEXT: xrivosvisni 0.1 'XRivosVisni' (Rivos Vector Integer Small New)
// CHECK-NEXT: xrivosvizip 0.1 'XRivosVizip' (Rivos Vector Register Zips)
+// CHECK-NEXT: xsfmclic 0.1 'XSfmclic' (SiFive CLIC Machine-mode CSRs)
+// CHECK-NEXT: xsfsclic 0.1 'XSfsclic' (SiFive CLIC Supervisor-mode CSRs)
// CHECK-EMPTY:
// CHECK-NEXT: Supported Profiles
// CHECK-NEXT: rva20s64
diff --git a/clang/test/Sema/riscv-interrupt-attr-qci.c b/clang/test/Sema/riscv-interrupt-attr-qci.c
index bdac4e154bb3c..e54c50c0e25bb 100644
--- a/clang/test/Sema/riscv-interrupt-attr-qci.c
+++ b/clang/test/Sema/riscv-interrupt-attr-qci.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple riscv32-unknown-elf -target-feature +experimental-xqciint -emit-llvm -DCHECK_IR < %s| FileCheck %s
+// RUN: %clang_cc1 -triple riscv32-unknown-elf -target-feature +experimental-xqciint -emit-llvm -DCHECK_IR < %s | FileCheck %s
// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -target-feature +experimental-xqciint -verify=enabled,both -fsyntax-only
// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify=disabled,both -fsyntax-only
// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -target-feature -experimental-xqciint -verify=disabled,both -fsyntax-only
@@ -11,10 +11,20 @@
__attribute__((interrupt("qci-nest")))
void foo_nest_interrupt(void) {}
-// CHECK-LABEL: @foo_nonnest_interrupt() #1
+// CHECK-LABEL: @foo_nest_nest_interrupt() #0
+// CHECK: ret void
+__attribute__((interrupt("qci-nest", "qci-nest")))
+void foo_nest_nest_interrupt(void) {}
+
+// CHECK-LABEL: @foo_nonest_interrupt() #1
// CHECK: ret void
__attribute__((interrupt("qci-nonest")))
-void foo_nonnest_interrupt(void) {}
+void foo_nonest_interrupt(void) {}
+
+// CHECK-LABEL: @foo_nonest_nonest_interrupt() #1
+// CHECK: ret void
+__attribute__((interrupt("qci-nonest", "qci-nonest")))
+void foo_nonest_nonest_interrupt(void) {}
// CHECK: attributes #0
// CHECK: "interrupt"="qci-nest"
@@ -22,18 +32,23 @@ void foo_nonnest_interrupt(void) {}
// CHECK: "interrupt"="qci-nonest"
#else
// Test for QCI extension's interrupt attribute support
-__attribute__((interrupt("qci-est"))) void foo_nest1(void) {} // both-warning {{'interrupt' attribute argument not supported: qci-est}}
-__attribute__((interrupt("qci-noest"))) void foo_nonest1(void) {} // both-warning {{'interrupt' attribute argument not supported: qci-noest}}
-__attribute__((interrupt(1))) void foo_nest2(void) {} // both-error {{expected string literal as argument of 'interrupt' attribute}}
-__attribute__((interrupt("qci-nest", "qci-nonest"))) void foo1(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("qci-nonest", "qci-nest"))) void foo2(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("", "qci-nonest"))) void foo3(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("", "qci-nest"))) void foo4(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("qci-nonest", 1))) void foo5(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("qci-nest", 1))) void foo6(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-
-__attribute__((interrupt("qci-nest"))) void foo_nest(void) {} // disabled-error {{RISC-V interrupt attribute 'qci-nest' requires extension 'Xqciint'}}
-__attribute__((interrupt("qci-nonest"))) void foo_nonest(void) {} // disabled-error {{RISC-V interrupt attribute 'qci-nonest' requires extension 'Xqciint'}}
+__attribute__((interrupt(1))) void foo1(void) {} // both-error {{expected string literal as argument of 'interrupt' attribute}}
+__attribute__((interrupt("qci-nonest", 1))) void foo_nonest2(void) {} // both-error {{expected string literal as argument of 'interrupt' attribute}}
+__attribute__((interrupt("qci-nest", 1))) void foo_nest2(void) {} // both-error {{expected string literal as argument of 'interrupt' attribute}}
+__attribute__((interrupt("qci-est"))) void foo_nest3(void) {} // both-warning {{'interrupt' attribute argument not supported: "qci-est"}}
+__attribute__((interrupt("qci-noest"))) void foo_nonest3(void) {} // both-warning {{'interrupt' attribute argument not supported: "qci-noest"}}
+__attribute__((interrupt("", "qci-nonest"))) void foo_nonest4(void) {} // both-warning {{'interrupt' attribute argument not supported: ""}}
+__attribute__((interrupt("", "qci-nest"))) void foo_nest4(void) {} // both-warning {{'interrupt' attribute argument not supported: ""}}
+
+__attribute__((interrupt("qci-nonest", "qci-nest"))) void foo_nonest5(void) {} // both-error {{RISC-V 'interrupt' attribute contains invalid combination of interrupt types}}
+__attribute__((interrupt("qci-nest", "qci-nonest"))) void foo_nest5(void) {} // both-error {{RISC-V 'interrupt' attribute contains invalid combination of interrupt types}}
+
+__attribute__((interrupt("qci-nest"))) void foo_nest(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nest' requires extension 'Xqciint'}}
+__attribute__((interrupt("qci-nonest"))) void foo_nonest(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nonest' requires extension 'Xqciint'}}
+
+__attribute__((interrupt("qci-nest", "qci-nest"))) void foo_nest_nest(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nest' requires extension 'Xqciint'}}
+__attribute__((interrupt("qci-nonest", "qci-nonest"))) void foo_nonest_nonest(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nonest' requires extension 'Xqciint'}}
+
// This tests the errors for the qci interrupts when using
// `__attribute__((target(...)))` - but they fail on RV64, because you cannot
@@ -44,8 +59,8 @@ __attribute__((target("arch=+xqciint"))) __attribute__((interrupt("qci-nonest"))
// The attribute order is important, the interrupt attribute must come after the
// target attribute
-__attribute__((interrupt("qci-nest"))) __attribute__((target("arch=+xqciint"))) void foo_nest_xqciint2(void) {} // disabled-error {{RISC-V interrupt attribute 'qci-nest' requires extension 'Xqciint'}}
-__attribute__((interrupt("qci-nonest"))) __attribute__((target("arch=+xqciint"))) void foo_nonest_xqciint2(void) {} // disabled-error {{RISC-V interrupt attribute 'qci-nonest' requires extension 'Xqciint'}}
+__attribute__((interrupt("qci-nest"))) __attribute__((target("arch=+xqciint"))) void foo_nest_xqciint2(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nest' requires extension 'Xqciint'}}
+__attribute__((interrupt("qci-nonest"))) __attribute__((target("arch=+xqciint"))) void foo_nonest_xqciint2(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nonest' requires extension 'Xqciint'}}
#endif
#endif
diff...
[truncated]
|
|
@llvm/pr-subscribers-mc Author: Sam Elliott (lenary) ChangesThis Change adds support for two SiFive vendor attributes in clang:
These can be given together, and can be combined with "machine", but These are handled primarily in RISCVFrameLowering:
This Change also adds support for the following two experimental extensions, which only contain CSRs:
The latter is needed for interrupt support. The CFI information for this implementation is not correct, but I'd prefer to correct this in a follow-up. While it's unlikely anyone wants to unwind through a handler, the CFI information is also used by debuggers so it would be good to get it right. Patch is 96.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132481.diff 22 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index b8cd3475bb88a..8f1f5baef24a2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2224,10 +2224,23 @@ def NoMicroMips : InheritableAttr, TargetSpecificAttr<TargetMips32> {
def RISCVInterrupt : InheritableAttr, TargetSpecificAttr<TargetRISCV> {
let Spellings = [GCC<"interrupt">];
let Subjects = SubjectList<[Function]>;
- let Args = [EnumArgument<"Interrupt", "InterruptType", /*is_string=*/true,
- ["supervisor", "machine", "qci-nest", "qci-nonest"],
- ["supervisor", "machine", "qcinest", "qcinonest"],
- 1>];
+ let Args = [VariadicEnumArgument<"Interrupt", "InterruptType", /*is_string=*/true,
+ [
+ "supervisor",
+ "machine",
+ "qci-nest",
+ "qci-nonest",
+ "SiFive-CLIC-preemptible",
+ "SiFive-CLIC-stack-swap",
+ ],
+ [
+ "supervisor",
+ "machine",
+ "qcinest",
+ "qcinonest",
+ "SiFiveCLICPreemptible",
+ "SiFiveCLICStackSwap",
+ ]>];
let ParseKind = "Interrupt";
let Documentation = [RISCVInterruptDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 34e7ff9612859..552cb7f45a7d2 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2863,8 +2863,9 @@ targets. This attribute may be attached to a function definition and instructs
the backend to generate appropriate function entry/exit code so that it can be
used directly as an interrupt service routine.
-Permissible values for this parameter are ``supervisor``, ``machine``,
-``qci-nest`` and ``qci-nonest``. If there is no parameter, then it defaults to
+Permissible values for this parameter are ``machine``, ``supervisor``,
+``qci-nest``, ``qci-nonest``, ``SiFive-CLIC-preemptible``, and
+``SiFive-CLIC-stack-swap``. If there is no parameter, then it defaults to
``machine``.
The ``qci-nest`` and ``qci-nonest`` values require Qualcomm's Xqciint extension
@@ -2875,6 +2876,15 @@ restore interrupt state to the stack -- the ``qci-nest`` value will use
begin the interrupt handler. Both of these will use ``qc.c.mileaveret`` to
restore the state and return to the previous context.
+The ``SiFive-CLIC-preemptible`` and ``SiFive-CLIC-stack-swap`` values are used
+for machine-mode interrupts. For ``SiFive-CLIC-preemptible`` interrupts, the
+values of ``mcause`` and ``mepc`` are saved onto the stack, and interrupts are
+re-enabled. For ``SiFive-CLIC-stack-swap`` interrupts, the stack pointer is
+swapped with ``mscratch`` before its first use and after its last use.
+
+The SiFive CLIC values may be combined with each other and with the ``machine``
+attribute value. Any other combination of different values is not allowed.
+
Repeated interrupt attribute on the same declaration will cause a warning
to be emitted. In case of repeated declarations, the last one prevails.
@@ -2884,6 +2894,7 @@ https://riscv.org/specifications/privileged-isa/
The RISC-V Instruction Set Manual Volume II: Privileged Architecture
Version 1.10.
https://github.com/quic/riscv-unified-db/releases/tag/Xqci-0.7
+https://sifive.cdn.prismic.io/sifive/d1984d2b-c9b9-4c91-8de0-d68a5e64fa0f_sifive-interrupt-cookbook-v1p2.pdf
}];
}
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1536a3b8c920a..0c587d8b677d5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12705,7 +12705,9 @@ def err_riscv_builtin_invalid_lmul : Error<
def err_riscv_type_requires_extension : Error<
"RISC-V type %0 requires the '%1' extension">;
def err_riscv_attribute_interrupt_requires_extension : Error<
- "RISC-V interrupt attribute '%0' requires extension '%1'">;
+ "RISC-V 'interrupt' attribute '%0' requires extension '%1'">;
+def err_riscv_attribute_interrupt_invalid_combination : Error<
+ "RISC-V 'interrupt' attribute contains invalid combination of interrupt types">;
def err_std_source_location_impl_not_found : Error<
"'std::source_location::__impl' was not found; it must be defined before '__builtin_source_location' is called">;
diff --git a/clang/lib/CodeGen/Targets/RISCV.cpp b/clang/lib/CodeGen/Targets/RISCV.cpp
index 5aa10ba41f5ed..14d4cee7c61d3 100644
--- a/clang/lib/CodeGen/Targets/RISCV.cpp
+++ b/clang/lib/CodeGen/Targets/RISCV.cpp
@@ -829,16 +829,39 @@ class RISCVTargetCodeGenInfo : public TargetCodeGenInfo {
if (!Attr)
return;
- const char *Kind;
- switch (Attr->getInterrupt()) {
- case RISCVInterruptAttr::supervisor: Kind = "supervisor"; break;
- case RISCVInterruptAttr::machine: Kind = "machine"; break;
- case RISCVInterruptAttr::qcinest:
- Kind = "qci-nest";
- break;
- case RISCVInterruptAttr::qcinonest:
- Kind = "qci-nonest";
- break;
+ StringRef Kind = "machine";
+ bool HasSiFiveCLICPreemptible = false;
+ bool HasSiFiveCLICStackSwap = false;
+ for (RISCVInterruptAttr::InterruptType type : Attr->interrupt()) {
+ switch (type) {
+ case RISCVInterruptAttr::machine:
+ // Do not update `Kind` because `Kind` is already "machine", or the
+ // kinds also contains SiFive types which need to be applied.
+ break;
+ case RISCVInterruptAttr::supervisor:
+ Kind = "supervisor";
+ break;
+ case RISCVInterruptAttr::qcinest:
+ Kind = "qci-nest";
+ break;
+ case RISCVInterruptAttr::qcinonest:
+ Kind = "qci-nonest";
+ break;
+ // There are three different LLVM IR attribute values for SiFive CLIC
+ // interrupt kinds, one for each kind and one extra for their combination.
+ case RISCVInterruptAttr::SiFiveCLICPreemptible: {
+ HasSiFiveCLICPreemptible = true;
+ Kind = HasSiFiveCLICStackSwap ? "SiFive-CLIC-preemptible-stack-swap"
+ : "SiFive-CLIC-preemptible";
+ break;
+ }
+ case RISCVInterruptAttr::SiFiveCLICStackSwap: {
+ HasSiFiveCLICStackSwap = true;
+ Kind = HasSiFiveCLICPreemptible ? "SiFive-CLIC-preemptible-stack-swap"
+ : "SiFive-CLIC-stack-swap";
+ break;
+ }
+ }
}
Fn->addFnAttr("interrupt", Kind);
diff --git a/clang/lib/Sema/SemaRISCV.cpp b/clang/lib/Sema/SemaRISCV.cpp
index f23827d566610..25ff1e99498ec 100644
--- a/clang/lib/Sema/SemaRISCV.cpp
+++ b/clang/lib/Sema/SemaRISCV.cpp
@@ -13,6 +13,7 @@
#include "clang/Sema/SemaRISCV.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
+#include "clang/AST/Attrs.inc"
#include "clang/AST/Decl.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/TargetBuiltins.h"
@@ -1453,25 +1454,14 @@ void SemaRISCV::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
return;
}
- // Check the attribute argument. Argument is optional.
- if (!AL.checkAtMostNumArgs(SemaRef, 1))
- return;
-
- StringRef Str;
- SourceLocation ArgLoc;
-
- // 'machine'is the default interrupt mode.
- if (AL.getNumArgs() == 0)
- Str = "machine";
- else if (!SemaRef.checkStringLiteralArgumentAttr(AL, 0, Str, &ArgLoc))
- return;
-
// Semantic checks for a function with the 'interrupt' attribute:
// - Must be a function.
// - Must have no parameters.
// - Must have the 'void' return type.
- // - The attribute itself must either have no argument or one of the
- // valid interrupt types, see [RISCVInterruptDocs].
+ // - The attribute itself must have at most 2 arguments
+ // - The attribute arguments must be string literals, and valid choices.
+ // - The attribute arguments must be a valid combination
+ // - The current target must support the right extensions for the combination.
if (D->getFunctionType() == nullptr) {
Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
@@ -1491,35 +1481,105 @@ void SemaRISCV::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
return;
}
- RISCVInterruptAttr::InterruptType Kind;
- if (!RISCVInterruptAttr::ConvertStrToInterruptType(Str, Kind)) {
- Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
- << AL << Str << ArgLoc;
+ if (!AL.checkAtMostNumArgs(SemaRef, 2))
return;
- }
- switch (Kind) {
- default:
- break;
- case RISCVInterruptAttr::InterruptType::qcinest:
- case RISCVInterruptAttr::InterruptType::qcinonest: {
- const TargetInfo &TI = getASTContext().getTargetInfo();
- llvm::StringMap<bool> FunctionFeatureMap;
- getASTContext().getFunctionFeatureMap(FunctionFeatureMap,
- dyn_cast<FunctionDecl>(D));
+ bool HasSiFiveCLICType = false;
+ bool HasUnaryType = false;
+
+ SmallSet<RISCVInterruptAttr::InterruptType, 2> Types;
+ for (unsigned ArgIndex = 0; ArgIndex < AL.getNumArgs(); ++ArgIndex) {
+ RISCVInterruptAttr::InterruptType Type;
+ StringRef TypeString;
+ SourceLocation Loc;
- if (!TI.hasFeature("experimental-xqciint") &&
- !FunctionFeatureMap.lookup("experimental-xqciint")) {
- Diag(AL.getLoc(), diag::err_riscv_attribute_interrupt_requires_extension)
- << Str << "Xqciint";
+ if (!SemaRef.checkStringLiteralArgumentAttr(AL, ArgIndex, TypeString, &Loc))
+ return;
+
+ if (!RISCVInterruptAttr::ConvertStrToInterruptType(TypeString, Type)) {
+ std::string TypeLiteral = ("\"" + TypeString + "\"").str();
+ Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
+ << AL << TypeLiteral << Loc;
return;
}
- break;
+
+ switch (Type) {
+ case RISCVInterruptAttr::machine:
+ // "machine" could be combined with the SiFive CLIC types, or could be
+ // just "machine".
+ break;
+ case RISCVInterruptAttr::SiFiveCLICPreemptible:
+ case RISCVInterruptAttr::SiFiveCLICStackSwap:
+ // SiFive-CLIC types can be combined with each other and "machine"
+ HasSiFiveCLICType = true;
+ break;
+ case RISCVInterruptAttr::supervisor:
+ case RISCVInterruptAttr::qcinest:
+ case RISCVInterruptAttr::qcinonest:
+ // "supervisor" and "qci-(no)nest" cannot be combined with any other types
+ HasUnaryType = true;
+ break;
+ }
+
+ Types.insert(Type);
+ }
+
+ if (HasUnaryType && Types.size() > 1) {
+ Diag(AL.getLoc(), diag::err_riscv_attribute_interrupt_invalid_combination);
+ return;
}
+
+ if (HasUnaryType && HasSiFiveCLICType) {
+ Diag(AL.getLoc(), diag::err_riscv_attribute_interrupt_invalid_combination);
+ return;
+ }
+
+ // "machine" is the default, if nothing is specified.
+ if (AL.getNumArgs() == 0)
+ Types.insert(RISCVInterruptAttr::machine);
+
+ const TargetInfo &TI = getASTContext().getTargetInfo();
+ llvm::StringMap<bool> FunctionFeatureMap;
+ getASTContext().getFunctionFeatureMap(FunctionFeatureMap,
+ dyn_cast<FunctionDecl>(D));
+
+ auto HasFeature = [&](StringRef FeatureName) -> bool {
+ return TI.hasFeature(FeatureName) || FunctionFeatureMap.lookup(FeatureName);
};
- D->addAttr(::new (getASTContext())
- RISCVInterruptAttr(getASTContext(), AL, Kind));
+ for (RISCVInterruptAttr::InterruptType Type : Types) {
+ switch (Type) {
+ // The QCI interrupt types require Xqciint
+ case RISCVInterruptAttr::qcinest:
+ case RISCVInterruptAttr::qcinonest: {
+ if (!HasFeature("experimental-xqciint")) {
+ Diag(AL.getLoc(),
+ diag::err_riscv_attribute_interrupt_requires_extension)
+ << RISCVInterruptAttr::ConvertInterruptTypeToStr(Type) << "Xqciint";
+ return;
+ }
+ } break;
+ // The SiFive CLIC interrupt types require Xsfmclic
+ case RISCVInterruptAttr::SiFiveCLICPreemptible:
+ case RISCVInterruptAttr::SiFiveCLICStackSwap: {
+ if (!HasFeature("experimental-xsfmclic")) {
+ Diag(AL.getLoc(),
+ diag::err_riscv_attribute_interrupt_requires_extension)
+ << RISCVInterruptAttr::ConvertInterruptTypeToStr(Type)
+ << "XSfmclic";
+ return;
+ }
+ } break;
+ default:
+ break;
+ }
+ }
+
+ SmallVector<RISCVInterruptAttr::InterruptType, 2> TypesVec(Types.begin(),
+ Types.end());
+
+ D->addAttr(::new (getASTContext()) RISCVInterruptAttr(
+ getASTContext(), AL, TypesVec.data(), TypesVec.size()));
}
bool SemaRISCV::isAliasValid(unsigned BuiltinID, StringRef AliasName) {
diff --git a/clang/test/Driver/print-supported-extensions-riscv.c b/clang/test/Driver/print-supported-extensions-riscv.c
index 35de2820ef84f..ca695221dee9d 100644
--- a/clang/test/Driver/print-supported-extensions-riscv.c
+++ b/clang/test/Driver/print-supported-extensions-riscv.c
@@ -213,6 +213,8 @@
// CHECK-NEXT: xqcisls 0.2 'Xqcisls' (Qualcomm uC Scaled Load Store Extension)
// CHECK-NEXT: xrivosvisni 0.1 'XRivosVisni' (Rivos Vector Integer Small New)
// CHECK-NEXT: xrivosvizip 0.1 'XRivosVizip' (Rivos Vector Register Zips)
+// CHECK-NEXT: xsfmclic 0.1 'XSfmclic' (SiFive CLIC Machine-mode CSRs)
+// CHECK-NEXT: xsfsclic 0.1 'XSfsclic' (SiFive CLIC Supervisor-mode CSRs)
// CHECK-EMPTY:
// CHECK-NEXT: Supported Profiles
// CHECK-NEXT: rva20s64
diff --git a/clang/test/Sema/riscv-interrupt-attr-qci.c b/clang/test/Sema/riscv-interrupt-attr-qci.c
index bdac4e154bb3c..e54c50c0e25bb 100644
--- a/clang/test/Sema/riscv-interrupt-attr-qci.c
+++ b/clang/test/Sema/riscv-interrupt-attr-qci.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple riscv32-unknown-elf -target-feature +experimental-xqciint -emit-llvm -DCHECK_IR < %s| FileCheck %s
+// RUN: %clang_cc1 -triple riscv32-unknown-elf -target-feature +experimental-xqciint -emit-llvm -DCHECK_IR < %s | FileCheck %s
// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -target-feature +experimental-xqciint -verify=enabled,both -fsyntax-only
// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify=disabled,both -fsyntax-only
// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -target-feature -experimental-xqciint -verify=disabled,both -fsyntax-only
@@ -11,10 +11,20 @@
__attribute__((interrupt("qci-nest")))
void foo_nest_interrupt(void) {}
-// CHECK-LABEL: @foo_nonnest_interrupt() #1
+// CHECK-LABEL: @foo_nest_nest_interrupt() #0
+// CHECK: ret void
+__attribute__((interrupt("qci-nest", "qci-nest")))
+void foo_nest_nest_interrupt(void) {}
+
+// CHECK-LABEL: @foo_nonest_interrupt() #1
// CHECK: ret void
__attribute__((interrupt("qci-nonest")))
-void foo_nonnest_interrupt(void) {}
+void foo_nonest_interrupt(void) {}
+
+// CHECK-LABEL: @foo_nonest_nonest_interrupt() #1
+// CHECK: ret void
+__attribute__((interrupt("qci-nonest", "qci-nonest")))
+void foo_nonest_nonest_interrupt(void) {}
// CHECK: attributes #0
// CHECK: "interrupt"="qci-nest"
@@ -22,18 +32,23 @@ void foo_nonnest_interrupt(void) {}
// CHECK: "interrupt"="qci-nonest"
#else
// Test for QCI extension's interrupt attribute support
-__attribute__((interrupt("qci-est"))) void foo_nest1(void) {} // both-warning {{'interrupt' attribute argument not supported: qci-est}}
-__attribute__((interrupt("qci-noest"))) void foo_nonest1(void) {} // both-warning {{'interrupt' attribute argument not supported: qci-noest}}
-__attribute__((interrupt(1))) void foo_nest2(void) {} // both-error {{expected string literal as argument of 'interrupt' attribute}}
-__attribute__((interrupt("qci-nest", "qci-nonest"))) void foo1(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("qci-nonest", "qci-nest"))) void foo2(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("", "qci-nonest"))) void foo3(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("", "qci-nest"))) void foo4(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("qci-nonest", 1))) void foo5(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-__attribute__((interrupt("qci-nest", 1))) void foo6(void) {} // both-error {{'interrupt' attribute takes no more than 1 argument}}
-
-__attribute__((interrupt("qci-nest"))) void foo_nest(void) {} // disabled-error {{RISC-V interrupt attribute 'qci-nest' requires extension 'Xqciint'}}
-__attribute__((interrupt("qci-nonest"))) void foo_nonest(void) {} // disabled-error {{RISC-V interrupt attribute 'qci-nonest' requires extension 'Xqciint'}}
+__attribute__((interrupt(1))) void foo1(void) {} // both-error {{expected string literal as argument of 'interrupt' attribute}}
+__attribute__((interrupt("qci-nonest", 1))) void foo_nonest2(void) {} // both-error {{expected string literal as argument of 'interrupt' attribute}}
+__attribute__((interrupt("qci-nest", 1))) void foo_nest2(void) {} // both-error {{expected string literal as argument of 'interrupt' attribute}}
+__attribute__((interrupt("qci-est"))) void foo_nest3(void) {} // both-warning {{'interrupt' attribute argument not supported: "qci-est"}}
+__attribute__((interrupt("qci-noest"))) void foo_nonest3(void) {} // both-warning {{'interrupt' attribute argument not supported: "qci-noest"}}
+__attribute__((interrupt("", "qci-nonest"))) void foo_nonest4(void) {} // both-warning {{'interrupt' attribute argument not supported: ""}}
+__attribute__((interrupt("", "qci-nest"))) void foo_nest4(void) {} // both-warning {{'interrupt' attribute argument not supported: ""}}
+
+__attribute__((interrupt("qci-nonest", "qci-nest"))) void foo_nonest5(void) {} // both-error {{RISC-V 'interrupt' attribute contains invalid combination of interrupt types}}
+__attribute__((interrupt("qci-nest", "qci-nonest"))) void foo_nest5(void) {} // both-error {{RISC-V 'interrupt' attribute contains invalid combination of interrupt types}}
+
+__attribute__((interrupt("qci-nest"))) void foo_nest(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nest' requires extension 'Xqciint'}}
+__attribute__((interrupt("qci-nonest"))) void foo_nonest(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nonest' requires extension 'Xqciint'}}
+
+__attribute__((interrupt("qci-nest", "qci-nest"))) void foo_nest_nest(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nest' requires extension 'Xqciint'}}
+__attribute__((interrupt("qci-nonest", "qci-nonest"))) void foo_nonest_nonest(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nonest' requires extension 'Xqciint'}}
+
// This tests the errors for the qci interrupts when using
// `__attribute__((target(...)))` - but they fail on RV64, because you cannot
@@ -44,8 +59,8 @@ __attribute__((target("arch=+xqciint"))) __attribute__((interrupt("qci-nonest"))
// The attribute order is important, the interrupt attribute must come after the
// target attribute
-__attribute__((interrupt("qci-nest"))) __attribute__((target("arch=+xqciint"))) void foo_nest_xqciint2(void) {} // disabled-error {{RISC-V interrupt attribute 'qci-nest' requires extension 'Xqciint'}}
-__attribute__((interrupt("qci-nonest"))) __attribute__((target("arch=+xqciint"))) void foo_nonest_xqciint2(void) {} // disabled-error {{RISC-V interrupt attribute 'qci-nonest' requires extension 'Xqciint'}}
+__attribute__((interrupt("qci-nest"))) __attribute__((target("arch=+xqciint"))) void foo_nest_xqciint2(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nest' requires extension 'Xqciint'}}
+__attribute__((interrupt("qci-nonest"))) __attribute__((target("arch=+xqciint"))) void foo_nonest_xqciint2(void) {} // disabled-error {{RISC-V 'interrupt' attribute 'qci-nonest' requires extension 'Xqciint'}}
#endif
#endif
diff...
[truncated]
|
|
Is there a reason why these aren't lowercase like the rest? |
They match the existing names documented in https://starfivetech.com/uploads/sifive-interrupt-cookbook-v1p2.pdf - we could check them in clang case-insensitively, though. |
|
For Info, the last time this was proposed was in 2020: https://reviews.llvm.org/D79521 - at that time, no vendor extensions had been accepted upstream, and there was not yet a policy for upstream supporting vendor extensions. |
topperc
reviewed
Mar 21, 2025
topperc
reviewed
Mar 21, 2025
kito-cheng
reviewed
Mar 27, 2025
Comment on lines
490
to
505
| let FeaturesRequired = [{ {RISCV::FeatureVendorXSfmclic} }] in { | ||
| def : SysReg<"mtvt", 0x307>; | ||
| def : SysReg<"mnxti", 0x345>; | ||
| def : SysReg<"mintstatus", 0x346>; | ||
| def : SysReg<"mscratchcsw", 0x348>; | ||
| def : SysReg<"mscratchcswl", 0x349>; | ||
| } | ||
|
|
||
| // XSfsclic | ||
| let FeaturesRequired = [{ {RISCV::FeatureVendorXSfsclic} }] in { | ||
| def : SysReg<"stvt", 0x107>; | ||
| def : SysReg<"snxti", 0x145>; | ||
| def : SysReg<"sintstatus", 0x146>; | ||
| def : SysReg<"sscratchcsw", 0x148>; | ||
| def : SysReg<"sscratchcswl", 0x149>; | ||
| } |
There was a problem hiding this comment.
I am little hesitate here, we should have sf. prefix here in theory....
There was a problem hiding this comment.
I'm happy to add this, I didn't because I presumed there exists legacy code which uses these names. If you want, I could add the prefixes to these, and then add a copy as deprecated aliases?
There was a problem hiding this comment.
From the call, I think we've concluded to go ahead with prefixed versions at least in this PR. We can loop back to discuss separately if there's demand to support the unprefixed form, but ideally we can do sf.* and if people need the old version for downstream users they can carry the alias downstream.
There was a problem hiding this comment.
We discussed this on the RISC-V LLVM Sync-up today. I will prefix these, and not add deprecated aliases.
|
@kito-cheng sorted, we now hard error (without a crash trace) if you try to combine frame pointers and preemptible interrupts |
|
I've resolved the merge conflict so I can merge this, now I'm back at work after some travel. I'll let some pre-commit checks run before I finally hit merge though. |
topperc
reviewed
Apr 25, 2025
| .setMIFlag(MachineInstr::FrameSetup); | ||
| BuildMI(MBB, MBBI, DL, TII->get(RISCV::CSRRS)) | ||
| .addReg(RISCV::X9, RegState::Define) | ||
| .addImm(RISCVSysReg::lookupSysRegByName("MEPC")->Encoding) |
There was a problem hiding this comment.
Can we use RISCVSysReg::mepc here to avoid the table lookup?
There was a problem hiding this comment.
Oh, I thought I had got these. Yes definitely.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/17991 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/187/builds/5614 Here is the relevant piece of the build log for the reference |
jyli0116
pushed a commit
to jyli0116/llvm-project
that referenced
this pull request
Apr 28, 2025
This Change adds support for two SiFive vendor attributes in clang: - "SiFive-CLIC-preemptible" - "SiFive-CLIC-stack-swap" These can be given together, and can be combined with "machine", but cannot be combined with any other interrupt attribute values. These are handled primarily in RISCVFrameLowering: - "SiFive-CLIC-stack-swap" entails swapping `sp` with `sf.mscratchcsw` at function entry and exit, which holds the trap stack pointer. - "SiFive-CLIC-preemptible" entails saving `mcause` and `mepc` before re-enabling interrupts using `mstatus`. To save these, `s0` and `s1` are first spilled to the stack, and then the values are read into these registers. If these registers are used in the function, their values will be spilled a second time onto the stack with the generic callee-saved-register handling. At the end of the function interrupts are disabled again before `mepc` and `mcause` are restored. This Change also adds support for the following two experimental extensions, which only contain CSRs: - XSfsclic - for SiFive's CLIC Supervisor-Mode CSRs - XSfmclic - for SiFive's CLIC Machine-Mode CSRs The latter is needed for interrupt support. The CFI information for this implementation is not correct, but I'd prefer to correct this in a follow-up. While it's unlikely anyone wants to unwind through a handler, the CFI information is also used by debuggers so it would be good to get it right. Co-authored-by: Ana Pazos <[email protected]>
|
@lenary, |
|
Sorry! I will look at it today. I'll push an xfail in the meantime, to get it green again. |
|
I pushed an xfail as |
|
Thank you @lenary! |
lenary
added a commit
to lenary/llvm-project
that referenced
this pull request
Apr 28, 2025
The expensive checks bots found issues with llvm#132481, due to not setting defs/uses correctly. In 31bd7a5 I added verify flags, so that the failure is reproduced without requiring expensive checks, and xfailed the test. This change: - Ensures that registers are correctly marked as defs/uses. - Removes the xfail. - Leaves the tests with `-verify-machineinstrs` which should have been present originally.
lenary
added a commit
that referenced
this pull request
Apr 29, 2025
The expensive checks bots found issues with #132481, due to not setting defs/uses correctly. In 31bd7a5 I added verify flags, so that the failure is reproduced without requiring expensive checks, and xfailed the test. This change: - Ensures that registers are correctly marked as defs/uses. - Removes the xfail. - Leaves the tests with `-verify-machineinstrs` which should have been present originally.
gizmondo
pushed a commit
to gizmondo/llvm-project
that referenced
this pull request
Apr 29, 2025
The expensive checks bots found issues with llvm#132481, due to not setting defs/uses correctly. In 31bd7a5 I added verify flags, so that the failure is reproduced without requiring expensive checks, and xfailed the test. This change: - Ensures that registers are correctly marked as defs/uses. - Removes the xfail. - Leaves the tests with `-verify-machineinstrs` which should have been present originally.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This Change adds support for two SiFive vendor attributes in clang: - "SiFive-CLIC-preemptible" - "SiFive-CLIC-stack-swap" These can be given together, and can be combined with "machine", but cannot be combined with any other interrupt attribute values. These are handled primarily in RISCVFrameLowering: - "SiFive-CLIC-stack-swap" entails swapping `sp` with `sf.mscratchcsw` at function entry and exit, which holds the trap stack pointer. - "SiFive-CLIC-preemptible" entails saving `mcause` and `mepc` before re-enabling interrupts using `mstatus`. To save these, `s0` and `s1` are first spilled to the stack, and then the values are read into these registers. If these registers are used in the function, their values will be spilled a second time onto the stack with the generic callee-saved-register handling. At the end of the function interrupts are disabled again before `mepc` and `mcause` are restored. This Change also adds support for the following two experimental extensions, which only contain CSRs: - XSfsclic - for SiFive's CLIC Supervisor-Mode CSRs - XSfmclic - for SiFive's CLIC Machine-Mode CSRs The latter is needed for interrupt support. The CFI information for this implementation is not correct, but I'd prefer to correct this in a follow-up. While it's unlikely anyone wants to unwind through a handler, the CFI information is also used by debuggers so it would be good to get it right. Co-authored-by: Ana Pazos <[email protected]>
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
The expensive checks bots found issues with llvm#132481, due to not setting defs/uses correctly. In 31bd7a5 I added verify flags, so that the failure is reproduced without requiring expensive checks, and xfailed the test. This change: - Ensures that registers are correctly marked as defs/uses. - Removes the xfail. - Leaves the tests with `-verify-machineinstrs` which should have been present originally.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This Change adds support for two SiFive vendor attributes in clang: - "SiFive-CLIC-preemptible" - "SiFive-CLIC-stack-swap" These can be given together, and can be combined with "machine", but cannot be combined with any other interrupt attribute values. These are handled primarily in RISCVFrameLowering: - "SiFive-CLIC-stack-swap" entails swapping `sp` with `sf.mscratchcsw` at function entry and exit, which holds the trap stack pointer. - "SiFive-CLIC-preemptible" entails saving `mcause` and `mepc` before re-enabling interrupts using `mstatus`. To save these, `s0` and `s1` are first spilled to the stack, and then the values are read into these registers. If these registers are used in the function, their values will be spilled a second time onto the stack with the generic callee-saved-register handling. At the end of the function interrupts are disabled again before `mepc` and `mcause` are restored. This Change also adds support for the following two experimental extensions, which only contain CSRs: - XSfsclic - for SiFive's CLIC Supervisor-Mode CSRs - XSfmclic - for SiFive's CLIC Machine-Mode CSRs The latter is needed for interrupt support. The CFI information for this implementation is not correct, but I'd prefer to correct this in a follow-up. While it's unlikely anyone wants to unwind through a handler, the CFI information is also used by debuggers so it would be good to get it right. Co-authored-by: Ana Pazos <[email protected]>
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
The expensive checks bots found issues with llvm#132481, due to not setting defs/uses correctly. In 31bd7a5 I added verify flags, so that the failure is reproduced without requiring expensive checks, and xfailed the test. This change: - Ensures that registers are correctly marked as defs/uses. - Removes the xfail. - Leaves the tests with `-verify-machineinstrs` which should have been present originally.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This Change adds support for two SiFive vendor attributes in clang: - "SiFive-CLIC-preemptible" - "SiFive-CLIC-stack-swap" These can be given together, and can be combined with "machine", but cannot be combined with any other interrupt attribute values. These are handled primarily in RISCVFrameLowering: - "SiFive-CLIC-stack-swap" entails swapping `sp` with `sf.mscratchcsw` at function entry and exit, which holds the trap stack pointer. - "SiFive-CLIC-preemptible" entails saving `mcause` and `mepc` before re-enabling interrupts using `mstatus`. To save these, `s0` and `s1` are first spilled to the stack, and then the values are read into these registers. If these registers are used in the function, their values will be spilled a second time onto the stack with the generic callee-saved-register handling. At the end of the function interrupts are disabled again before `mepc` and `mcause` are restored. This Change also adds support for the following two experimental extensions, which only contain CSRs: - XSfsclic - for SiFive's CLIC Supervisor-Mode CSRs - XSfmclic - for SiFive's CLIC Machine-Mode CSRs The latter is needed for interrupt support. The CFI information for this implementation is not correct, but I'd prefer to correct this in a follow-up. While it's unlikely anyone wants to unwind through a handler, the CFI information is also used by debuggers so it would be good to get it right. Co-authored-by: Ana Pazos <[email protected]>
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
The expensive checks bots found issues with llvm#132481, due to not setting defs/uses correctly. In 31bd7a5 I added verify flags, so that the failure is reproduced without requiring expensive checks, and xfailed the test. This change: - Ensures that registers are correctly marked as defs/uses. - Removes the xfail. - Leaves the tests with `-verify-machineinstrs` which should have been present originally.
GeorgeARM
pushed a commit
to GeorgeARM/llvm-project
that referenced
this pull request
May 7, 2025
The expensive checks bots found issues with llvm#132481, due to not setting defs/uses correctly. In 31bd7a5 I added verify flags, so that the failure is reproduced without requiring expensive checks, and xfailed the test. This change: - Ensures that registers are correctly marked as defs/uses. - Removes the xfail. - Leaves the tests with `-verify-machineinstrs` which should have been present originally.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
This Change adds support for two SiFive vendor attributes in clang: - "SiFive-CLIC-preemptible" - "SiFive-CLIC-stack-swap" These can be given together, and can be combined with "machine", but cannot be combined with any other interrupt attribute values. These are handled primarily in RISCVFrameLowering: - "SiFive-CLIC-stack-swap" entails swapping `sp` with `sf.mscratchcsw` at function entry and exit, which holds the trap stack pointer. - "SiFive-CLIC-preemptible" entails saving `mcause` and `mepc` before re-enabling interrupts using `mstatus`. To save these, `s0` and `s1` are first spilled to the stack, and then the values are read into these registers. If these registers are used in the function, their values will be spilled a second time onto the stack with the generic callee-saved-register handling. At the end of the function interrupts are disabled again before `mepc` and `mcause` are restored. This Change also adds support for the following two experimental extensions, which only contain CSRs: - XSfsclic - for SiFive's CLIC Supervisor-Mode CSRs - XSfmclic - for SiFive's CLIC Machine-Mode CSRs The latter is needed for interrupt support. The CFI information for this implementation is not correct, but I'd prefer to correct this in a follow-up. While it's unlikely anyone wants to unwind through a handler, the CFI information is also used by debuggers so it would be good to get it right. Co-authored-by: Ana Pazos <[email protected]>
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
The expensive checks bots found issues with llvm#132481, due to not setting defs/uses correctly. In 31bd7a5 I added verify flags, so that the failure is reproduced without requiring expensive checks, and xfailed the test. This change: - Ensures that registers are correctly marked as defs/uses. - Removes the xfail. - Leaves the tests with `-verify-machineinstrs` which should have been present originally.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This Change adds support for two SiFive vendor attributes in clang:
These can be given together, and can be combined with "machine", but
cannot be combined with any other interrupt attribute values.
These are handled primarily in RISCVFrameLowering:
spwithsf.mscratchcswat function entry and exit, which holds the trap stack pointer.
mcauseandmepcbeforere-enabling interrupts using
mstatus. To save these,s0ands1are first spilled to the stack, and then the values are read into
these registers. If these registers are used in the function, their
values will be spilled a second time onto the stack with the generic
callee-saved-register handling. At the end of the function interrupts
are disabled again before
mepcandmcauseare restored.This Change also adds support for the following two experimental extensions, which only contain CSRs:
The latter is needed for interrupt support.
The CFI information for this implementation is not correct, but I'd prefer to correct this in a follow-up. While it's unlikely anyone wants to unwind through a handler, the CFI information is also used by debuggers so it would be good to get it right.