[clang][FMV][AArch64] Improve streaming mode compatibility. #100181
Merged
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
* Allow arm-streaming if all the functions versions adhere to it. * Allow arm-streaming-compatible if all the functions versions adhere to it. * Allow arm-locally-streaming regardless of the other functions versions. When the caller needs to toggle the streaming mode all the function versions of the callee must adhere to the same mode, otherwise the call will yield a runtime error. Imagine the versions of the callee live in separate TUs. The version that is visible to the caller will determine the calling convention used when generating code for the callsite. Therefore we cannot support mixing streaming with non-streaming function versions. Imagine TU1 has a streaming caller and calls foo._sme which is streaming-compatible. The codegen for the callsite will not switch off the streaming mode. Then in TU2 we have a version which is non-streaming and could potentially be called in streaming mode. Similarly if the caller is non-streaming and the called version is streaming-compatible the codegen for the callsite will not switch on the streaming mode, but other versions may be streaming.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Alexandros Lamprineas (labrinea) Changes
When the caller needs to toggle the streaming mode all the function versions of the callee must adhere to the same mode, otherwise the call will yield a runtime error. Imagine the versions of the callee live in separate TUs. The version that is visible to the caller will determine the calling convention used when generating code for the callsite. Therefore we cannot support mixing streaming with non-streaming function versions. Imagine TU1 has a streaming caller and calls foo._sme which is streaming-compatible. The codegen for the callsite will not switch off the streaming mode. Then in TU2 we have a version which is non-streaming and could potentially be called in streaming mode. Similarly if the caller is non-streaming and the called version is streaming-compatible the codegen for the callsite will not switch on the streaming mode, but other versions may be streaming. Full diff: https://github.com/llvm/llvm-project/pull/100181.diff 7 Files Affected:
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 608bd90fcc3ff..da25a7ec80773 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -3183,6 +3183,22 @@ class ASTContext : public RefCountedBase<ASTContext> {
const FunctionDecl *FD,
llvm::function_ref<void(FunctionDecl *)> Pred) const;
+ bool areFMVCompatible(const FunctionDecl *FD1,
+ const FunctionDecl *FD2) const {
+ if (!hasSameType(FD1->getReturnType(), FD2->getReturnType()))
+ return false;
+
+ if (FD1->getNumParams() != FD2->getNumParams())
+ return false;
+
+ for (unsigned I = 0; I < FD1->getNumParams(); ++I)
+ if (!hasSameType(FD1->getParamDecl(I)->getOriginalType(),
+ FD2->getParamDecl(I)->getOriginalType()))
+ return false;
+
+ return true;
+ }
+
const CXXConstructorDecl *
getCopyConstructorForExceptionObject(CXXRecordDecl *RD);
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d60f32674ca3a..f9294964d3774 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3802,8 +3802,6 @@ def warn_sme_locally_streaming_has_vl_args_returns : Warning<
InGroup<AArch64SMEAttributes>, DefaultIgnore;
def err_conflicting_attributes_arm_state : Error<
"conflicting attributes for state '%0'">;
-def err_sme_streaming_cannot_be_multiversioned : Error<
- "streaming function cannot be multi-versioned">;
def err_unknown_arm_state : Error<
"unknown state '%0'">;
def err_missing_arm_state : Error<
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 90bcbea072e39..b9cce939561ef 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12449,8 +12449,7 @@ void ASTContext::forEachMultiversionedFunctionVersion(
for (auto *CurDecl :
FD->getDeclContext()->getRedeclContext()->lookup(FD->getDeclName())) {
FunctionDecl *CurFD = CurDecl->getAsFunction()->getMostRecentDecl();
- if (CurFD && hasSameType(CurFD->getType(), FD->getType()) &&
- !SeenDecls.contains(CurFD)) {
+ if (CurFD && areFMVCompatible(CurFD, FD) && !SeenDecls.contains(CurFD)) {
SeenDecls.insert(CurFD);
Pred(CurFD);
}
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index bb25a0b3a45ae..d8be19d62d68c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11009,6 +11009,9 @@ static bool AttrCompatibleWithMultiVersion(attr::Kind Kind,
switch (Kind) {
default:
return false;
+ case attr::ArmLocallyStreaming:
+ return MVKind == MultiVersionKind::TargetVersion ||
+ MVKind == MultiVersionKind::TargetClones;
case attr::Used:
return MVKind == MultiVersionKind::Target;
case attr::NonNull:
@@ -11145,7 +11148,24 @@ bool Sema::areMultiversionVariantFunctionsCompatible(
FunctionType::ExtInfo OldTypeInfo = OldType->getExtInfo();
FunctionType::ExtInfo NewTypeInfo = NewType->getExtInfo();
- if (OldTypeInfo.getCC() != NewTypeInfo.getCC())
+ const auto *OldFPT = OldFD->getType()->getAs<FunctionProtoType>();
+ const auto *NewFPT = NewFD->getType()->getAs<FunctionProtoType>();
+
+ bool ArmStreamingCCMismatched = false;
+ // Locally streaming does not affect the calling convention.
+ if (OldFPT && NewFPT && !OldFD->hasAttr<ArmLocallyStreamingAttr>() &&
+ !NewFD->hasAttr<ArmLocallyStreamingAttr>()) {
+ unsigned Diff =
+ OldFPT->getAArch64SMEAttributes() ^ NewFPT->getAArch64SMEAttributes();
+ // Streaming versions cannot be mixed with non-streaming versions.
+ if (Diff & FunctionType::SME_PStateSMEnabledMask)
+ ArmStreamingCCMismatched = true;
+ // Streaming-compatible versions cannot be mixed with anything else.
+ if (Diff & FunctionType::SME_PStateSMCompatibleMask)
+ ArmStreamingCCMismatched = true;
+ }
+
+ if (OldTypeInfo.getCC() != NewTypeInfo.getCC() || ArmStreamingCCMismatched)
return Diag(DiffDiagIDAt.first, DiffDiagIDAt.second) << CallingConv;
QualType OldReturnType = OldType->getReturnType();
@@ -11165,9 +11185,8 @@ bool Sema::areMultiversionVariantFunctionsCompatible(
if (!CLinkageMayDiffer && OldFD->isExternC() != NewFD->isExternC())
return Diag(DiffDiagIDAt.first, DiffDiagIDAt.second) << LanguageLinkage;
- if (CheckEquivalentExceptionSpec(
- OldFD->getType()->getAs<FunctionProtoType>(), OldFD->getLocation(),
- NewFD->getType()->getAs<FunctionProtoType>(), NewFD->getLocation()))
+ if (CheckEquivalentExceptionSpec(OldFPT, OldFD->getLocation(), NewFPT,
+ NewFD->getLocation()))
return true;
}
return false;
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 50e7be9ea6020..46a95462ed61e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3024,9 +3024,6 @@ bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unsupported << None << CurFeature << TargetVersion;
}
- if (IsArmStreamingFunction(cast<FunctionDecl>(D),
- /*IncludeLocallyStreaming=*/false))
- return Diag(LiteralLoc, diag::err_sme_streaming_cannot_be_multiversioned);
return false;
}
@@ -3123,10 +3120,6 @@ bool Sema::checkTargetClonesAttrString(
HasNotDefault = true;
}
}
- if (IsArmStreamingFunction(cast<FunctionDecl>(D),
- /*IncludeLocallyStreaming=*/false))
- return Diag(LiteralLoc,
- diag::err_sme_streaming_cannot_be_multiversioned);
} else {
// Other targets ( currently X86 )
if (Cur.starts_with("arch=")) {
diff --git a/clang/test/Sema/aarch64-fmv-streaming.c b/clang/test/Sema/aarch64-fmv-streaming.c
new file mode 100644
index 0000000000000..050ce5c8c803b
--- /dev/null
+++ b/clang/test/Sema/aarch64-fmv-streaming.c
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme -Waarch64-sme-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme -Waarch64-sme-attributes -fsyntax-only -verify=expected-cpp -x c++ %s
+
+__attribute__((target_clones("sve", "simd"))) void ok_arm_streaming(void) __arm_streaming {}
+__arm_locally_streaming __attribute__((target_version("sme2"))) void ok_arm_streaming(void) {}
+__attribute__((target_version("default"))) void ok_arm_streaming(void) __arm_streaming {}
+
+__attribute__((target_clones("sve", "simd"))) void ok_arm_streaming_compatible(void) __arm_streaming_compatible {}
+__arm_locally_streaming __attribute__((target_version("sme2"))) void ok_arm_streaming_compatible(void) {}
+__attribute__((target_version("default"))) void ok_arm_streaming_compatible(void) __arm_streaming_compatible {}
+
+__arm_locally_streaming __attribute__((target_clones("sve", "simd"))) void ok_no_streaming(void) {}
+__attribute__((target_version("sme2"))) void ok_no_streaming(void) {}
+__attribute__((target_version("default"))) void ok_no_streaming(void) {}
+
+__attribute__((target_clones("sve", "simd"))) void bad_mixed_streaming(void) {}
+// expected-cpp-error@+2 {{multiversioned function declaration has a different calling convention}}
+// expected-error@+1 {{multiversioned function declaration has a different calling convention}}
+__attribute__((target_version("sme2"))) void bad_mixed_streaming(void) __arm_streaming {}
+// expected-cpp-error@+2 {{multiversioned function declaration has a different calling convention}}
+// expected-error@+1 {{multiversioned function declaration has a different calling convention}}
+__attribute__((target_version("default"))) void bad_mixed_streaming(void) __arm_streaming_compatible {}
+
+void n_caller(void) {
+ ok_arm_streaming();
+ ok_arm_streaming_compatible();
+ ok_no_streaming();
+ bad_mixed_streaming();
+}
+
+void s_caller(void) __arm_streaming {
+ ok_arm_streaming();
+ ok_arm_streaming_compatible();
+ ok_no_streaming();
+ bad_mixed_streaming();
+}
+
+void sc_caller(void) __arm_streaming_compatible {
+ ok_arm_streaming();
+ ok_arm_streaming_compatible();
+ ok_no_streaming();
+ bad_mixed_streaming();
+}
diff --git a/clang/test/Sema/aarch64-sme-func-attrs.c b/clang/test/Sema/aarch64-sme-func-attrs.c
index 6db39d6a71e36..0c263eb2610cf 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs.c
+++ b/clang/test/Sema/aarch64-sme-func-attrs.c
@@ -455,48 +455,6 @@ void unimplemented_spill_fill_za(void (*share_zt0_only)(void) __arm_inout("zt0")
share_zt0_only();
}
-// expected-cpp-error@+2 {{streaming function cannot be multi-versioned}}
-// expected-error@+1 {{streaming function cannot be multi-versioned}}
-__attribute__((target_version("sme2")))
-void cannot_work_version(void) __arm_streaming {}
-// expected-cpp-error@+5 {{function declared 'void ()' was previously declared 'void () __arm_streaming', which has different SME function attributes}}
-// expected-cpp-note@-2 {{previous declaration is here}}
-// expected-error@+3 {{function declared 'void (void)' was previously declared 'void (void) __arm_streaming', which has different SME function attributes}}
-// expected-note@-4 {{previous declaration is here}}
-__attribute__((target_version("default")))
-void cannot_work_version(void) {}
-
-
-// expected-cpp-error@+2 {{streaming function cannot be multi-versioned}}
-// expected-error@+1 {{streaming function cannot be multi-versioned}}
-__attribute__((target_clones("sme2")))
-void cannot_work_clones(void) __arm_streaming {}
-
-
-__attribute__((target("sme2")))
-void just_fine_streaming(void) __arm_streaming {}
-__attribute__((target_version("sme2")))
-void just_fine(void) { just_fine_streaming(); }
-__attribute__((target_version("default")))
-void just_fine(void) {}
-
-
-__arm_locally_streaming
-__attribute__((target_version("sme2")))
-void incompatible_locally_streaming(void) {}
-// expected-error@-1 {{attribute 'target_version' multiversioning cannot be combined with attribute '__arm_locally_streaming'}}
-// expected-cpp-error@-2 {{attribute 'target_version' multiversioning cannot be combined with attribute '__arm_locally_streaming'}}
-__attribute__((target_version("default")))
-void incompatible_locally_streaming(void) {}
-
-
-void fmv_caller() {
- cannot_work_version();
- cannot_work_clones();
- just_fine();
- incompatible_locally_streaming();
-}
-
void sme_streaming_with_vl_arg(__SVInt8_t a) __arm_streaming { }
__SVInt8_t sme_streaming_returns_vl(void) __arm_streaming { __SVInt8_t r; return r; }
|
jroelofs
approved these changes
Jul 23, 2024
|
relevant issue I've been meaning to file: #100200 |
* Disregard declarations with different variadic type. Note that we are not diagnosing such differences, we just do not consider the two declarations part of the same declaration chain. As a result the diagnostic comes upon use: "ambiguous call". This is NFC. * Added a sema test for variadic type mismatch. * Added a codegen test for the calling conventions.
Made __arm_locally_streaming require the same calling convention as the rest of the callee versions and updated the tests.
Combined two separate SME_PState bitmask checks into one as suggested.
mgabka
reviewed
Jul 25, 2024
* Replaced areFMVCompatible with hasSameType. Looks like this change was unnecessary in the first place. Most likely a residue from my WIP before I raised the PR. Thanks Sander for finding this! * Removed the corresponding sema test for variadic type mismatch.
sdesmalen-arm
approved these changes
Jul 26, 2024
|
/cherry-pick f8ae128 |
|
Failed to cherry-pick: f8ae128 https://github.com/llvm/llvm-project/actions/runs/10108416112 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
|
/cherry-pick f8ae128 |
|
Failed to cherry-pick: f8ae128 https://github.com/llvm/llvm-project/actions/runs/10140964466 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
|
Manually created a PR for the backport #101007 |
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.
When the caller needs to toggle the streaming mode all the function versions of the callee must adhere to the same mode, otherwise the call will yield a runtime error.
Imagine the versions of the callee live in separate TUs. The version that is visible to the caller will determine the calling convention used when generating code for the callsite. Therefore we cannot support mixing streaming with non-streaming function versions. Imagine TU1 has a streaming caller and calls foo._sme which is streaming-compatible. The codegen for the callsite will not switch off the streaming mode. Then in TU2 we have a version which is non-streaming and could potentially be called in streaming mode. Similarly if the caller is non-streaming and the called version is streaming-compatible the codegen for the callsite will not switch on the streaming mode, but other versions may be streaming.