[PowerPC][X86] Make cpu id builtins target independent and lower for PPC #68919
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
nemanjai
requested review from
arsenm,
nikic,
MaskRay,
RKSimon,
lei137,
stefanp-synopsys and
ilinpv
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-clang Author: Nemanja Ivanovic (nemanjai) ChangesMake _builtin_cpu{init|supports|is} target independent and provide an opt-in query for targets that want to support it. Each target is still responsible for their specific lowering/code-gen. Also provide code-gen for PowerPC. I originally proposed this in https://reviews.llvm.org/D152914 and this addresses the comments I received there. Patch is 34.63 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/68919.diff 17 Files Affected:
diff --git a/clang/include/clang/Basic/Builtins.def b/clang/include/clang/Basic/Builtins.def
index 6ea8484606cfd5d..5e1f4088ff63f8a 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -118,6 +118,11 @@
# define LANGBUILTIN(ID, TYPE, ATTRS, BUILTIN_LANG) BUILTIN(ID, TYPE, ATTRS)
#endif
+// Builtins for checking CPU features based on the GCC builtins.
+BUILTIN(__builtin_cpu_supports, "bcC*", "nc")
+BUILTIN(__builtin_cpu_is, "bcC*", "nc")
+BUILTIN(__builtin_cpu_init, "v", "n")
+
// Standard libc/libm functions:
BUILTIN(__builtin_atan2 , "ddd" , "Fne")
BUILTIN(__builtin_atan2f, "fff" , "Fne")
diff --git a/clang/include/clang/Basic/BuiltinsX86.def b/clang/include/clang/Basic/BuiltinsX86.def
index e4802f8ab1c1562..2acc5ce0f4a3653 100644
--- a/clang/include/clang/Basic/BuiltinsX86.def
+++ b/clang/include/clang/Basic/BuiltinsX86.def
@@ -26,13 +26,6 @@
# define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) BUILTIN(ID, TYPE, ATTRS)
#endif
-// Miscellaneous builtin for checking x86 cpu features.
-// TODO: Make this somewhat generic so that other backends
-// can use it?
-BUILTIN(__builtin_cpu_init, "v", "n")
-BUILTIN(__builtin_cpu_supports, "bcC*", "nc")
-BUILTIN(__builtin_cpu_is, "bcC*", "nc")
-
// Undefined Values
//
TARGET_BUILTIN(__builtin_ia32_undef128, "V2d", "ncV:128:", "")
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 9d56e97a3d4bb88..3d83b387aac0931 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1415,6 +1415,12 @@ class TargetInfo : public TransferrableTargetInfo,
getTriple().isOSFreeBSD());
}
+ // Identify whether this target supports __builtin_cpu_supports and
+ // __builtin_cpu_is.
+ virtual bool supportsCpuSupports() const { return false; }
+ virtual bool supportsCpuIs() const { return false; }
+ virtual bool supportsCpuInit() const { return false; }
+
// Validate the contents of the __builtin_cpu_supports(const char*)
// argument.
virtual bool validateCpuSupports(StringRef Name) const { return false; }
diff --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 0d87a3a4e8c20f3..d8759c86c9932ca 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -873,3 +873,17 @@ ArrayRef<Builtin::Info> PPCTargetInfo::getTargetBuiltins() const {
return llvm::ArrayRef(BuiltinInfo,
clang::PPC::LastTSBuiltin - Builtin::FirstTSBuiltin);
}
+
+bool PPCTargetInfo::validateCpuSupports(StringRef FeatureStr) const {
+#define PPC_FEATURE(NAME, DESC, ENUMNAME, ENUMVAL, HWCAPN) .Case(NAME, true)
+ return llvm::StringSwitch<bool>(FeatureStr)
+#include "llvm/TargetParser/PPCTargetParser.def"
+ .Default(false);
+}
+
+bool PPCTargetInfo::validateCpuIs(StringRef CPUName) const {
+#define PPC_CPU(NAME, NUM) .Case(NAME, true)
+ return llvm::StringSwitch<bool>(CPUName)
+#include "llvm/TargetParser/PPCTargetParser.def"
+ .Default(false);
+}
diff --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index 4d62673ba7fb8c5..f700b625b790309 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -359,6 +359,13 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public TargetInfo {
bool isSPRegName(StringRef RegName) const override {
return RegName.equals("r1") || RegName.equals("x1");
}
+
+ // We support __builtin_cpu_supports/__builtin_cpu_is on targets that
+ // have GLIBC since it is GLIBC that provides the HWCAP[2] in the auxv.
+ bool supportsCpuSupports() const override { return getTriple().isOSGlibc(); }
+ bool supportsCpuIs() const override { return getTriple().isOSGlibc(); }
+ bool validateCpuSupports(StringRef Feature) const override;
+ bool validateCpuIs(StringRef Name) const override;
};
class LLVM_LIBRARY_VISIBILITY PPC32TargetInfo : public PPCTargetInfo {
diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 4fdc94de1e0cb4d..3b379715163a8a8 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -211,6 +211,10 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public TargetInfo {
return RegName.equals("esp") || RegName.equals("rsp");
}
+ bool supportsCpuSupports() const override { return true; }
+ bool supportsCpuIs() const override { return true; }
+ bool supportsCpuInit() const override { return true; }
+
bool validateCpuSupports(StringRef FeatureStr) const override;
bool validateCpuIs(StringRef FeatureStr) const override;
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 8cb7943df9a7822..583b2be69ba1664 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -13585,11 +13585,11 @@ CodeGenFunction::EmitAArch64CpuSupports(ArrayRef<StringRef> FeaturesStrs) {
Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned BuiltinID,
const CallExpr *E) {
- if (BuiltinID == X86::BI__builtin_cpu_is)
+ if (BuiltinID == Builtin::BI__builtin_cpu_is)
return EmitX86CpuIs(E);
- if (BuiltinID == X86::BI__builtin_cpu_supports)
+ if (BuiltinID == Builtin::BI__builtin_cpu_supports)
return EmitX86CpuSupports(E);
- if (BuiltinID == X86::BI__builtin_cpu_init)
+ if (BuiltinID == Builtin::BI__builtin_cpu_init)
return EmitX86CpuInit();
// Handle MSVC intrinsics before argument evaluation to prevent double
@@ -16086,6 +16086,42 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned BuiltinID,
switch (BuiltinID) {
default: return nullptr;
+ case Builtin::BI__builtin_cpu_is: {
+ const Expr *CPUExpr = E->getArg(0)->IgnoreParenCasts();
+ StringRef CPUStr = cast<clang::StringLiteral>(CPUExpr)->getString();
+ unsigned NumCPUID = StringSwitch<unsigned>(CPUStr)
+#define PPC_CPU(Name, NumericID) .Case(Name, NumericID)
+#include "llvm/TargetParser/PPCTargetParser.def"
+ .Default(-1U);
+ Value *Op0 =
+ llvm::ConstantInt::get(Int32Ty, PPC_FAWORD_CPUID);
+ llvm::Function *F = CGM.getIntrinsic(Intrinsic::ppc_fixed_addr_ld);
+ Value *TheCall = Builder.CreateCall(F, {Op0}, "cpu_is");
+ return Builder.CreateICmpEQ(TheCall,
+ llvm::ConstantInt::get(Int32Ty, NumCPUID));
+ }
+ case Builtin::BI__builtin_cpu_supports: {
+ unsigned FeatureWord;
+ unsigned BitMask;
+ const Expr *CPUExpr = E->getArg(0)->IgnoreParenCasts();
+ StringRef CPUStr = cast<clang::StringLiteral>(CPUExpr)->getString();
+ std::tie(FeatureWord, BitMask) =
+ StringSwitch<std::pair<unsigned, unsigned>>(CPUStr)
+#define PPC_FEATURE(Name, Description, EnumName, Bitmask, FA_WORD) \
+ .Case(Name, {FA_WORD, Bitmask})
+#include "llvm/TargetParser/PPCTargetParser.def"
+ .Default({0, 0});
+ 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");
+ Value *Mask =
+ Builder.CreateAnd(TheCall, llvm::ConstantInt::get(Int32Ty, BitMask));
+ return Builder.CreateICmpNE(Mask, llvm::Constant::getNullValue(Int32Ty));
+#undef PPC_FAWORD_HWCAP
+#undef PPC_FAWORD_HWCAP2
+#undef PPC_FAWORD_CPUID
+ }
+
// __builtin_ppc_get_timebase is GCC 4.8+'s PowerPC-specific name for what we
// call __builtin_readcyclecounter.
case PPC::BI__builtin_ppc_get_timebase:
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2594a8f97f7d94e..6e67f4edc9da32e 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2110,6 +2110,66 @@ static bool checkFPMathBuiltinElementType(Sema &S, SourceLocation Loc,
return false;
}
+/// SemaBuiltinCpuSupports - Handle __builtin_cpu_supports(char *).
+/// This checks that the target supports __builtin_cpu_supports and
+/// that the string argument is constant and valid.
+static bool SemaBuiltinCpuSupports(Sema &S, const TargetInfo &TI,
+ const TargetInfo *AuxTI, CallExpr *TheCall) {
+ Expr *Arg = TheCall->getArg(0);
+
+ const TargetInfo *TheTI = nullptr;
+ if (TI.supportsCpuSupports())
+ TheTI = &TI;
+ else if (AuxTI && AuxTI->supportsCpuSupports())
+ TheTI = AuxTI;
+ else
+ return S.Diag(TheCall->getBeginLoc(), diag::err_builtin_target_unsupported)
+ << SourceRange(TheCall->getBeginLoc(), TheCall->getEndLoc());
+
+ // Check if the argument is a string literal.
+ if (!isa<StringLiteral>(Arg->IgnoreParenImpCasts()))
+ return S.Diag(TheCall->getBeginLoc(), diag::err_expr_not_string_literal)
+ << Arg->getSourceRange();
+
+ // Check the contents of the string.
+ StringRef Feature =
+ cast<StringLiteral>(Arg->IgnoreParenImpCasts())->getString();
+ if (!TheTI->validateCpuSupports(Feature))
+ return S.Diag(TheCall->getBeginLoc(), diag::err_invalid_cpu_supports)
+ << Arg->getSourceRange();
+ return false;
+}
+
+/// SemaBuiltinCpuIs - Handle __builtin_cpu_is(char *).
+/// This checks that the target supports __builtin_cpu_is and
+/// that the string argument is constant and valid.
+static bool SemaBuiltinCpuIs(Sema &S, const TargetInfo &TI,
+ const TargetInfo *AuxTI, CallExpr *TheCall) {
+ Expr *Arg = TheCall->getArg(0);
+
+ const TargetInfo *TheTI = nullptr;
+ if (TI.supportsCpuIs())
+ TheTI = &TI;
+ else if (AuxTI && AuxTI->supportsCpuIs())
+ TheTI = AuxTI;
+ else
+ return S.Diag(TheCall->getBeginLoc(), diag::err_builtin_target_unsupported)
+ << SourceRange(TheCall->getBeginLoc(), TheCall->getEndLoc());
+
+ // Check if the argument is a string literal.
+ if (!isa<StringLiteral>(Arg->IgnoreParenImpCasts()))
+ return S.Diag(TheCall->getBeginLoc(), diag::err_expr_not_string_literal)
+ << Arg->getSourceRange();
+
+ // Check the contents of the string.
+ StringRef Feature =
+ cast<StringLiteral>(Arg->IgnoreParenImpCasts())->getString();
+ if (!TheTI->validateCpuIs(Feature))
+ return S.Diag(TheCall->getBeginLoc(), diag::err_invalid_cpu_is)
+ << Arg->getSourceRange();
+ return false;
+}
+
ExprResult
Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
CallExpr *TheCall) {
@@ -2137,6 +2197,23 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
}
switch (BuiltinID) {
+ case Builtin::BI__builtin_cpu_supports:
+ if (SemaBuiltinCpuSupports(*this, Context.getTargetInfo(),
+ Context.getAuxTargetInfo(), TheCall))
+ return ExprError();
+ break;
+ case Builtin::BI__builtin_cpu_is:
+ if (SemaBuiltinCpuIs(*this, Context.getTargetInfo(),
+ Context.getAuxTargetInfo(), TheCall))
+ return ExprError();
+ break;
+ case Builtin::BI__builtin_cpu_init:
+ if (!Context.getTargetInfo().supportsCpuInit()) {
+ Diag(TheCall->getBeginLoc(), diag::err_builtin_target_unsupported)
+ << SourceRange(TheCall->getBeginLoc(), TheCall->getEndLoc());
+ return ExprError();
+ }
+ break;
case Builtin::BI__builtin___CFStringMakeConstantString:
// CFStringMakeConstantString is currently not implemented for GOFF (i.e.,
// on z/OS) and for XCOFF (i.e., on AIX). Emit unsupported
@@ -5582,47 +5659,6 @@ bool Sema::CheckNVPTXBuiltinFunctionCall(const TargetInfo &TI,
return false;
}
-/// SemaBuiltinCpuSupports - Handle __builtin_cpu_supports(char *).
-/// This checks that the target supports __builtin_cpu_supports and
-/// that the string argument is constant and valid.
-static bool SemaBuiltinCpuSupports(Sema &S, const TargetInfo &TI,
- CallExpr *TheCall) {
- Expr *Arg = TheCall->getArg(0);
-
- // Check if the argument is a string literal.
- if (!isa<StringLiteral>(Arg->IgnoreParenImpCasts()))
- return S.Diag(TheCall->getBeginLoc(), diag::err_expr_not_string_literal)
- << Arg->getSourceRange();
-
- // Check the contents of the string.
- StringRef Feature =
- cast<StringLiteral>(Arg->IgnoreParenImpCasts())->getString();
- if (!TI.validateCpuSupports(Feature))
- return S.Diag(TheCall->getBeginLoc(), diag::err_invalid_cpu_supports)
- << Arg->getSourceRange();
- return false;
-}
-
-/// SemaBuiltinCpuIs - Handle __builtin_cpu_is(char *).
-/// This checks that the target supports __builtin_cpu_is and
-/// that the string argument is constant and valid.
-static bool SemaBuiltinCpuIs(Sema &S, const TargetInfo &TI, CallExpr *TheCall) {
- Expr *Arg = TheCall->getArg(0);
-
- // Check if the argument is a string literal.
- if (!isa<StringLiteral>(Arg->IgnoreParenImpCasts()))
- return S.Diag(TheCall->getBeginLoc(), diag::err_expr_not_string_literal)
- << Arg->getSourceRange();
-
- // Check the contents of the string.
- StringRef Feature =
- cast<StringLiteral>(Arg->IgnoreParenImpCasts())->getString();
- if (!TI.validateCpuIs(Feature))
- return S.Diag(TheCall->getBeginLoc(), diag::err_invalid_cpu_is)
- << Arg->getSourceRange();
- return false;
-}
-
// Check if the rounding mode is legal.
bool Sema::CheckX86BuiltinRoundingOrSAE(unsigned BuiltinID, CallExpr *TheCall) {
// Indicates if this instruction has rounding control or just SAE.
@@ -6097,12 +6133,6 @@ static bool isX86_32Builtin(unsigned BuiltinID) {
bool Sema::CheckX86BuiltinFunctionCall(const TargetInfo &TI, unsigned BuiltinID,
CallExpr *TheCall) {
- if (BuiltinID == X86::BI__builtin_cpu_supports)
- return SemaBuiltinCpuSupports(*this, TI, TheCall);
-
- if (BuiltinID == X86::BI__builtin_cpu_is)
- return SemaBuiltinCpuIs(*this, TI, TheCall);
-
// Check for 32-bit only builtins on a 64-bit target.
const llvm::Triple &TT = TI.getTriple();
if (TT.getArch() != llvm::Triple::x86 && isX86_32Builtin(BuiltinID))
diff --git a/clang/test/CodeGen/builtin-cpu-supports.c b/clang/test/CodeGen/builtin-cpu-supports.c
index 796611c1fcd9afd..42497d8845437a4 100644
--- a/clang/test/CodeGen/builtin-cpu-supports.c
+++ b/clang/test/CodeGen/builtin-cpu-supports.c
@@ -1,11 +1,16 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm < %s| FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm < %s | FileCheck %s \
+// RUN: --check-prefix=CHECK-X86
+// RUN: %clang_cc1 -triple ppc64le-linux-gnu -emit-llvm < %s | FileCheck %s \
+// RUN: --check-prefix=CHECK-PPC
+
+#ifndef __PPC__
// Test that we have the structure definition, the gep offsets, the name of the
// global, the bit grab, and the icmp correct.
extern void a(const char *);
-// CHECK: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
-// CHECK: @__cpu_features2 = external dso_local global [3 x i32]
+// CHECK-X86: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
+// CHECK-X86: @__cpu_features2 = external dso_local global [3 x i32]
int main(void) {
__builtin_cpu_init();
@@ -15,38 +20,57 @@ int main(void) {
if (__builtin_cpu_supports("sse4.2"))
a("sse4.2");
- // CHECK: [[LOAD:%[^ ]+]] = load i32, ptr getelementptr inbounds ({ i32, i32, i32, [1 x i32] }, ptr @__cpu_model, i32 0, i32 3, i32 0)
- // CHECK: [[AND:%[^ ]+]] = and i32 [[LOAD]], 256
- // CHECK: = icmp eq i32 [[AND]], 256
+ // CHECK-X86: [[LOAD:%[^ ]+]] = load i32, ptr getelementptr inbounds ({ i32, i32, i32, [1 x i32] }, ptr @__cpu_model, i32 0, i32 3, i32 0)
+ // CHECK-X86: [[AND:%[^ ]+]] = and i32 [[LOAD]], 256
+ // CHECK-X86: = icmp eq i32 [[AND]], 256
if (__builtin_cpu_supports("gfni"))
a("gfni");
- // CHECK: [[LOAD:%[^ ]+]] = load i32, ptr @__cpu_features2
- // CHECK: [[AND:%[^ ]+]] = and i32 [[LOAD]], 1
- // CHECK: = icmp eq i32 [[AND]], 1
+ // CHECK-X86: [[LOAD:%[^ ]+]] = load i32, ptr @__cpu_features2
+ // CHECK-X86: [[AND:%[^ ]+]] = and i32 [[LOAD]], 1
+ // CHECK-X86: = icmp eq i32 [[AND]], 1
return 0;
}
-// CHECK: declare dso_local void @__cpu_indicator_init()
+// CHECK-X86: declare dso_local void @__cpu_indicator_init()
-// CHECK-LABEL: define{{.*}} @baseline(
-// CHECK: [[LOAD:%.*]] = load i32, ptr getelementptr inbounds ([[[#]] x i32], ptr @__cpu_features2, i32 0, i32 1)
-// CHECK-NEXT: and i32 [[LOAD]], -2147483648
+// CHECK-X86-LABEL: define{{.*}} @baseline(
+// CHECK-X86: [[LOAD:%.*]] = load i32, ptr getelementptr inbounds ([[[#]] x i32], ptr @__cpu_features2, i32 0, i32 1)
+// CHECK-X86-NEXT: and i32 [[LOAD]], -2147483648
int baseline() { return __builtin_cpu_supports("x86-64"); }
-// CHECK-LABEL: define{{.*}} @v2(
-// CHECK: [[LOAD:%.*]] = load i32, ptr getelementptr inbounds ([[[#]] x i32], ptr @__cpu_features2, i32 0, i32 2)
-// CHECK-NEXT: and i32 [[LOAD]], 1
+// CHECK-X86-LABEL: define{{.*}} @v2(
+// CHECK-X86: [[LOAD:%.*]] = load i32, ptr getelementptr inbounds ([[[#]] x i32], ptr @__cpu_features2, i32 0, i32 2)
+// CHECK-X86-NEXT: and i32 [[LOAD]], 1
int v2() { return __builtin_cpu_supports("x86-64-v2"); }
-// CHECK-LABEL: define{{.*}} @v3(
-// CHECK: [[LOAD:%.*]] = load i32, ptr getelementptr inbounds ([[[#]] x i32], ptr @__cpu_features2, i32 0, i32 2)
-// CHECK-NEXT: and i32 [[LOAD]], 2
+// CHECK-X86-LABEL: define{{.*}} @v3(
+// CHECK-X86: [[LOAD:%.*]] = load i32, ptr getelementptr inbounds ([[[#]] x i32], ptr @__cpu_features2, i32 0, i32 2)
+// CHECK-X86-NEXT: and i32 [[LOAD]], 2
int v3() { return __builtin_cpu_supports("x86-64-v3"); }
-// CHECK-LABEL: define{{.*}} @v4(
-// CHECK: [[LOAD:%.*]] = load i32, ptr getelementptr inbounds ([[[#]] x i32], ptr @__cpu_features2, i32 0, i32 2)
-// CHECK-NEXT: and i32 [[LOAD]], 4
+// CHECK-X86-LABEL: define{{.*}} @v4(
+// CHECK-X86: [[LOAD:%.*]] = load i32, ptr getelementptr inbounds ([[[#]] x i32], ptr @__cpu_features2, i32 0, i32 2)
+// CHECK-X86-NEXT: and i32 [[LOAD]], 4
int v4() { return __builtin_cpu_supports("x86-64-v4"); }
+#else
+int test(int a) {
+// CHECK-PPC: [[CPUSUP:%[^ ]+]] = call i32 @llvm.ppc.fixed.addr.ld(i32 2)
+// CHECK-PPC: [[AND:%[^ ]+]] = and i32 [[CPUSUP]], 8388608
+// CHECK-PPC: icmp ne i32 [[AND]], 0
+// CHECK-PPC: [[CPUSUP2:%[^ ]+]] = call i32 @llvm.ppc.fixed.addr.ld(i32 1)
+// CHECK-PPC: [[AND2:%[^ ]+]] = and i32 [[CPUSUP2]], 67108864
+// CHECK-PPC: icmp ne i32 [[AND2]], 0
+// CHECK-PPC: [[CPUID:%[^ ]+]] = call i32 @llvm.ppc.fixed.addr.ld(i32 3)
+// CHECK-PPC: icmp eq i32 [[CPUID]], 39
+ if (__builtin_cpu_supports("arch_3_00")) // HWCAP2
+ return a;
+ else if (__builtin_cpu_supports("mmu")) // HWCAP
+ return a - 5;
+ else if (__builtin_cpu_is("power7")) // CPUID
+ return a + a;
+ return a + 5;
+}
+#endif
diff --git a/clang/test/Sema/builtin-cpu-supports.c b/clang/test/Sema/builtin-cpu-supports.c
index ad310128fecebf6..cc6f1beb5d8a7c4 100644
--- a/clang/test/Sema/builtin-cpu-supports.c
+++ b/clang/test/Sema/builtin-cpu-supports.c
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -triple x86_64-pc-linux-gnu -verify %s
-// RUN: %clang_cc1 -fsyntax-only -triple powerpc64le-linux-gnu -verify %s
+// RUN: %clang_cc1 -fsyntax-only -triple aarch64-linux-gnu -verify %s
extern void a(const char *);
@@ -27,11 +27,13 @@ int main(void) {
(void)__builtin_cpu_supports("x86-64-v4");
(void)__builtin_cpu_supports("x86-64-v5"); // expected-error {{invalid cpu feature string for builtin}}
#else
- if (__builtin_cpu_supports("vsx")) // expected-error {{use of unknown builtin}}
+ if (__builtin_cpu_supports("aes")) // expected-error {{builtin is not supported on this target}}
a("vsx");
- if (__builtin_cpu_is("pwr9")) // expected-error {{use of unknown builtin}}
+ if (__builtin_cpu_is("cortex-x3")) // expected-error {{builtin is not supported on this target}}
a("pwr9");
+
+ __builtin_cpu_init(); // expected-error {{builtin is not supported on this target}}
#endif
return 0;
diff --git a/llvm/include/llvm/IR/IntrinsicsPowerPC.td b/llvm/include/llvm/IR/IntrinsicsPowerPC.td
index 3ede2a3736bf30d..f4e64764b4d1ccd 100644
--- a/llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ b/llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -210,6 +210,12 @@ let TargetPrefix = "ppc" in { // All intrinsics start with "llvm.ppc.".
[llvm_float_ty],
[llvm_float_ty, llvm_float_ty, llvm_float_ty, ll...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ilinpv
reviewed
Oct 16, 2023
kamaub
reviewed
Nov 8, 2023
Comment on lines
213
to
214
| // Load of a value provided by the system library at a fixed address. Used for | ||
| // accessing things like HWCAP word provided by GLIBC. |
There was a problem hiding this comment.
Suggested change
| // Load of a value provided by the system library at a fixed address. Used for | |
| // accessing things like HWCAP word provided by GLIBC. | |
| // This intrinsic is provided to allow back ends to emit load | |
| // instructions that load a value from a fixed address. The | |
| // parameter to the intrinsic is not an address, but an | |
| // immediate index into an enumeration that contains the | |
| // union of all such values available on all back ends. | |
| // An example is the HWCAP/HWCAP2/CPUID words | |
| // provided by GLIBC on PowerPC to allow fast access | |
| // to commonly used parts of AUXV. These are provided | |
| // at a fixed offset into the TCB (accessible through the | |
| // thread pointer). |
Missing expanded comment from previous round of reviews in https://reviews.llvm.org/D152914
There was a problem hiding this comment.
I don't think that comment would make sense any longer now that I have made this intrinsic PPC-specific. This is why I removed it and shortened it. I will put back the PPC-specific portions of the elaborated comment though.
kamaub
reviewed
Nov 8, 2023
| @@ -0,0 +1,80 @@ | |||
| #ifndef PPC_FEATURE | |||
There was a problem hiding this comment.
Missing file description similar to the head of clang/include/clang/Basic/Builtins.def
amy-kwan
reviewed
Nov 8, 2023
clang/lib/Sema/SemaChecking.cpp
Outdated
| << SourceRange(TheCall->getBeginLoc(), TheCall->getEndLoc()); | ||
|
|
||
| // Check if the argument is a string literal. | ||
| if (!isa<StringLiteral>(Arg->IgnoreParenImpCasts())) |
There was a problem hiding this comment.
It seems like we can pull out some of the calls are are repeated, such as Arg->IgnoreParenImpCasts().
There was a problem hiding this comment.
Done. In fact, it makes sense to point to the source location of the actual argument in the messages too, rather than the parens.
clang/lib/Sema/SemaChecking.cpp
Outdated
| << SourceRange(TheCall->getBeginLoc(), TheCall->getEndLoc()); | ||
|
|
||
| // Check if the argument is a string literal. | ||
| if (!isa<StringLiteral>(Arg->IgnoreParenImpCasts())) |
There was a problem hiding this comment.
Likewise here, perhaps we can pull out some of the calls that we call more than once.
amy-kwan
reviewed
Nov 8, 2023
clang/lib/Sema/SemaChecking.cpp
Outdated
| /// This checks that the target supports __builtin_cpu_supports and | ||
| /// that the string argument is constant and valid. | ||
| static bool SemaBuiltinCpuSupports(Sema &S, const TargetInfo &TI, | ||
| const TargetInfo *AuxTI, CallExpr *TheCall) { |
There was a problem hiding this comment.
Question:
Just wondering, what is and why are we checking an auxiliary target?
There was a problem hiding this comment.
I believe this is for OpenMP when compiling for both the host and device. I had it in here because it was in the original code but seems to have been removed.
There was a problem hiding this comment.
Turns out this is actually needed. There is a test that builds with -triple aarch64 -aux-triple x86_64. I will have to put the AUX target checks back.
amy-kwan
reviewed
Nov 13, 2023
clang/lib/Sema/SemaChecking.cpp
Outdated
| << Arg->getSourceRange(); | ||
|
|
||
| // Check the contents of the string. | ||
| StringRef Feature = |
There was a problem hiding this comment.
Is it more accurate to call this CPU instead of Feature here?
There was a problem hiding this comment.
Sure. But I ended up merging the two.
diggerlin
reviewed
Nov 14, 2023
| // The HWCAP and HWCAP2 word offsets are reversed on big endian Linux. | ||
| if ((FAType == PPC_FAWORD_HWCAP && Subtarget.isLittleEndian()) || | ||
| (FAType == PPC_FAWORD_HWCAP2 && !Subtarget.isLittleEndian())) | ||
| Offset = Subtarget.isPPC64() ? -0x7064 : -0x703C; |
There was a problem hiding this comment.
suggest do not use integer 0x7064 here, it is not easy to know what the 0x7064 stand for. please using a constexpr int Name=-0x7064 to explain what is the -0x7064 stand for.
There was a problem hiding this comment.
That is a good point. These aren't externally documented- by Glibc, but they should be documented here in the code.
clang/lib/Sema/SemaChecking.cpp
Outdated
| /// SemaBuiltinCpuSupports - Handle __builtin_cpu_supports(char *). | ||
| /// This checks that the target supports __builtin_cpu_supports and | ||
| /// that the string argument is constant and valid. | ||
| static bool SemaBuiltinCpuSupports(Sema &S, const TargetInfo &TI, |
There was a problem hiding this comment.
most of the source code of function SemaBuiltinCpuSupports and SemaBuiltinCpuIs are same , we can add helper function to reduce the code. TI.supportsCpuSupports() and diag::err_invalid_cpu_supports can be argument of the helper function.
amy-kwan
reviewed
Nov 14, 2023
amy-kwan
reviewed
Nov 15, 2023
diggerlin
reviewed
Nov 21, 2023
| #else | ||
| int test(int a) { | ||
| // CHECK-PPC: [[CPUSUP:%[^ ]+]] = call i32 @llvm.ppc.fixed.addr.ld(i32 2) | ||
| // CHECK-PPC: [[AND:%[^ ]+]] = and i32 [[CPUSUP]], 8388608 |
There was a problem hiding this comment.
change to CHECK-PPC-NEXT:
There was a problem hiding this comment.
Sure. I just re-generated the checks using the script.
| int test(int a) { | ||
| // CHECK-PPC: [[CPUSUP:%[^ ]+]] = call i32 @llvm.ppc.fixed.addr.ld(i32 2) | ||
| // CHECK-PPC: [[AND:%[^ ]+]] = and i32 [[CPUSUP]], 8388608 | ||
| // CHECK-PPC: icmp ne i32 [[AND]], 0 |
There was a problem hiding this comment.
change to CHECK-PPC-NEXT:
There was a problem hiding this comment.
Sure. I just re-generated the checks using the script.
| // CHECK-PPC: [[AND:%[^ ]+]] = and i32 [[CPUSUP]], 8388608 | ||
| // CHECK-PPC: icmp ne i32 [[AND]], 0 | ||
| // CHECK-PPC: [[CPUSUP2:%[^ ]+]] = call i32 @llvm.ppc.fixed.addr.ld(i32 1) | ||
| // CHECK-PPC: [[AND2:%[^ ]+]] = and i32 [[CPUSUP2]], 67108864 |
There was a problem hiding this comment.
change to CHECK-PPC-NEXT:
There was a problem hiding this comment.
Sure. I just re-generated the checks using the script.
| // CHECK-PPC: icmp ne i32 [[AND]], 0 | ||
| // CHECK-PPC: [[CPUSUP2:%[^ ]+]] = call i32 @llvm.ppc.fixed.addr.ld(i32 1) | ||
| // CHECK-PPC: [[AND2:%[^ ]+]] = and i32 [[CPUSUP2]], 67108864 | ||
| // CHECK-PPC: icmp ne i32 [[AND2]], 0 |
There was a problem hiding this comment.
Sure. I just re-generated the checks using the script.
| // CHECK-PPC: [[AND2:%[^ ]+]] = and i32 [[CPUSUP2]], 67108864 | ||
| // CHECK-PPC: icmp ne i32 [[AND2]], 0 | ||
| // CHECK-PPC: [[CPUID:%[^ ]+]] = call i32 @llvm.ppc.fixed.addr.ld(i32 3) | ||
| // CHECK-PPC: icmp eq i32 [[CPUID]], 39 |
There was a problem hiding this comment.
Sure. I just re-generated the checks using the script.
diggerlin
reviewed
Nov 23, 2023
| @@ -0,0 +1,80 @@ | |||
| #ifndef PPC_FEATURE | |||
There was a problem hiding this comment.
we will support these feature in AIX OS soon.
can we change PPC_FEATURE to PPC_LINUX_FEATURE
and PPC_CPU to PPC_LINUX_CPU
and when implement the __builtin_cpu_is and __builtin_cpu_supports in AIX OD
we can PPC_AIX_FEATURE and PPC_AIX_CPU
There was a problem hiding this comment.
I don't have an issue with that, assuming we will use the same mechanism on AIX.
diggerlin
reviewed
Jan 12, 2024
| #define PPC_LNX_FEATURE(Name, Description, EnumName, Bitmask, FA_WORD) \ | ||
| .Case(Name, {FA_WORD, Bitmask}) | ||
| #include "llvm/TargetParser/PPCTargetParser.def" | ||
| .Default({0, 0}); |
There was a problem hiding this comment.
Do we want a default here? without a default it will hit an assert here if fell off the end of the string-switch.
in the class of StringSwitch , there is function as
[[nodiscard]] operator R() {
assert(Result && "Fell off the end of a string-switch");
return std::move(*Result);
}
There was a problem hiding this comment.
I think this is a good point. If we somehow have a string argument that would produce the default case and it has made it past Sema checking, it would be good to crash here rather than produce an invalid call to the intrinsic (or an invalid mask).
Of course, that assert is not all that friendly and it might be better to assert within this function.
What do you think about keeping the default and adding an assert below such as:
assert(BitMask && "Invalid target feature string. Missed by SemaChecking?");
amy-kwan
reviewed
Jan 12, 2024
| @@ -210,6 +210,15 @@ let TargetPrefix = "ppc" in { // All intrinsics start with "llvm.ppc.". | |||
| [llvm_float_ty], | |||
| [llvm_float_ty, llvm_float_ty, llvm_float_ty, llvm_vararg_ty], | |||
| [IntrNoMem]>; | |||
| // Load of a value provided by the system library at a fixed address. Used for | |||
| // accessing things like HWCAP word provided by GLIBC. The immediate argument | |||
There was a problem hiding this comment.
Minor nits:
- Should it be
the HWCAP word? - We use GLIBC on this line but Glibc on line 217. I think it would be better if we were consistent and use one way of addressing GLIBC.
There was a problem hiding this comment.
I'm fine with both of these suggestions.
amy-kwan
approved these changes
Jan 18, 2024
Make __builtin_cpu_{init|supports|is} target independent and provide
an opt-in query for targets that want to support it. Each target is
still responsible for their specific lowering/code-gen.
Also provide code-gen for PowerPC.
- Use script for generating test checks - Clean up code and rename macros for clarity - Remove magic number offsets - Get rid of auxiliary targets in Sema checking - Clarify some comments and add missing ones - Add check for undefined symbol added for safety
The way we define target-independent builtins has changed which required a rebase. Also cleaned up some comments and improved asserts as suggested in the review.
nemanjai
force-pushed
the
nemanja/builtin-cpu
branch
from
4253c52 to
b121664
Compare
|
Hi Aokamal,
This maco was not taken into account in the implementation. Can you please open a bug and assign the issue to nemanjai?
Thank-you for reporting this.
Regards,
Lei
From: Aokamal ***@***.***>
Date: Tuesday, January 30, 2024 at 10:30 AM
To: llvm/llvm-project ***@***.***>
Cc: Lei Huang ***@***.***>, State change ***@***.***>
Subject: [EXTERNAL] Re: [llvm/llvm-project] [PowerPC][X86] Make cpu id builtins target independent and lower for PPC (PR #68919)
Hi, Sorry I kind of don't know much about the change. We have the following code snippet #if ABSL_HAVE_BUILTIN(__builtin_cpu_supports) if (__builtin_cpu_supports("avx2")) { .. . } #endif when run with arm cpu, it fails with error: builtin is
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
This message came from outside your organization.
<https://us-phishalarm-ewt.proofpoint.com/EWT/v1/PjiDSg!2U-qSTa06VIRMu4YcIkOZYJv87-YdAlWKJZhcdfdL-hOL9LK6PP42RfIRBxJiqbiimkckMLPKtrGCex3SbDVRbwy2koA67SnPo8$>
Report Suspicious <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/PjiDSg!2U-qSTa06VIRMu4YcIkOZYJv87-YdAlWKJZhcdfdL-hOL9LK6PP42RfIRBxJiqbiimkckMLPKtrGCex3SbDVRbwy2koA67SnPo8$>
ZjQcmQRYFpfptBannerEnd
Hi,
Sorry I kind of don't know much about the change. We have the following code snippet
#if ABSL_HAVE_BUILTIN(__builtin_cpu_supports)
if (__builtin_cpu_supports("avx2")) {
...
}
#endif
when run with arm cpu, it fails with error: builtin is not supported on this target on the second if. Does it mean ABSL_HAVE_BUILTIN is not accurate here ?
—
Reply to this email directly, view it on GitHub<#68919 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKTLOI65B4HXCU6BOZW3G73YREGYJAVCNFSM6AAAAAA56ALXCOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJXGIYTSNJSGM>.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
|
Sorry, I'll put up a patch for review shortly to fix this problem. |
|
Fix in #80058 |
ilinpv
added a commit
to ilinpv/llvm-project
that referenced
this pull request
Feb 20, 2024
The patch complements llvm#68919 and adds AArch64 support for builtin __builtin_cpu_supports("feature1+...+featureN") which return true if all specified CPU features in argument are detected. Also compiler-rt aarch64 native run tests for features detection mechanism were added and 'cpu_model' check was fixed after its refactor merged llvm#75635 Original RFC was https://reviews.llvm.org/D153153
ilinpv
added a commit
to ilinpv/llvm-project
that referenced
this pull request
Feb 22, 2024
The patch complements llvm#68919 and adds AArch64 support for builtin __builtin_cpu_supports("feature1+...+featureN") which return true if all specified CPU features in argument are detected. Also compiler-rt aarch64 native run tests for features detection mechanism were added and 'cpu_model' check was fixed after its refactor merged llvm#75635 Original RFC was https://reviews.llvm.org/D153153
ilinpv
added a commit
that referenced
this pull request
Feb 22, 2024
The patch complements #68919 and adds AArch64 support for builtin `__builtin_cpu_supports("feature1+...+featureN")` which return true if all specified CPU features in argument are detected. Also compiler-rt aarch64 native run tests for features detection mechanism were added and 'cpu_model' check was fixed after its refactor merged #75635 Original RFC was https://reviews.llvm.org/D153153
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.
Make _builtin_cpu{init|supports|is} target independent and provide an opt-in query for targets that want to support it. Each target is still responsible for their specific lowering/code-gen. Also provide code-gen for PowerPC.
I originally proposed this in https://reviews.llvm.org/D152914 and this addresses the comments I received there.