[AIX] Lower intrinsic __builtin_cpu_is into AIX platform-specific code. #80069

diggerlin · 2024-01-30T22:13:33Z

On AIX OS, __builtin_cpu_is() references the runtime external variable _system_configuration from /usr/include/sys/systemcfg.h.

ref issue: #80042

llvmbot · 2024-01-30T22:14:04Z

@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: zhijian lin (diggerlin)

Changes

implement the function builtin_cpu_is() for issue #80042

Full diff: https://github.com/llvm/llvm-project/pull/80069.diff

8 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
(modified) clang/lib/Basic/Targets/PPC.cpp (+11-2)
(modified) clang/lib/Basic/Targets/PPC.h (+11-1)
(modified) clang/lib/CodeGen/CGBuiltin.cpp (+55-2)
(modified) clang/lib/Sema/SemaChecking.cpp (+4-1)
(added) clang/test/CodeGen/aix-builtin-cpu-is.c (+83)
(added) clang/test/Sema/aix-builtin-cpu-unsupports.c (+6)
(modified) llvm/include/llvm/TargetParser/PPCTargetParser.def (+53)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1c0ebbe4e6834..139a90a302068 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10347,6 +10347,8 @@ def err_x86_builtin_tile_arg_duplicate : Error<
 
 def err_builtin_target_unsupported : Error<
   "builtin is not supported on this target">;
+def err_builtin_aix_os_unsupported : Error<
+  "this builtin is available only in AIX 7.2 and later operating systems">;
 def err_builtin_longjmp_unsupported : Error<
   "__builtin_longjmp is not supported for the current target">;
 def err_builtin_setjmp_unsupported : Error<
diff --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 8c891ccdeb59d..c9cf743533440 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -904,8 +904,17 @@ bool PPCTargetInfo::validateCpuSupports(StringRef FeatureStr) const {
 }
 
 bool PPCTargetInfo::validateCpuIs(StringRef CPUName) const {
+  llvm::Triple Triple = getTriple();
+  if (Triple.isOSLinux()) {
 #define PPC_LNX_CPU(NAME, NUM) .Case(NAME, true)
-  return llvm::StringSwitch<bool>(CPUName)
+    return llvm::StringSwitch<bool>(CPUName)
 #include "llvm/TargetParser/PPCTargetParser.def"
-      .Default(false);
+        .Default(false);
+  } else if (Triple.isOSAIX()) {
+#define PPC_AIX_CPU(NAME, SUPPORT, INDEX, MASK, OP, VALUE) .Case(NAME, true)
+    return llvm::StringSwitch<bool>(CPUName)
+#include "llvm/TargetParser/PPCTargetParser.def"
+        .Default(false);
+  }
+  return false;
 }
diff --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index a91bdede53e40..5f92c80d15c0f 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -362,8 +362,18 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public TargetInfo {
 
   // 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.
+#define MINIMUM_AIX_OS_MAJOR 7
+#define MINIMUM_AIX_OS_MINOR 2
   bool supportsCpuSupports() const override { return getTriple().isOSGlibc(); }
-  bool supportsCpuIs() const override { return getTriple().isOSGlibc(); }
+  bool supportsCpuIs() const override {
+    llvm::Triple Tri = getTriple();
+    // The AIX7.2 is mininum requirement to support __builtin_cpu_is().
+    return Tri.isOSGlibc() ||
+           (Tri.isOSAIX() &&
+            !Tri.isOSVersionLT(MINIMUM_AIX_OS_MAJOR, MINIMUM_AIX_OS_MINOR));
+  }
+#undef MINIMUM_AIX_OS_MAJOR
+#undef MINIMUM_AIX_OS_MINOR
   bool validateCpuSupports(StringRef Feature) const override;
   bool validateCpuIs(StringRef Name) const override;
 };
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f3ab5ad7b08ec..bab5b144275fd 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -16542,22 +16542,75 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned BuiltinID,
 
   Intrinsic::ID ID = Intrinsic::not_intrinsic;
 
+#include "llvm/TargetParser/PPCTargetParser.def"
+  auto GetOpRes = [&](Value *FieldValue, unsigned Mask, unsigned Op,
+                      unsigned Op_Value) -> Value * {
+    Value *Value1 = FieldValue;
+    if (Mask)
+      Value1 = Builder.CreateAnd(Value1, Mask);
+    assert((Op == OP_EQ) && "Only support equal comparision");
+    return Builder.CreateICmp(ICmpInst::ICMP_EQ, FieldValue,
+                              ConstantInt::get(Int32Ty, Op_Value));
+  };
+
+  auto ConvBuiltinCpu = [&](unsigned SupportOP, unsigned FieldIdx,
+                            unsigned Op_Mask, unsigned Op,
+                            unsigned Op_Value) -> Value * {
+    if (SupportOP == AIX_BUILTIN_PPC_FALSE)
+      return llvm::ConstantInt::getFalse(ConvertType(E->getType()));
+
+    if (SupportOP == AIX_BUILTIN_PPC_TRUE)
+      return llvm::ConstantInt::getTrue(ConvertType(E->getType()));
+
+    assert(SupportOP <= COMP_OP && "Invalid value for SupportOP.");
+    llvm::Type *STy = llvm::StructType::get(PPC_SYSTEMCONFIG_TYPE);
+
+    llvm::Constant *SysConf =
+        CGM.CreateRuntimeVariable(STy, "_system_configuration");
+
+    // Grab the appropriate field from _system_configuration.
+    llvm::Value *Idxs[] = {ConstantInt::get(Int32Ty, 0),
+                           ConstantInt::get(Int32Ty, FieldIdx)};
+
+    llvm::Value *FieldValue = Builder.CreateGEP(STy, SysConf, Idxs);
+    FieldValue = Builder.CreateAlignedLoad(Int32Ty, FieldValue,
+                                           CharUnits::fromQuantity(4));
+
+    return GetOpRes(FieldValue, Op_Mask, Op, Op_Value);
+  };
+
   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)
+    llvm::Triple Triple = getTarget().getTriple();
+    if (Triple.isOSLinux()) {
+      unsigned NumCPUID = StringSwitch<unsigned>(CPUStr)
 #define PPC_LNX_CPU(Name, NumericID) .Case(Name, NumericID)
 #include "llvm/TargetParser/PPCTargetParser.def"
-                            .Default(-1U);
+                              .Default(-1U);
     assert(NumCPUID < -1U && "Invalid CPU name. Missed by SemaChecking?");
     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));
+    } else if (Triple.isOSAIX()) {
+      unsigned IsCpuSupport, FieldIdx, CpuIdMask, CpuIdOp, CpuIdValue;
+      typedef std::tuple<unsigned, unsigned, unsigned, unsigned, unsigned>
+          CPUType;
+      std::tie(IsCpuSupport, FieldIdx, CpuIdMask, CpuIdOp, CpuIdValue) =
+          static_cast<CPUType>(StringSwitch<CPUType>(CPUStr)
+#define PPC_AIX_CPU(NAME, SUPPORT, MASK, INDEX, OP, VALUE)                     \
+  .Case(NAME, {SUPPORT, MASK, INDEX, OP, VALUE})
+#include "llvm/TargetParser/PPCTargetParser.def"
+          );
+      return ConvBuiltinCpu(IsCpuSupport, FieldIdx, CpuIdMask, CpuIdOp,
+                            CpuIdValue);
+    }
+    LLVM_FALLTHROUGH;
   }
   case Builtin::BI__builtin_cpu_supports: {
     unsigned FeatureWord;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 502b24bcdf8b4..a2d3e8133a449 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2165,7 +2165,10 @@ static bool SemaBuiltinCpu(Sema &S, const TargetInfo &TI, CallExpr *TheCall,
     return S.Diag(TheCall->getBeginLoc(), diag::err_builtin_target_unsupported)
            << SourceRange(TheCall->getBeginLoc(), TheCall->getEndLoc());
   if (!IsCPUSupports && !TheTI->supportsCpuIs())
-    return S.Diag(TheCall->getBeginLoc(), diag::err_builtin_target_unsupported)
+    return S.Diag(TheCall->getBeginLoc(),
+                  TI.getTriple().isOSAIX()
+                      ? diag::err_builtin_aix_os_unsupported
+                      : diag::err_builtin_target_unsupported)
            << SourceRange(TheCall->getBeginLoc(), TheCall->getEndLoc());
 
   Expr *Arg = TheCall->getArg(0)->IgnoreParenImpCasts();
diff --git a/clang/test/CodeGen/aix-builtin-cpu-is.c b/clang/test/CodeGen/aix-builtin-cpu-is.c
new file mode 100644
index 0000000000000..87864ba5e2ef0
--- /dev/null
+++ b/clang/test/CodeGen/aix-builtin-cpu-is.c
@@ -0,0 +1,83 @@
+// RUN: echo "int main() { return __builtin_cpu_is(\"ppc970\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefix=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"ppc-cell-be\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefix=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"ppca2\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefix=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"ppc405\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefix=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"ppc440\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefix=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"ppc464\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefix=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"ppc476\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefix=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"power4\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefixes=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"power5\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefixes=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"power5+\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefixes=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"power6\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefixes=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"power6x\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s \
+// RUN:   --check-prefixes=CHECKBOOL
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"power7\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s -DVALUE=32768 \
+// RUN:   --check-prefixes=CHECKOP
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"power8\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s -DVALUE=65536 \
+// RUN:   --check-prefixes=CHECKOP
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"power9\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s -DVALUE=131072\
+// RUN:   --check-prefixes=CHECKOP
+
+// RUN: echo "int main() { return __builtin_cpu_is(\"power10\");}" > %t.c 
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.2.0.0 -emit-llvm -o - %t.c | FileCheck %s -DVALUE=262144 \
+// RUN:   --check-prefixes=CHECKOP
+
+// CHECKBOOL:     define i32 @main() #0 {
+// CHECKBOOL-NEXT: entry:
+// CHECKBOOL-NEXT:   %retval = alloca i32, align 4
+// CHECKBOOL-NEXT:   store i32 0, ptr %retval, align 4
+// CHECKBOOL-NEXT:   ret i32 0 
+// CHECKBOOL-NEXT: }
+
+// CHECKOP: @_system_configuration = external global { i32, i32, i32 }
+// CHECKOP:   define i32 @main() #0 {
+// CHECKOP-NEXT: entry:
+// CHECKOP-NEXT:   %retval = alloca i32, align 4
+// CHECKOP-NEXT:   store i32 0, ptr %retval, align 4
+// CHECKOP-NEXT:   %0 = load i32, ptr getelementptr inbounds ({ i32, i32, i32 }, ptr @_system_configuration, i32 0, i32 1), align 4
+// CHECKOP-NEXT:   %1 = icmp eq i32 %0, [[VALUE]] 
+// CHECKOP-NEXT:  %conv = zext i1 %1 to i32
+// CHECKOP-NEXT:   ret i32 %conv
+// CHECKOP-NEXT: }
+
+
diff --git a/clang/test/Sema/aix-builtin-cpu-unsupports.c b/clang/test/Sema/aix-builtin-cpu-unsupports.c
new file mode 100644
index 0000000000000..2f07ac0f02e2f
--- /dev/null
+++ b/clang/test/Sema/aix-builtin-cpu-unsupports.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -triple  powerpc-ibm-aix7.1.0.0 -verify %s
+
+int main(void) {
+  if (__builtin_cpu_is("power8")) // expected-error {{this builtin is available only in AIX 7.2 and later operating systems}}
+    return 1;
+}
diff --git a/llvm/include/llvm/TargetParser/PPCTargetParser.def b/llvm/include/llvm/TargetParser/PPCTargetParser.def
index f2c44b46fa673..d4f10b4d1fe9e 100644
--- a/llvm/include/llvm/TargetParser/PPCTargetParser.def
+++ b/llvm/include/llvm/TargetParser/PPCTargetParser.def
@@ -126,4 +126,57 @@ PPC_LNX_CPU("power10",47)
 #undef PPC_LNX_DEFINE_OFFSETS
 #undef PPC_LNX_FEATURE
 #undef PPC_LNX_CPU
+
+// Definition of following value are found in the AIX header file <systemcfg.h>
+#ifndef AIX_POWERPC_SYS_CONF
+#define AIX_POWERPC_SYS_CONF
+#define AIX_SYSCON_IMPL_IDX 1
+#define AIX_PPC7_VALUE 0x00008000
+#define AIX_PPC8_VALUE 0x00010000
+#define AIX_PPC9_VALUE 0x00020000
+#define AIX_PPC10_VALUE 0x00040000
+
+#define AIX_BUILTIN_PPC_TRUE 1
+#define AIX_BUILTIN_PPC_FALSE 0
+#define COMP_OP 2
+
+#define OP_EQ  0
+
+#endif
+
+//The value of SUPPORT is COMP_OP, it means the feature depend on the V(INDEX)&MASK OP VALUE
+//If the value of MASK is zero, it means we do not need to do mask, just check V(INDEX) OP VALUE.
+
+#ifndef PPC_AIX_CPU
+#define PPC_AIX_CPU(NAME, SUPPORT, INDEX, MASK, OP, VALUE)
+#endif
+
+//Since the __builtin_cpu_is() is supported only on AIX7.2 or AIX7.2 later OS.
+//And AIX 7.2 is supported only on IBM POWER 7 and later processors.
+//The __builtin_cpu_is() function returns false for other CPUs which are not Power7 and Power7 up.
+
+PPC_AIX_CPU("power4",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("ppc970",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("power5",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("power5+",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("power6",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("ppc-cell-be",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("power6x",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("power7",COMP_OP,AIX_SYSCON_IMPL_IDX,0,OP_EQ,AIX_PPC7_VALUE)
+PPC_AIX_CPU("ppca2",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("ppc405",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("ppc440",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("ppc464",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("ppc476",AIX_BUILTIN_PPC_FALSE,0,0,0,0)
+PPC_AIX_CPU("power8",COMP_OP,AIX_SYSCON_IMPL_IDX,0,OP_EQ,AIX_PPC8_VALUE)
+PPC_AIX_CPU("power9",COMP_OP,AIX_SYSCON_IMPL_IDX,0,OP_EQ,AIX_PPC9_VALUE)
+PPC_AIX_CPU("power10",COMP_OP,AIX_SYSCON_IMPL_IDX,0,OP_EQ,AIX_PPC10_VALUE)
+#undef PPC_AIX_CPU
+
+#ifndef PPC_SYSTEMCONFIG_TYPE
+#define PPC_SYSTEMCONFIG_TYPE \
+Int32Ty, Int32Ty, Int32Ty
+#endif
+
+
 #endif // !PPC_TGT_PARSER_UNDEF_MACROS

clang/lib/Basic/Targets/PPC.h

clang/lib/CodeGen/CGBuiltin.cpp

llvm/include/llvm/TargetParser/PPCTargetParser.def

amy-kwan · 2024-02-05T16:47:05Z

llvm/include/llvm/TargetParser/PPCTargetParser.def

+//Since the __builtin_cpu_is() is supported only on AIX7.2 or AIX7.2 later OS.
+//And AIX 7.2 is supported only on IBM POWER 7 and later processors.
+//The __builtin_cpu_is() function returns false for other CPUs which are not Power7 and Power7 up.


Suggested change

//Since the __builtin_cpu_is() is supported only on AIX7.2 or AIX7.2 later OS.

//And AIX 7.2 is supported only on IBM POWER 7 and later processors.

//The __builtin_cpu_is() function returns false for other CPUs which are not Power7 and Power7 up.

// __builtin_cpu_is() is supported only on Power7 and up.

This might be a bit more clearer.

clang/lib/CodeGen/CGBuiltin.cpp

clang/test/CodeGen/aix-builtin-cpu-is.c

llvm/include/llvm/TargetParser/PPCTargetParser.def

amy-kwan · 2024-02-07T19:34:11Z

llvm/include/llvm/TargetParser/PPCTargetParser.def

+
+//The value of SUPPORT is COMP_OP, it means the feature depend on the V(INDEX)&MASK OP VALUE
+//If the value of MASK is zero, it means we do not need to do mask, just check V(INDEX) OP VALUE.
+


Remove this new line, to clearly show that the comment is applicable to this define specifically.

I think this new line should still be removed.

clang/lib/CodeGen/CGBuiltin.cpp

amy-kwan · 2024-02-07T20:09:35Z

clang/lib/CodeGen/CGBuiltin.cpp

+          CPUType;
+      std::tie(IsCpuSupport, FieldIdx, CpuIdMask, CpuIdOp, CpuIdValue) =
+          static_cast<CPUType>(StringSwitch<CPUType>(CPUStr)
+#define PPC_AIX_CPU(NAME, SUPPORT, MASK, INDEX, OP, VALUE)                     \


Suggested change

#define PPC_AIX_CPU(NAME, SUPPORT, MASK, INDEX, OP, VALUE) \

#define PPC_AIX_CPU(NAME, SUPPORT, MASK, INDEX, COMPARE_OP, VALUE) \

It seems like OP should be called a compare op (here and throughout the patch).

llvm/include/llvm/TargetParser/PPCTargetParser.def

nemanjai

Do we really want to support only __builtin_cpu_is on AIX? It doesn't seem like this would achieve the desired goal. Most users will use these builtins to test for some capability on the target machine. It almost never really matters to a user whether the CPU is a Power10. They are much more likely to care about whether the system supports MMA so they can insert calls to MMA functions; prefixed instructions so they can add them in inline asm, etc.
It is not clear to me what goal is achieved by just providing the processor identification and not its capabilities.

I think it would be better if (in consultation with the AIX team), we determine what it means when the kernel reports that the CPU is for example Power10 and then we emulate __builtin_cpu_supports as well based on that.
For example, a call to __builtin_cpu_supports("mma") ends up emitting a check for whether the CPU is Power10 or above.

clang/include/clang/Basic/DiagnosticSemaKinds.td

nemanjai · 2024-02-08T19:06:34Z

clang/lib/Basic/Targets/PPC.h

@@ -362,8 +362,18 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public TargetInfo {

  // 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.
+#define MINIMUM_AIX_OS_MAJOR 7


We can probably just make these constexpr int variables rather than #defining them.

clang/lib/CodeGen/CGBuiltin.cpp

nemanjai · 2024-02-08T19:18:21Z

clang/lib/CodeGen/CGBuiltin.cpp

@@ -16542,22 +16542,75 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned BuiltinID,

  Intrinsic::ID ID = Intrinsic::not_intrinsic;

+#include "llvm/TargetParser/PPCTargetParser.def"


This is getting a bit tangled. Can you please provide two static functions:
EmitPPCAIXCpuIDFunc(...), EmitPPCLinuxCpuIDFunc(...) and just call the right one from here.

I think the function is too short, It not worth to have a two separate static functions. and I reorganize the code in other way.

clang/lib/CodeGen/CGBuiltin.cpp

clang/include/clang/Basic/DiagnosticSemaKinds.td

clang/lib/Basic/Targets/PPC.cpp

clang/test/CodeGen/aix-builtin-cpu-is.c

diggerlin · 2024-02-09T18:54:47Z

Do we really want to support only __builtin_cpu_is on AIX? It doesn't seem like this would achieve the desired goal. Most users will use these builtins to test for some capability on the target machine. It almost never really matters to a user whether the CPU is a Power10. They are much more likely to care about whether the system supports MMA so they can insert calls to MMA functions; prefixed instructions so they can add them in inline asm, etc. It is not clear to me what goal is achieved by just providing the processor identification and not its capabilities.

I think it would be better if (in consultation with the AIX team), we determine what it means when the kernel reports that the CPU is for example Power10 and then we emulate __builtin_cpu_supports as well based on that. For example, a call to __builtin_cpu_supports("mma") ends up emitting a check for whether the CPU is Power10 or above.

The patch only support __builtin_cpu_is. I will have another two patches to support the __builtin_cpu_supports() later.

github-actions · 2024-02-12T18:43:31Z

✅ With the latest revision this PR passed the C/C++ code formatter.

amy-kwan

Some initial comments for now.

clang/include/clang/Basic/DiagnosticSemaKinds.td

amy-kwan · 2024-02-14T17:51:33Z

clang/lib/CodeGen/CGBuiltin.cpp

+  // The lambda function converts builtin_cpu_is function into directly
+  // returning false or true, or it gets and checks the information from the
+  // kernel variable _system_configuration for AIX OS.


Suggested change

// The lambda function converts builtin_cpu_is function into directly

// returning false or true, or it gets and checks the information from the

// kernel variable _system_configuration for AIX OS.

// This lambda function converts __builtin_cpu_is() into directly

// returning true or false, or it gets and checks the information from the

// kernel variable _system_configuration from the AIX OS.

I'm not sure if this wording sounds better or more accurate, so we can change this but basically I think we can update this comment a bit more so that it makes more sense.

amy-kwan · 2024-02-14T17:52:52Z

clang/lib/CodeGen/CGBuiltin.cpp

+    FieldValue = Builder.CreateAlignedLoad(Int32Ty, FieldValue,
+                                           CharUnits::fromQuantity(4));
+
+    assert((CompOp == COMP_EQ) && "Only equal comparisons are supported!");


I noticed that this does not get set later in this function. Should this be checked earlier rather than later?

I moved the assert up and check earlier

amy-kwan

Group review comments.

llvm/include/llvm/TargetParser/PPCTargetParser.def

amy-kwan · 2024-02-14T19:25:33Z

llvm/include/llvm/TargetParser/PPCTargetParser.def

+#endif
+
+// When the value of SUPPORT_MAGIC is SYS_CONF, the return value depends on the 
+// _system_configuration[INDEX] COMPARE_OP VALUE.


This comment may not make sense here. However, if we do put a comment here, this comment should be clarified, since the comment should describe the intent behind the code.

We should change the COMPARE_OP to be more explicit to show the possible comparison operations.

We should not be using _system_configuration[INDEX], since this doesn't seem to be described in this change.

Where is SUPPORT_MAGIC defined?

llvm/include/llvm/TargetParser/PPCTargetParser.def

amy-kwan · 2024-02-14T19:50:26Z

llvm/include/llvm/TargetParser/PPCTargetParser.def

+// When the value of SUPPORT_MAGIC is SYS_CONF, the return value depends on the 
+// _system_configuration[INDEX] COMPARE_OP VALUE.
+#ifndef PPC_AIX_CPU
+  #define PPC_AIX_CPU(NAME, SUPPORT_MAGIC, INDEX, COMPARE_OP, VALUE)


Change we update this to the following, since it may be more clear?

#define PPC_AIX_CPU(NAME, SUPPORT_METHOD, SYS_CFG_INDEX, COMPARE_OP, SYS_CFG_VALUE)

we will have API call in other patch later, I do not think change INDEX to SYS_CFG_INDEX and VALUE to SYS_CFG_VALUE is a good idea.

amy-kwan · 2024-02-14T19:51:14Z

llvm/include/llvm/TargetParser/PPCTargetParser.def

+
+  #define AIX_BUILTIN_PPC_TRUE 1
+  #define AIX_BUILTIN_PPC_FALSE 0
+  #define SYS_CONF 2


Suggested change

#define SYS_CONF 2

#define USE_SYS_CONF 2

Might be more clear this way.

amy-kwan · 2024-02-14T19:53:42Z

llvm/include/llvm/TargetParser/PPCTargetParser.def

+PPC_AIX_CPU("power6",AIX_BUILTIN_PPC_FALSE,0,0,0)
+PPC_AIX_CPU("ppc-cell-be",AIX_BUILTIN_PPC_FALSE,0,0,0)
+PPC_AIX_CPU("power6x",AIX_BUILTIN_PPC_FALSE,0,0,0)
+PPC_AIX_CPU("power7",SYS_CONF,AIX_SYSCON_IMPL_IDX,COMP_EQ,AIX_PPC7_VALUE)


Can we move this above 167?

llvm/include/llvm/TargetParser/PPCTargetParser.def

clang/lib/Basic/Targets/PPC.h

clang/lib/Basic/Targets/PPC.cpp

amy-kwan · 2024-02-14T20:11:48Z

clang/lib/CodeGen/CGBuiltin.cpp

+    assert(Triple.isOSLinux() && "Triple for AIX OS has already been checked; "
+                                 "it must be Linux OS here.");


Comment should be updated to something like:

__builtin_cpu_is() is only supported for AIX and Linux.

For here and on line 915 on PPC.cpp.

llvm/include/llvm/TargetParser/PPCTargetParser.def

pressed wrong buttong

amy-kwan · 2024-02-16T18:45:27Z

llvm/include/llvm/TargetParser/PPCTargetParser.def

+#endif
+
+// The value of SUPPORT_METHOD can be AIX_BUILTIN_PPC_TRUE,
+// AIX_BUILTIN_PPC_FALSE, Or USE_SYS_CONF.


Suggested change

// AIX_BUILTIN_PPC_FALSE, Or USE_SYS_CONF.

// AIX_BUILTIN_PPC_FALSE, or USE_SYS_CONF.

amy-kwan · 2024-02-16T18:47:51Z

clang/lib/CodeGen/CGBuiltin.cpp

@@ -16542,12 +16542,62 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned BuiltinID,

  Intrinsic::ID ID = Intrinsic::not_intrinsic;

+#include "llvm/TargetParser/PPCTargetParser.def"
+  // This lambda function converts builtin_cpu_is() into directly


Suggested change

// This lambda function converts builtin_cpu_is() into directly

// This lambda function converts __builtin_cpu_is() into directly

amy-kwan · 2024-02-16T18:48:02Z

clang/lib/CodeGen/CGBuiltin.cpp

+#include "llvm/TargetParser/PPCTargetParser.def"
+  // This lambda function converts builtin_cpu_is() into directly
+  // returning true or false, or it gets and checks the information from the
+  // kernel variable _system_configuration fromr the AIX OS.


Suggested change

// kernel variable _system_configuration fromr the AIX OS.

// kernel variable _system_configuration from the AIX OS.

amy-kwan · 2024-02-16T18:57:38Z

llvm/include/llvm/TargetParser/PPCTargetParser.def

+
+//The value of SUPPORT is COMP_OP, it means the feature depend on the V(INDEX)&MASK OP VALUE
+//If the value of MASK is zero, it means we do not need to do mask, just check V(INDEX) OP VALUE.
+


I think this new line should still be removed.

amy-kwan · 2024-02-16T18:58:32Z

clang/lib/CodeGen/CGBuiltin.cpp

+    FieldValue = Builder.CreateAlignedLoad(Int32Ty, FieldValue,
+                                           CharUnits::fromQuantity(4));
+    assert(FieldValue->getType()->isIntegerTy(32) &&
+           "Only supports 32-bit integer in the GenAIXPPCBuiltinCpuExpr.");


Suggested change

"Only supports 32-bit integer in the GenAIXPPCBuiltinCpuExpr.");

"Only 32-bit integers are supported in GenAIXPPCBuiltinCpuExpr().");

I think this may be clearer.

amy-kwan · 2024-02-16T19:01:30Z

llvm/include/llvm/TargetParser/PPCTargetParser.def

+// PPC_SYSTEMCONFIG_TYPE defines the IR data struture of the _system_conf
+// found in the AIX OS header file: </usr/include/sys/systemcfg.h>.


Suggested change

// PPC_SYSTEMCONFIG_TYPE defines the IR data struture of the _system_conf

// found in the AIX OS header file: </usr/include/sys/systemcfg.h>.

// PPC_SYSTEMCONFIG_TYPE defines the IR data structure of _system_conf,

// that is found in the AIX OS header file: </usr/include/sys/systemcfg.h>.

lei137

LGTM
Please address remaining nits before commit.

amy-kwan

Aside from the comments about updating the assert in SemaChecking.cpp and the two comments in PPCTargetParser.def, this also LGTM.

support builtin_cpu_is() for aix

a82a60e

diggerlin requested a review from nemanjai January 30, 2024 22:13

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Jan 30, 2024

diggerlin requested review from daltenty and lei137 January 30, 2024 22:13

diggerlin requested review from stephenpeckham, amy-kwan, mandlebug and syzaara January 31, 2024 00:29

amy-kwan reviewed Feb 5, 2024

View reviewed changes

amy-kwan reviewed Feb 7, 2024

View reviewed changes

nemanjai reviewed Feb 8, 2024

View reviewed changes

amy-kwan reviewed Feb 8, 2024

View reviewed changes

diggerlin added 2 commits February 12, 2024 13:24

address comment

6dfd528

address comment

e0eb549

amy-kwan reviewed Feb 14, 2024

View reviewed changes

address group review comment

e019084

diggerlin requested review from nemanjai and amy-kwan February 16, 2024 14:32

lei137 changed the title ~~[AIX] support builtin_cpu_is() for aix~~ [AIX] support builtin_cpu_is() Feb 16, 2024

diggerlin changed the title ~~[AIX] support builtin_cpu_is()~~ [AIX] Lower intrinsic __builtin_cpu_is into AIX platform-specific code. Feb 16, 2024

lei137 approved these changes Feb 16, 2024

View reviewed changes

llvm/include/llvm/TargetParser/PPCTargetParser.def Show resolved Hide resolved

llvm/include/llvm/TargetParser/PPCTargetParser.def Show resolved Hide resolved

lei137 previously approved these changes Feb 16, 2024

View reviewed changes

another round of address comment

7ebb81a

diggerlin requested a review from lei137 February 16, 2024 18:37

amy-kwan reviewed Feb 16, 2024

View reviewed changes

modify some comments

a33b383

lei137 approved these changes Feb 16, 2024

View reviewed changes

amy-kwan approved these changes Feb 20, 2024

View reviewed changes

address comment

1bc9e3c

llvmbot added the backend:PowerPC label Feb 21, 2024

diggerlin merged commit 5b8e560 into llvm:main Feb 22, 2024


		//The value of SUPPORT is COMP_OP, it means the feature depend on the V(INDEX)&MASK OP VALUE
		//If the value of MASK is zero, it means we do not need to do mask, just check V(INDEX) OP VALUE.

	#define PPC_AIX_CPU(NAME, SUPPORT, MASK, INDEX, OP, VALUE) \
	#define PPC_AIX_CPU(NAME, SUPPORT, MASK, INDEX, COMPARE_OP, VALUE) \

		@@ -16542,22 +16542,75 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned BuiltinID,

		Intrinsic::ID ID = Intrinsic::not_intrinsic;

		#include "llvm/TargetParser/PPCTargetParser.def"

		assert(Triple.isOSLinux() && "Triple for AIX OS has already been checked; "
		"it must be Linux OS here.");

	// AIX_BUILTIN_PPC_FALSE, Or USE_SYS_CONF.
	// AIX_BUILTIN_PPC_FALSE, or USE_SYS_CONF.

	// This lambda function converts builtin_cpu_is() into directly
	// This lambda function converts __builtin_cpu_is() into directly

	// kernel variable _system_configuration fromr the AIX OS.
	// kernel variable _system_configuration from the AIX OS.

	"Only supports 32-bit integer in the GenAIXPPCBuiltinCpuExpr.");
	"Only 32-bit integers are supported in GenAIXPPCBuiltinCpuExpr().");

		// PPC_SYSTEMCONFIG_TYPE defines the IR data struture of the _system_conf
		// found in the AIX OS header file: </usr/include/sys/systemcfg.h>.

[AIX] Lower intrinsic __builtin_cpu_is into AIX platform-specific code. #80069

[AIX] Lower intrinsic __builtin_cpu_is into AIX platform-specific code. #80069

Uh oh!

Conversation

diggerlin commented Jan 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amy-kwan Feb 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nemanjai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

diggerlin commented Feb 9, 2024

Uh oh!

github-actions bot commented Feb 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amy-kwan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amy-kwan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diggerlin commented Jan 30, 2024 •

edited

Loading

llvmbot commented Jan 30, 2024 •

edited

Loading

amy-kwan Feb 5, 2024 •

edited

Loading

github-actions bot commented Feb 12, 2024 •

edited

Loading

amy-kwan Feb 16, 2024 •

edited

Loading