[X86][AArch64][PowerPC] __builtin_cpu_supports accepts unknown options. #83515
Conversation
This file contains 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
The patch fixes llvm#83407 modifing __builtin_cpu_supports behaviour so that it returns false if unsupported features names provided in parameter and issue a warning. __builtin_cpu_supports is target independent, but currently supported by X86, AArch64 and PowerPC only.
ilinpv
requested review from
MaskRay,
jroelofs,
DanielKristofKiss,
erichkeane and
nemanjai
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Pavel Iliin (ilinpv) ChangesThe patch fixes #83407 modifing __builtin_cpu_supports behaviour so that it returns false if unsupported features names provided in parameter and issue a warning. __builtin_cpu_supports is target independent, but currently supported by X86, AArch64 and PowerPC only. Full diff: https://github.com/llvm/llvm-project/pull/83515.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4f8902e37bd3bb..938de5859513f8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -765,7 +765,7 @@ def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">;
def err_arm_invalid_specialreg : Error<"invalid special register for builtin">;
def err_arm_invalid_coproc : Error<"coprocessor %0 must be configured as "
"%select{GCP|CDE}1">;
-def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">;
+def warn_invalid_cpu_supports : Warning<"invalid cpu feature string for builtin">;
def err_invalid_cpu_is : Error<"invalid cpu name for builtin">;
def err_invalid_cpu_specific_dispatch_value : Error<
"invalid option '%0' for %select{cpu_specific|cpu_dispatch}1">;
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 98684448f4ff5c..e90014261217bc 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -13952,6 +13952,8 @@ Value *CodeGenFunction::EmitX86CpuIs(StringRef CPUStr) {
Value *CodeGenFunction::EmitX86CpuSupports(const CallExpr *E) {
const Expr *FeatureExpr = E->getArg(0)->IgnoreParenCasts();
StringRef FeatureStr = cast<StringLiteral>(FeatureExpr)->getString();
+ if (!getContext().getTargetInfo().validateCpuSupports(FeatureStr))
+ return Builder.getFalse();
return EmitX86CpuSupports(FeatureStr);
}
@@ -14041,6 +14043,8 @@ Value *CodeGenFunction::EmitAArch64CpuSupports(const CallExpr *E) {
ArgStr.split(Features, "+");
for (auto &Feature : Features) {
Feature = Feature.trim();
+ if (!llvm::AArch64::parseArchExtension(Feature))
+ return Builder.getFalse();
if (Feature != "default")
Features.push_back(Feature);
}
@@ -16639,7 +16643,8 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned BuiltinID,
.Case(Name, {FA_WORD, Bitmask})
#include "llvm/TargetParser/PPCTargetParser.def"
.Default({0, 0});
- assert(BitMask && "Invalid target feature string. Missed by SemaChecking?");
+ if (!BitMask)
+ return Builder.getFalse();
Value *Op0 = llvm::ConstantInt::get(Int32Ty, FeatureWord);
llvm::Function *F = CGM.getIntrinsic(Intrinsic::ppc_fixed_addr_ld);
Value *TheCall = Builder.CreateCall(F, {Op0}, "cpu_supports");
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 690bdaa63d058b..7be2b31df2413f 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2180,9 +2180,11 @@ static bool SemaBuiltinCpu(Sema &S, const TargetInfo &TI, CallExpr *TheCall,
// Check the contents of the string.
StringRef Feature = cast<StringLiteral>(Arg)->getString();
- if (IsCPUSupports && !TheTI->validateCpuSupports(Feature))
- return S.Diag(TheCall->getBeginLoc(), diag::err_invalid_cpu_supports)
- << Arg->getSourceRange();
+ if (IsCPUSupports && !TheTI->validateCpuSupports(Feature)) {
+ S.Diag(TheCall->getBeginLoc(), diag::warn_invalid_cpu_supports)
+ << Arg->getSourceRange();
+ return false;
+ }
if (!IsCPUSupports && !TheTI->validateCpuIs(Feature))
return S.Diag(TheCall->getBeginLoc(), diag::err_invalid_cpu_is)
<< Arg->getSourceRange();
diff --git a/clang/test/CodeGen/aarch64-cpu-supports.c b/clang/test/CodeGen/aarch64-cpu-supports.c
index 872fec6827ef11..c54b7475a3fd5f 100644
--- a/clang/test/CodeGen/aarch64-cpu-supports.c
+++ b/clang/test/CodeGen/aarch64-cpu-supports.c
@@ -34,6 +34,11 @@
// CHECK-NEXT: store i32 3, ptr [[RETVAL]], align 4
// CHECK-NEXT: br label [[RETURN]]
// CHECK: if.end4:
+// CHECK-NEXT: br i1 false, label [[IF_THEN5:%.*]], label [[IF_END6:%.*]]
+// CHECK: if.then5:
+// CHECK-NEXT: store i32 4, ptr [[RETVAL]], align 4
+// CHECK-NEXT: br label [[RETURN]]
+// CHECK: if.end6:
// CHECK-NEXT: store i32 0, ptr [[RETVAL]], align 4
// CHECK-NEXT: br label [[RETURN]]
// CHECK: return:
@@ -50,5 +55,8 @@ int main(void) {
if (__builtin_cpu_supports("sme2+ls64_v+wfxt"))
return 3;
+ if (__builtin_cpu_supports("avx2"))
+ return 4;
+
return 0;
}
diff --git a/clang/test/Misc/warning-flags.c b/clang/test/Misc/warning-flags.c
index 9d4cac9e39b420..bb3c7d816d2f0e 100644
--- a/clang/test/Misc/warning-flags.c
+++ b/clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@ This test serves two purposes:
The list of warnings below should NEVER grow. It should gradually shrink to 0.
-CHECK: Warnings without flags (66):
+CHECK: Warnings without flags (67):
CHECK-NEXT: ext_expected_semi_decl_list
CHECK-NEXT: ext_explicit_specialization_storage_class
@@ -58,6 +58,7 @@ CHECK-NEXT: warn_ignoring_ftabstop_value
CHECK-NEXT: warn_implements_nscopying
CHECK-NEXT: warn_incompatible_qualified_id
CHECK-NEXT: warn_invalid_asm_cast_lvalue
+CHECK-NEXT: warn_invalid_cpu_supports
CHECK-NEXT: warn_maynot_respond
CHECK-NEXT: warn_method_param_redefinition
CHECK-NEXT: warn_missing_case_for_condition
diff --git a/clang/test/Sema/aarch64-cpu-supports.c b/clang/test/Sema/aarch64-cpu-supports.c
index 24aae9542dbc42..ddeed7c5bc9e97 100644
--- a/clang/test/Sema/aarch64-cpu-supports.c
+++ b/clang/test/Sema/aarch64-cpu-supports.c
@@ -5,19 +5,19 @@ int test_aarch64_features(void) {
// expected-error@+1 {{expression is not a string literal}}
if (__builtin_cpu_supports(ssbs2))
return 1;
- // expected-error@+1 {{invalid cpu feature string}}
+ // expected-warning@+1 {{invalid cpu feature string}}
if (__builtin_cpu_supports(""))
return 2;
- // expected-error@+1 {{invalid cpu feature string}}
+ // expected-warning@+1 {{invalid cpu feature string}}
if (__builtin_cpu_supports("pmull128"))
return 3;
- // expected-error@+1 {{invalid cpu feature string}}
+ // expected-warning@+1 {{invalid cpu feature string}}
if (__builtin_cpu_supports("sve2,rpres"))
return 4;
- // expected-error@+1 {{invalid cpu feature string}}
+ // expected-warning@+1 {{invalid cpu feature string}}
if (__builtin_cpu_supports("dgh+sve2-pmull"))
return 5;
- // expected-error@+1 {{invalid cpu feature string}}
+ // expected-warning@+1 {{invalid cpu feature string}}
if (__builtin_cpu_supports("default"))
return 6;
if (__builtin_cpu_supports(" ssbs + bti "))
diff --git a/clang/test/Sema/builtin-cpu-supports.c b/clang/test/Sema/builtin-cpu-supports.c
index 733d797f3ff8f8..51ee9661807f8b 100644
--- a/clang/test/Sema/builtin-cpu-supports.c
+++ b/clang/test/Sema/builtin-cpu-supports.c
@@ -7,7 +7,7 @@ extern const char *str;
int main(void) {
#ifdef __x86_64__
- if (__builtin_cpu_supports("ss")) // expected-error {{invalid cpu feature string}}
+ if (__builtin_cpu_supports("ss")) // expected-warning {{invalid cpu feature string}}
a("sse4.2");
if (__builtin_cpu_supports(str)) // expected-error {{expression is not a string literal}}
@@ -25,9 +25,9 @@ int main(void) {
(void)__builtin_cpu_supports("x86-64-v2");
(void)__builtin_cpu_supports("x86-64-v3");
(void)__builtin_cpu_supports("x86-64-v4");
- (void)__builtin_cpu_supports("x86-64-v5"); // expected-error {{invalid cpu feature string for builtin}}
+ (void)__builtin_cpu_supports("x86-64-v5"); // expected-warning {{invalid cpu feature string for builtin}}
#else
- if (__builtin_cpu_supports("neon")) // expected-error {{invalid cpu feature string for builtin}}
+ if (__builtin_cpu_supports("neon")) // expected-warning {{invalid cpu feature string for builtin}}
a("vsx");
if (__builtin_cpu_is("cortex-x3")) // expected-error {{builtin is not supported on this target}}
|
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.
The patch fixes #83407 modifing __builtin_cpu_supports behaviour so that it returns false if unsupported features names provided in parameter and issue a warning. __builtin_cpu_supports is target independent, but currently supported by X86, AArch64 and PowerPC only.