-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[flang] Emit target features for PPC #169860
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-flang-driver Author: Kelvin Li (kkwli) ChangesThis patch is to emit target features for PPC. The feature list is expected to be the same as what is emitted by clang. Full diff: https://github.com/llvm/llvm-project/pull/169860.diff 6 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 438de23be0103..4bb7612999d40 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -544,6 +544,7 @@ void Flang::addTargetOptions(const ArgList &Args,
case llvm::Triple::ppc:
case llvm::Triple::ppc64:
case llvm::Triple::ppc64le:
+ getTargetFeatures(D, Triple, Args, CmdArgs, /*ForAs*/ false);
AddPPCTargetArgs(Args, CmdArgs);
break;
case llvm::Triple::loongarch64:
diff --git a/flang/include/flang/Frontend/TargetOptions.h b/flang/include/flang/Frontend/TargetOptions.h
index f6e5634d5a995..96b94c5f98965 100644
--- a/flang/include/flang/Frontend/TargetOptions.h
+++ b/flang/include/flang/Frontend/TargetOptions.h
@@ -42,6 +42,9 @@ class TargetOptions {
/// the command line.
std::vector<std::string> featuresAsWritten;
+ /// The default target features.
+ std::string targetFeatureStr;
+
/// The real KINDs disabled for this target
std::vector<int> disabledRealKinds;
diff --git a/flang/lib/Frontend/CompilerInstance.cpp b/flang/lib/Frontend/CompilerInstance.cpp
index 5920ed82114f8..f9f6417aaf815 100644
--- a/flang/lib/Frontend/CompilerInstance.cpp
+++ b/flang/lib/Frontend/CompilerInstance.cpp
@@ -28,6 +28,7 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
+#include "llvm/TargetParser/PPCTargetParser.h"
#include "llvm/TargetParser/TargetParser.h"
#include "llvm/TargetParser/Triple.h"
@@ -318,8 +319,67 @@ getExplicitAndImplicitNVPTXTargetFeatures(clang::DiagnosticsEngine &diags,
return llvm::join(featuresVec, ",");
}
+enum ppcCPU {prePwr8, prePwr10};
+static std::optional<ppcCPU> ppcType(std::string &cpu) {
+ return llvm::StringSwitch<std::optional<ppcCPU>>(cpu)
+ .Case("future", std::nullopt)
+ .Case("pwr11", std::nullopt)
+ .Case("pwr10", std::nullopt)
+ .Case("pwr9", prePwr10)
+ .Case("pwr8", prePwr10)
+ .Default(prePwr8);
+}
+
+static std::string
+getExplicitAndImplicitPPCTargetFeatures(clang::DiagnosticsEngine &diags,
+ TargetOptions &targetOpts,
+ const llvm::Triple triple) {
+ std::vector<std::string> featuresVec;
+ std::optional<llvm::StringMap<bool>> FeaturesOpt =
+ llvm::PPC::getPPCDefaultTargetFeatures(triple, targetOpts.cpu);
+ if (FeaturesOpt) {
+ for (auto &I : FeaturesOpt.value()) {
+ featuresVec.push_back(
+ (llvm::Twine(I.second ? "+" : "-") + I.first().str()).str());
+ }
+ }
+ // Check if the feature already exists.
+ for (auto v : targetOpts.featuresAsWritten) {
+ if (std::find(featuresVec.begin(), featuresVec.end(), v) ==
+ featuresVec.end()) {
+ featuresVec.push_back(v);
+ }
+ }
+
+ // Some features are not supported in earlier CPUs.
+ std::map<std::string_view, std::vector<ppcCPU>> unsupportedFeatures{
+ {"+mma", {prePwr8, prePwr10}},
+ {"+paired-vector-memops", {prePwr8, prePwr10}},
+ {"+pcrelative-memops", {prePwr8, prePwr10}},
+ {"+prefix-instrs", {prePwr8, prePwr10}},
+ {"+privileged", {prePwr8}},
+ {"+rop-protect", {prePwr8}}};
+ // Check if there are any unsupported features specified.
+ if (auto cpuType = ppcType(targetOpts.cpu)) {
+ for (auto f : unsupportedFeatures) {
+ for (auto g : f.second) {
+ if (cpuType.value() == g) {
+ if (llvm::is_contained(featuresVec, f.first)) {
+ diags.Report(clang::diag::err_opt_not_valid_on_target) << f.first;
+ return std::string();
+ }
+ }
+ }
+ }
+ }
+
+ llvm::sort(featuresVec);
+ targetOpts.targetFeatureStr = llvm::join(featuresVec, ",");
+ return targetOpts.targetFeatureStr;
+}
+
std::string CompilerInstance::getTargetFeatures() {
- const TargetOptions &targetOpts = getInvocation().getTargetOpts();
+ TargetOptions &targetOpts = getInvocation().getTargetOpts();
const llvm::Triple triple(targetOpts.triple);
// Clang does not append all target features to the clang -cc1 invocation.
@@ -334,13 +394,16 @@ std::string CompilerInstance::getTargetFeatures() {
} else if (triple.isNVPTX()) {
return getExplicitAndImplicitNVPTXTargetFeatures(getDiagnostics(),
targetOpts, triple);
+ } else if (triple.isPPC()) {
+ return getExplicitAndImplicitPPCTargetFeatures(getDiagnostics(),
+ targetOpts, triple);
}
return llvm::join(targetOpts.featuresAsWritten.begin(),
targetOpts.featuresAsWritten.end(), ",");
}
bool CompilerInstance::setUpTargetMachine() {
- const TargetOptions &targetOpts = getInvocation().getTargetOpts();
+ TargetOptions &targetOpts = getInvocation().getTargetOpts();
const std::string &theTriple = targetOpts.triple;
// Create `Target`
@@ -371,6 +434,11 @@ bool CompilerInstance::setUpTargetMachine() {
/*Reloc::Model=*/CGOpts.getRelocationModel(),
/*CodeModel::Model=*/cm, OptLevel));
assert(targetMachine && "Failed to create TargetMachine");
+
+ if (!triple.isPPC()) {
+ targetOpts.targetFeatureStr = targetMachine->getTargetFeatureString();
+ }
+
if (cm.has_value()) {
if ((cm == llvm::CodeModel::Medium || cm == llvm::CodeModel::Large) &&
triple.getArch() == llvm::Triple::x86_64) {
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 6f9dc32297272..cdeff1240cfd8 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -7159,7 +7159,7 @@ Fortran::lower::LoweringBridge::LoweringBridge(
targetOpts.atomicIgnoreDenormalMode);
fir::setAtomicFineGrainedMemory(*module, targetOpts.atomicFineGrainedMemory);
fir::setAtomicRemoteMemory(*module, targetOpts.atomicRemoteMemory);
- fir::setTargetFeatures(*module, targetMachine.getTargetFeatureString());
+ fir::setTargetFeatures(*module, targetOpts.targetFeatureStr);
fir::support::setMLIRDataLayout(*module, targetMachine.createDataLayout());
fir::setIdent(*module, Fortran::common::getFlangFullVersion());
if (cgOpts.RecordCommandLine)
diff --git a/flang/test/Driver/target-cpu-features-invalid.f90 b/flang/test/Driver/target-cpu-features-invalid.f90
index 288da8d57e81d..664c183ff692b 100644
--- a/flang/test/Driver/target-cpu-features-invalid.f90
+++ b/flang/test/Driver/target-cpu-features-invalid.f90
@@ -1,16 +1,17 @@
-! REQUIRES: aarch64-registered-target, amdgpu-registered-target
-
! Test that invalid cpu and features are ignored.
-! RUN: %flang_fc1 -triple aarch64-linux-gnu -target-cpu supercpu \
-! RUN: -o /dev/null -S %s 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-CPU
+! RUN: %if aarch64-registered-target %{ %flang_fc1 -triple aarch64-linux-gnu -target-cpu supercpu \
+! RUN: -o /dev/null -S %s 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-CPU %}
+
+! RUN: %if aarch64-registered-target %{ %flang_fc1 -triple aarch64-linux-gnu -target-feature +superspeed \
+! RUN: -o /dev/null -S %s 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-FEATURE %}
-! RUN: %flang_fc1 -triple aarch64-linux-gnu -target-feature +superspeed \
-! RUN: -o /dev/null -S %s 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-FEATURE
+! RUN: %if amdgpu-registered-target %{ not %flang_fc1 -triple amdgcn-amd-amdhsa -target-feature +wavefrontsize32 \
+! RUN: -target-feature +wavefrontsize64 -o /dev/null -S %s 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-WAVEFRONT %}
-! RUN: not %flang_fc1 -triple amdgcn-amd-amdhsa -target-feature +wavefrontsize32 \
-! RUN: -target-feature +wavefrontsize64 -o /dev/null -S %s 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-WAVEFRONT
+! RUN: %if powerpc-registered-target %{ not %flang_fc1 -triple powerpc64le-linux-gnu -target-feature +mma -target-cpu pwr9 -o /dev/null -S %s 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-PPC-FEATURE %}
! CHECK-INVALID-CPU: 'supercpu' is not a recognized processor for this target (ignoring processor)
! CHECK-INVALID-FEATURE: '+superspeed' is not a recognized feature for this target (ignoring feature)
! CHECK-INVALID-WAVEFRONT: 'wavefrontsize32' and 'wavefrontsize64' are mutually exclusive
+! CHECK-INVALID-PPC-FEATURE: option '+mma' cannot be specified on this target
diff --git a/flang/test/Lower/target-features-ppc.f90 b/flang/test/Lower/target-features-ppc.f90
new file mode 100644
index 0000000000000..1c3429e6936a0
--- /dev/null
+++ b/flang/test/Lower/target-features-ppc.f90
@@ -0,0 +1,15 @@
+! REQUIRES: target=powerpc{{.*}}
+! RUN: %flang_fc1 -emit-fir -target-cpu pwr10 %s -o - | FileCheck %s --check-prefixes=ALL,FEATURE
+! RUN: %flang_fc1 -emit-fir -target-cpu pwr10 -target-feature +privileged %s -o - | FileCheck %s --check-prefixes=ALL,BOTH
+
+! ALL: module attributes {
+
+! ALL: fir.target_cpu = "pwr10"
+
+! FEATURE: fir.target_features = #llvm.target_features<[
+! FEATURE: "+64bit-support", "+allow-unaligned-fp-access", "+altivec", "+bpermd", "+cmpb", "+crbits", "+crypto", "+direct-move", "+extdiv", "+fast-MFLR", "+fcpsgn", "+fpcvt", "+fprnd", "+fpu", "+fre", "+fres", "+frsqrte", "+frsqrtes", "+fsqrt", "+fuse-add-logical", "+fuse-arith-add", "+fuse-logical", "+fuse-logical-add", "+fuse-sha3", "+fuse-store", "+fusion", "+hard-float", "+icbt", "+isa-v206-instructions", "+isa-v207-instructions", "+isa-v30-instructions", "+isa-v31-instructions", "+isel", "+ldbrx", "+lfiwax", "+mfocrf", "+mma", "+paired-vector-memops", "+partword-atomics", "+pcrelative-memops", "+popcntd", "+power10-vector", "+power8-altivec", "+power8-vector", "+power9-altivec", "+power9-vector", "+ppc-postra-sched", "+ppc-prera-sched", "+predictable-select-expensive", "+prefix-instrs", "+quadword-atomics", "+recipprec", "+stfiwx", "+two-const-nr", "+vsx"
+! FEATURE: ]>
+
+! BOTH: fir.target_features = #llvm.target_features<[
+! BOTH: "+64bit-support", "+allow-unaligned-fp-access", "+altivec", "+bpermd", "+cmpb", "+crbits", "+crypto", "+direct-move", "+extdiv", "+fast-MFLR", "+fcpsgn", "+fpcvt", "+fprnd", "+fpu", "+fre", "+fres", "+frsqrte", "+frsqrtes", "+fsqrt", "+fuse-add-logical", "+fuse-arith-add", "+fuse-logical", "+fuse-logical-add", "+fuse-sha3", "+fuse-store", "+fusion", "+hard-float", "+icbt", "+isa-v206-instructions", "+isa-v207-instructions", "+isa-v30-instructions", "+isa-v31-instructions", "+isel", "+ldbrx", "+lfiwax", "+mfocrf", "+mma", "+paired-vector-memops", "+partword-atomics", "+pcrelative-memops", "+popcntd", "+power10-vector", "+power8-altivec", "+power8-vector", "+power9-altivec", "+power9-vector", "+ppc-postra-sched", "+ppc-prera-sched", "+predictable-select-expensive", "+prefix-instrs", "+privileged", "+quadword-atomics", "+recipprec", "+stfiwx", "+two-const-nr", "+vsx"
+! BOTH: ]>
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
jeanPerier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it makes sense to me that target features are set-up and emitted for PPC, I do not get why the way is done has to be modified in a custom way for PPC (see inline comments). I am not saying the changes are not needed, I just do not understand why it is only needed for PPC.
Can you explain what is not going as expected if you where to not add the new targetFeatureStr and set-it up in getExplicitAndImplicitPPCTargetFeatures?
|
|
||
| static std::string | ||
| getExplicitAndImplicitPPCTargetFeatures(clang::DiagnosticsEngine &diags, | ||
| TargetOptions &targetOpts, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it really tricky that this version is modifying the targetOpts while the other similar versions for AMD and NVPTX are not.
Why is this required for PPC and not the other platform with implicit target features?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to get the list from llvm::PPC::getPPCDefaultTargetFeatures for PPC. On other platforms, it relies on the list obtained from the createTargetMachine call.
| if (!triple.isPPC()) { | ||
| targetOpts.targetFeatureStr = targetMachine->getTargetFeatureString(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, it is odd to me that PPC is doing something different than the rest here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to use the list (returned from llvm::PPC::getPPCDefaultTargetFeatures) preserved. For PPC, targetMacine->getTargetFeatureString() returns a longer list.
@jeanPerier Thanks for the review. The issue I encounter is in |
This patch is to emit target features for PPC. The feature list is expected to be the same as what is emitted by clang.