sdesmalen updated this revision to Diff 547933.
sdesmalen marked 2 inline comments as done.
sdesmalen added a comment.
Removed double error message for missing 'sme' for both ZA and Streaming
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157269/new/
https://reviews.llvm.org/D157269
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDecl.cpp
clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp
llvm/lib/Target/AArch64/AArch64SMEInstrInfo.td
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
llvm/test/CodeGen/AArch64/sme-call-streaming-compatible-to-normal-fn-wihout-sme-attr.ll
Index: llvm/test/CodeGen/AArch64/sme-call-streaming-compatible-to-normal-fn-wihout-sme-attr.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sme-call-streaming-compatible-to-normal-fn-wihout-sme-attr.ll
@@ -0,0 +1,41 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc < %s | FileCheck %s
+
+; This that the following code can be compiled without +sme, because if the
+; call is not entered in streaming-SVE mode at runtime, the codepath leading
+; to the smstop/smstart pair will not be executed either.
+
+target triple = "aarch64"
+
+define void @streaming_compatible() #0 {
+; CHECK-LABEL: streaming_compatible:
+; CHECK: // %bb.0:
+; CHECK-NEXT: stp d15, d14, [sp, #-80]! // 16-byte Folded Spill
+; CHECK-NEXT: stp d13, d12, [sp, #16] // 16-byte Folded Spill
+; CHECK-NEXT: stp d11, d10, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT: stp d9, d8, [sp, #48] // 16-byte Folded Spill
+; CHECK-NEXT: stp x30, x19, [sp, #64] // 16-byte Folded Spill
+; CHECK-NEXT: bl __arm_sme_state
+; CHECK-NEXT: and x19, x0, #0x1
+; CHECK-NEXT: tbz x19, #0, .LBB0_2
+; CHECK-NEXT: // %bb.1:
+; CHECK-NEXT: msr SVCRSM, #0
+; CHECK-NEXT: .LBB0_2:
+; CHECK-NEXT: bl non_streaming
+; CHECK-NEXT: tbz x19, #0, .LBB0_4
+; CHECK-NEXT: // %bb.3:
+; CHECK-NEXT: msr SVCRSM, #1
+; CHECK-NEXT: .LBB0_4:
+; CHECK-NEXT: ldp x30, x19, [sp, #64] // 16-byte Folded Reload
+; CHECK-NEXT: ldp d9, d8, [sp, #48] // 16-byte Folded Reload
+; CHECK-NEXT: ldp d11, d10, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT: ldp d13, d12, [sp, #16] // 16-byte Folded Reload
+; CHECK-NEXT: ldp d15, d14, [sp], #80 // 16-byte Folded Reload
+; CHECK-NEXT: ret
+ call void @non_streaming()
+ ret void
+}
+
+declare void @non_streaming()
+
+attributes #0 = { nounwind "aarch64_pstate_sm_compatible" }
Index: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
===================================================================
--- llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -442,8 +442,6 @@
assert((!StreamingSVEMode || I->hasSME()) &&
"Expected SME to be available");
- assert((!StreamingCompatibleSVEMode || I->hasSVEorSME()) &&
- "Expected SVE or SME to be available");
return I.get();
}
Index: llvm/lib/Target/AArch64/AArch64SMEInstrInfo.td
===================================================================
--- llvm/lib/Target/AArch64/AArch64SMEInstrInfo.td
+++ llvm/lib/Target/AArch64/AArch64SMEInstrInfo.td
@@ -154,6 +154,12 @@
let Inst{11-9} = pstatefield;
let Inst{8} = imm;
let Inst{7-5} = 0b011; // op2
+
+ // We shouldn't require 'sme' to compile streaming-compatible functions which
+ // call non-streaming functions as the SME smstart/smstop will only be
+ // executed at runtime if streaming-mode is enabled, which also means SME must
+ // be enabled.
+ let Predicates = [];
}
def : InstAlias<"smstart", (MSRpstatesvcrImm1 0b011, 0b1)>;
@@ -226,12 +232,6 @@
def : Pat<(AArch64_smstop (i32 svcr_op:$pstate), (i64 0), (i64 1)), // after call
(MSRpstatesvcrImm1 svcr_op:$pstate, 0b0)>;
-// The generic case which gets expanded to a pseudo node.
-def : Pat<(AArch64_smstart (i32 svcr_op:$pstate), (i64 GPR64:$rtpstate), (i64 timm0_1:$expected_pstate)),
- (MSRpstatePseudo svcr_op:$pstate, 0b1, GPR64:$rtpstate, timm0_1:$expected_pstate)>;
-def : Pat<(AArch64_smstop (i32 svcr_op:$pstate), (i64 GPR64:$rtpstate), (i64 timm0_1:$expected_pstate)),
- (MSRpstatePseudo svcr_op:$pstate, 0b0, GPR64:$rtpstate, timm0_1:$expected_pstate)>;
-
// Read and write TPIDR2_EL0
def : Pat<(int_aarch64_sme_set_tpidr2 i64:$val),
(MSR 0xde85, GPR64:$val)>;
@@ -243,6 +243,12 @@
(OBSCURE_COPY GPR64:$idx)>;
} // End let Predicates = [HasSME]
+// The generic case which gets expanded to a pseudo node (doesn't require SME or SVE)
+def : Pat<(AArch64_smstart (i32 svcr_op:$pstate), (i64 GPR64:$rtpstate), (i64 timm0_1:$expected_pstate)),
+ (MSRpstatePseudo svcr_op:$pstate, 0b1, GPR64:$rtpstate, timm0_1:$expected_pstate)>;
+def : Pat<(AArch64_smstop (i32 svcr_op:$pstate), (i64 GPR64:$rtpstate), (i64 timm0_1:$expected_pstate)),
+ (MSRpstatePseudo svcr_op:$pstate, 0b0, GPR64:$rtpstate, timm0_1:$expected_pstate)>;
+
//===----------------------------------------------------------------------===//
// SME2 Instructions
//===----------------------------------------------------------------------===//
Index: clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp
===================================================================
--- /dev/null
+++ clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -fsyntax-only -verify %s
+
+// This test is testing the diagnostic that Clang emits when compiling without '+sme'.
+
+void streaming_compatible_def() __arm_streaming_compatible {} // OK
+void streaming_def() __arm_streaming { } // expected-error {{function executed in streaming-SVE mode requires 'sme'}}
+void shared_za_def() __arm_shared_za { } // expected-error {{function using ZA state requires 'sme'}}
+__arm_new_za void new_za_def() { } // expected-error {{function using ZA state requires 'sme'}}
+__arm_locally_streaming void locally_streaming_def() { } // expected-error {{function executed in streaming-SVE mode requires 'sme'}}
+void streaming_shared_za_def() __arm_streaming __arm_shared_za { } // expected-error {{function executed in streaming-SVE mode requires 'sme'}}
+
+// It should work fine when we explicitly add the target("sme") attribute.
+__attribute__((target("sme"))) void streaming_compatible_def_sme_attr() __arm_streaming_compatible {} // OK
+__attribute__((target("sme"))) void streaming_def_sme_attr() __arm_streaming { } // OK
+__attribute__((target("sme"))) void shared_za_def_sme_attr() __arm_shared_za { } // OK
+__arm_new_za __attribute__((target("sme"))) void new_za_def_sme_attr() {} // OK
+__arm_locally_streaming __attribute__((target("sme"))) void locally_streaming_def_sme_attr() {} // OK
+
+// No code is generated for declarations, so it should be fine to declare using the attribute.
+void streaming_compatible_decl() __arm_streaming_compatible; // OK
+void streaming_decl() __arm_streaming; // OK
+void shared_za_decl() __arm_shared_za; // OK
+
+void non_streaming_decl();
+void non_streaming_def() {
+ streaming_compatible_decl(); // OK
+ streaming_decl(); // expected-error {{call to a streaming function requires 'sme'}}
+}
+
+void streaming_compatible_def2() __arm_streaming_compatible {
+ non_streaming_decl(); // OK
+ streaming_compatible_decl(); // OK
+ streaming_decl(); // expected-error {{call to a streaming function requires 'sme'}}
+}
+
+// Also test when call-site is not a function.
+int streaming_decl_ret_int() __arm_streaming;
+int x = streaming_decl_ret_int(); // expected-error {{call to a streaming function requires 'sme'}}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12140,6 +12140,33 @@
if (!Redeclaration && LangOpts.CUDA)
checkCUDATargetOverload(NewFD, Previous);
}
+
+ // Check if the function definition uses any AArch64 SME features without
+ // having the '+sme' feature enabled.
+ if (DeclIsDefn) {
+ bool UsesSM = NewFD->hasAttr<ArmLocallyStreamingAttr>();
+ bool UsesZA = NewFD->hasAttr<ArmNewZAAttr>();
+ if (const auto *FPT = NewFD->getType()->getAs<FunctionProtoType>()) {
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+ UsesSM |=
+ EPI.AArch64SMEAttributes & FunctionType::SME_PStateSMEnabledMask;
+ UsesZA |= EPI.AArch64SMEAttributes & FunctionType::SME_PStateZASharedMask;
+ }
+
+ if (UsesSM || UsesZA) {
+ llvm::StringMap<bool> FeatureMap;
+ Context.getFunctionFeatureMap(FeatureMap, NewFD);
+ if (!FeatureMap.count("sme")) {
+ if (UsesSM)
+ Diag(NewFD->getLocation(),
+ diag::err_sme_definition_using_sm_in_non_sme_target);
+ else if (UsesZA)
+ Diag(NewFD->getLocation(),
+ diag::err_sme_definition_using_za_in_non_sme_target);
+ }
+ }
+ }
+
return Redeclaration;
}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -6669,8 +6669,8 @@
}
/// Handles the checks for format strings, non-POD arguments to vararg
-/// functions, NULL arguments passed to non-NULL parameters, and diagnose_if
-/// attributes.
+/// functions, NULL arguments passed to non-NULL parameters, diagnose_if
+/// attributes and AArch64 SME attributes.
void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
const Expr *ThisArg, ArrayRef<const Expr *> Args,
bool IsMemberFunction, SourceLocation Loc,
@@ -6751,6 +6751,20 @@
ArgTy, ParamTy);
}
}
+
+ // If the callee has an AArch64 SME attribute to indicate that it is an
+ // __arm_streaming function, then the caller requires SME to be available.
+ FunctionProtoType::ExtProtoInfo ExtInfo = Proto->getExtProtoInfo();
+ if (ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateSMEnabledMask) {
+ if (auto *CallerFD = dyn_cast<FunctionDecl>(CurContext)) {
+ llvm::StringMap<bool> CallerFeatureMap;
+ Context.getFunctionFeatureMap(CallerFeatureMap, CallerFD);
+ if (!CallerFeatureMap.count("sme"))
+ Diag(Loc, diag::err_sme_call_in_non_sme_target);
+ } else if (!Context.getTargetInfo().hasFeature("sme")) {
+ Diag(Loc, diag::err_sme_call_in_non_sme_target);
+ }
+ }
}
if (FDecl && FDecl->hasAttr<AllocAlignAttr>()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3627,6 +3627,12 @@
"the vecreturn attribute can only be used on a POD (plain old data) class or structure (i.e. no virtual functions)">;
def err_sme_attr_mismatch : Error<
"function declared %0 was previously declared %1, which has different SME function attributes">;
+def err_sme_call_in_non_sme_target : Error<
+ "call to a streaming function requires 'sme'">;
+def err_sme_definition_using_sm_in_non_sme_target : Error<
+ "function executed in streaming-SVE mode requires 'sme'">;
+def err_sme_definition_using_za_in_non_sme_target : Error<
+ "function using ZA state requires 'sme'">;
def err_cconv_change : Error<
"function declared '%0' here was previously declared "
"%select{'%2'|without calling convention}1">;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
