https://github.com/sdesmalen-arm updated
https://github.com/llvm/llvm-project/pull/109420
>From af4cd0b3643e682fcb34042d209df03037743eb0 Mon Sep 17 00:00:00 2001
From: Sander de Smalen <sander.desma...@arm.com>
Date: Fri, 20 Sep 2024 14:16:23 +0100
Subject: [PATCH 1/2] [Clang][AArch64] Fix checkArmStreamingBuiltin for
'sve-b16b16'
The implementation made the assumption that any feature starting with
"sve" meant that this was an SVE feature. This is not the case for
"sve-b16b16", as this is a feature that applies to both SVE and SME.
This meant that:
__attribute__((target("+sme2,+sve2,+sve-b16b16")))
svbfloat16_t foo(svbfloat16_t a, svbfloat16_t b, svbfloat16_t c)
__arm_streaming {
return svclamp_bf16(a, b, c);
}
would result in an incorrect diagnostic saying that `svclamp_bf16`
could only be used in non-streaming functions.
---
clang/lib/Sema/SemaARM.cpp | 21 ++++++++++++-------
...reaming-sme-or-nonstreaming-sve-builtins.c | 6 ++++++
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp
index efde354860de43..fba1453e5d38fc 100644
--- a/clang/lib/Sema/SemaARM.cpp
+++ b/clang/lib/Sema/SemaARM.cpp
@@ -567,15 +567,18 @@ static bool checkArmStreamingBuiltin(Sema &S, CallExpr
*TheCall,
// * When compiling for SVE only, the caller must be in non-streaming mode.
// * When compiling for both SVE and SME, the caller can be in either mode.
if (BuiltinType == SemaARM::VerifyRuntimeMode) {
- auto DisableFeatures = [](llvm::StringMap<bool> &Map, StringRef S) {
- for (StringRef K : Map.keys())
- if (K.starts_with(S))
- Map[K] = false;
- };
-
llvm::StringMap<bool> CallerFeatureMapWithoutSVE;
S.Context.getFunctionFeatureMap(CallerFeatureMapWithoutSVE, FD);
- DisableFeatures(CallerFeatureMapWithoutSVE, "sve");
+ CallerFeatureMapWithoutSVE["sve"] = false;
+ CallerFeatureMapWithoutSVE["sve2"] = false;
+ CallerFeatureMapWithoutSVE["sve2p1"] = false;
+ // FIXME: This list must be updated with future extensions, because when
+ // an intrinsic is enabled by (sve2p1|sme2p1), disabling just "sve" is
+ // not sufficient, as the feature dependences are not resolved.
+ // At the moment, it should be sufficient to test the 'base' architectural
+ // support for SVE and SME, which must always be provided in the
+ // target guard. e.g. TargetGuard = "sve-b16b16" without "sme" or "sve"
+ // is not sufficient.
// Avoid emitting diagnostics for a function that can never compile.
if (FnType == SemaARM::ArmStreaming && !CallerFeatureMapWithoutSVE["sme"])
@@ -583,7 +586,9 @@ static bool checkArmStreamingBuiltin(Sema &S, CallExpr
*TheCall,
llvm::StringMap<bool> CallerFeatureMapWithoutSME;
S.Context.getFunctionFeatureMap(CallerFeatureMapWithoutSME, FD);
- DisableFeatures(CallerFeatureMapWithoutSME, "sme");
+ CallerFeatureMapWithoutSME["sme"] = false;
+ CallerFeatureMapWithoutSME["sme2"] = false;
+ CallerFeatureMapWithoutSME["sme2p1"] = false;
// We know the builtin requires either some combination of SVE flags, or
// some combination of SME flags, but we need to figure out which part
diff --git
a/clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c
b/clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c
index 45776eb13e4fbc..792d79ee3e600d 100644
--- a/clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c
+++ b/clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c
@@ -38,6 +38,12 @@ svfloat32_t good6(svfloat32_t a, svfloat32_t b, svfloat32_t
c) __arm_streaming_c
return svclamp(a, b, c);
}
+// Test that the +sve-b16b16 is not considered an SVE flag (it applies to both)
+__attribute__((target("+sme2,+sve2,+sve-b16b16")))
+svbfloat16_t good7(svbfloat16_t a, svbfloat16_t b, svbfloat16_t c)
__arm_streaming {
+ return svclamp_bf16(a, b, c);
+}
+
// Without '+sme2', the builtin is only valid in non-streaming mode.
__attribute__((target("+sve2p1,+sme")))
svfloat32_t bad1(svfloat32_t a, svfloat32_t b, svfloat32_t c) __arm_streaming {
>From bfa1348e06a78c2cc30f2cf7e64ae993191fcd2d Mon Sep 17 00:00:00 2001
From: Sander de Smalen <sander.desma...@arm.com>
Date: Tue, 24 Sep 2024 07:23:37 +0000
Subject: [PATCH 2/2] Add TableGen checks
---
clang/lib/Sema/SemaARM.cpp | 20 +++-----
clang/utils/TableGen/SveEmitter.cpp | 76 ++++++++++++++++++++++++++++-
2 files changed, 81 insertions(+), 15 deletions(-)
diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp
index fba1453e5d38fc..de2236207564bd 100644
--- a/clang/lib/Sema/SemaARM.cpp
+++ b/clang/lib/Sema/SemaARM.cpp
@@ -569,16 +569,9 @@ static bool checkArmStreamingBuiltin(Sema &S, CallExpr
*TheCall,
if (BuiltinType == SemaARM::VerifyRuntimeMode) {
llvm::StringMap<bool> CallerFeatureMapWithoutSVE;
S.Context.getFunctionFeatureMap(CallerFeatureMapWithoutSVE, FD);
- CallerFeatureMapWithoutSVE["sve"] = false;
- CallerFeatureMapWithoutSVE["sve2"] = false;
- CallerFeatureMapWithoutSVE["sve2p1"] = false;
- // FIXME: This list must be updated with future extensions, because when
- // an intrinsic is enabled by (sve2p1|sme2p1), disabling just "sve" is
- // not sufficient, as the feature dependences are not resolved.
- // At the moment, it should be sufficient to test the 'base' architectural
- // support for SVE and SME, which must always be provided in the
- // target guard. e.g. TargetGuard = "sve-b16b16" without "sme" or "sve"
- // is not sufficient.
+ for (StringRef Feat : {"sve", "sve2", "sve2p1", "sve2-aes", "sve2-sha3",
+ "sve2-sm4", "sve2-bitperm"})
+ CallerFeatureMapWithoutSVE[Feat] = false;
// Avoid emitting diagnostics for a function that can never compile.
if (FnType == SemaARM::ArmStreaming && !CallerFeatureMapWithoutSVE["sme"])
@@ -586,9 +579,10 @@ static bool checkArmStreamingBuiltin(Sema &S, CallExpr
*TheCall,
llvm::StringMap<bool> CallerFeatureMapWithoutSME;
S.Context.getFunctionFeatureMap(CallerFeatureMapWithoutSME, FD);
- CallerFeatureMapWithoutSME["sme"] = false;
- CallerFeatureMapWithoutSME["sme2"] = false;
- CallerFeatureMapWithoutSME["sme2p1"] = false;
+ for (StringRef Feat :
+ {"sme", "sme2", "sme2p1", "sme-f64f64", "sme-i16i64", "sme-b16b16",
+ "sme-f16f16", "sme-f8f32", "sme-f8f16"})
+ CallerFeatureMapWithoutSME[Feat] = false;
// We know the builtin requires either some combination of SVE flags, or
// some combination of SME flags, but we need to figure out which part
diff --git a/clang/utils/TableGen/SveEmitter.cpp
b/clang/utils/TableGen/SveEmitter.cpp
index 2f9747e7de3de2..8fdbd49655d4fe 100644
--- a/clang/utils/TableGen/SveEmitter.cpp
+++ b/clang/utils/TableGen/SveEmitter.cpp
@@ -1770,6 +1770,58 @@ void SVEEmitter::createBuiltinZAState(raw_ostream &OS) {
OS << "#endif\n\n";
}
+static StringRef parseGuardParenExpr(StringRef &S) {
+ unsigned N = 0;
+ assert(S[0] == '(' && "Expected lparen");
+ for (unsigned I = 0; I < S.size(); ++I) {
+ if (S[I] == '(')
+ ++N;
+ else if (S[I] == ')')
+ --N;
+ if (N == 0) {
+ StringRef Expr = S.substr(1, I - 1);
+ S = S.drop_front(I + 1);
+ return Expr;
+ }
+ }
+ llvm_unreachable("Unmatched parenthesi");
+}
+
+static StringRef parseGuardFeature(StringRef &S) {
+ assert(std::isalpha(S[0]) && "expected feature name");
+ unsigned I;
+ for (I = 0; I < S.size(); ++I) {
+ if (S[I] == ',' || S[I] == '|' || S[I] == ')')
+ break;
+ }
+ StringRef Expr = S.take_front(I);
+ S = S.drop_front(I);
+ return Expr;
+}
+
+static StringRef parseGuardExpr(StringRef &S) {
+ if (S[0] == '(')
+ return parseGuardParenExpr(S);
+ if (std::isalpha(S[0]))
+ return parseGuardFeature(S);
+ llvm_unreachable("Unexpected token in expression");
+}
+
+// Parse the TargetGuard and verify that it satisfies at least one of the
+// features from the Required list.
+static bool verifyGuard(StringRef S, ArrayRef<StringRef> Required) {
+ if (S.empty())
+ return false;
+ StringRef LHS = parseGuardExpr(S);
+ if (S.empty())
+ return llvm::any_of(Required, [LHS](StringRef R) { return R == LHS; });
+ if (S[0] == '|')
+ return verifyGuard(LHS, Required) && verifyGuard(S.drop_front(1),
Required);
+ if (S[0] == ',')
+ return verifyGuard(LHS, Required) || verifyGuard(S.drop_front(1),
Required);
+ llvm_unreachable("Unexpected token in expression");
+}
+
void SVEEmitter::createStreamingAttrs(raw_ostream &OS, ACLEKind Kind) {
std::vector<const Record *> RV = Records.getAllDerivedDefinitions("Inst");
SmallVector<std::unique_ptr<Intrinsic>, 128> Defs;
@@ -1802,9 +1854,29 @@ void SVEEmitter::createStreamingAttrs(raw_ostream &OS,
ACLEKind Kind) {
if (Def->isFlagSet(IsStreamingFlag))
StreamingMap["ArmStreaming"].insert(Def->getMangledName());
- else if (Def->isFlagSet(VerifyRuntimeMode))
+ else if (Def->isFlagSet(VerifyRuntimeMode)) {
+ // Verify that the target guards contain at least one feature that
+ // actually enables SVE or SME (explicitly, or implicitly). This is
needed
+ // for the code in SemaARM.cpp (checkArmStreamingBuiltin) that checks
+ // whether the required runtime mode for an intrinsic matches with the
+ // given set of target features and function attributes.
+ //
+ // The feature lists below must match the disabled features in
+ // 'checkArmStreamingBuiltin'!
+ if (!Def->getSVEGuard().empty() &&
+ !verifyGuard(Def->getSVEGuard(),
+ {"sve", "sve2", "sve2p1", "sve2-aes", "sve2-sha3",
+ "sve2-sm4", "sve2-bitperm"}))
+ llvm_unreachable(
+ "SVE guard must include at least one base SVE version");
+ if (!Def->getSMEGuard().empty() &&
+ !verifyGuard(Def->getSMEGuard(),
+ {"sme", "sme2", "sme2p1", "sme-f64f64", "sme-i16i64",
+ "sme-b16b16", "sme-f16f16", "sme-f8f32", "sme-f8f16"}))
+ llvm_unreachable(
+ "SME guard must include at least one base SME version");
StreamingMap["VerifyRuntimeMode"].insert(Def->getMangledName());
- else if (Def->isFlagSet(IsStreamingCompatibleFlag))
+ } else if (Def->isFlagSet(IsStreamingCompatibleFlag))
StreamingMap["ArmStreamingCompatible"].insert(Def->getMangledName());
else
StreamingMap["ArmNonStreaming"].insert(Def->getMangledName());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
