Skip to content

[Clang][AArch64] Fix checkArmStreamingBuiltin for 'sve-b16b16' #109420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 8, 2024
Merged

[Clang][AArch64] Fix checkArmStreamingBuiltin for 'sve-b16b16' #109420

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
af4cd0b
[Clang][AArch64] Fix checkArmStreamingBuiltin for 'sve-b16b16'
sdesmalen-arm Sep 20, 2024
bfa1348
Add TableGen checks
sdesmalen-arm Sep 24, 2024
49c3ed3
Simplify approach by prepending target guard with "sve," or "sme,"
sdesmalen-arm Oct 2, 2024
e34305f
Reorder conditions
sdesmalen-arm Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions clang/lib/Sema/SemaARM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,23 +567,28 @@ 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.
Copy link
Collaborator

@paulwalker-arm paulwalker-arm Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to add code (SVEEmitter?) to parse the target guards to ensure there is at least one of the required SVE and SME features referenced. This should link to this code as something to update if the intent really is to add a new "base" option.


// Avoid emitting diagnostics for a function that can never compile.
if (FnType == SemaARM::ArmStreaming && !CallerFeatureMapWithoutSVE["sme"])
return false;

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
Expand Down
6 changes: 6 additions & 0 deletions clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down