[clang] [llvm] [RISCV] Update TargetAttr target-cpu override rule (PR #75804)
efriedma-quic wrote: It doesn't seem like a good idea to make target-independent logic behave in target-specific ways; that's going to confusing for both people hacking on clang, and for users if it's user-visible. Is there some way we can make this logic consistent across targets? Maybe we can explicitly translate the "arch" feature into enabling/disabling all the relevant features? https://github.com/llvm/llvm-project/pull/75804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [RISCV] Update TargetAttr target-cpu override rule (PR #75804)
https://github.com/BeMg updated https://github.com/llvm/llvm-project/pull/75804
>From 139ab4d26badc0d43c14fc94fe73db32342cfc1e Mon Sep 17 00:00:00 2001
From: Piyou Chen
Date: Sun, 17 Dec 2023 23:12:12 -0800
Subject: [PATCH 1/2] Make target-cpu override rule correct
---
clang/lib/CodeGen/CodeGenModule.cpp | 25
.../RISCV/riscv-func-attr-target-mcpu-rv32.c | 63 +++
.../RISCV/riscv-func-attr-target-mcpu-rv64.c | 63 +++
.../CodeGen/RISCV/riscv-func-attr-target.c| 10 +--
...riscv-func-target-feature-mcpu-override.ll | 36 +++
5 files changed, 192 insertions(+), 5 deletions(-)
create mode 100644 clang/test/CodeGen/RISCV/riscv-func-attr-target-mcpu-rv32.c
create mode 100644 clang/test/CodeGen/RISCV/riscv-func-attr-target-mcpu-rv64.c
create mode 100644
llvm/test/CodeGen/RISCV/riscv-func-target-feature-mcpu-override.ll
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index 7ad26ace328ab2..83a530f1fee45d 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2571,6 +2571,31 @@ bool
CodeGenModule::GetCPUAndFeaturesAttributes(GlobalDecl GD,
if (!ParsedAttr.Tune.empty() &&
getTarget().isValidCPUName(ParsedAttr.Tune))
TuneCPU = ParsedAttr.Tune;
+
+ if (getTarget().getTriple().isRISCV()) {
+// attr-cpu override march only if arch isn't present.
+if (TD->getFeaturesStr().contains("arch=")) {
+ // Before drop attr-cpu, keep it in tune-cpu
+ if (!TargetCPU.empty() && TuneCPU.empty())
+TuneCPU = TargetCPU;
+
+ // Reassign mcpu due to attr-arch= need
+ // target-feature from mcpu/march.
+ // Use attr-cpu will affect target-feature.
+ TargetCPU = getTarget().getTargetOpts().CPU;
+
+ // arch= need keep target feature clean,
+ // use the baseline cpu.
+ if (llvm::find(ParsedAttr.Features,
+ "__RISCV_TargetAttrNeedOverride") !=
+ ParsedAttr.Features.end())
+TargetCPU = (getTarget().getTriple().isRISCV32()) ? "generic-rv32"
+ : "generic-rv64";
+
+ if (TargetCPU == TuneCPU)
+TuneCPU = "";
+}
+ }
}
if (SD) {
diff --git a/clang/test/CodeGen/RISCV/riscv-func-attr-target-mcpu-rv32.c
b/clang/test/CodeGen/RISCV/riscv-func-attr-target-mcpu-rv32.c
new file mode 100644
index 00..abbeb57f54faa3
--- /dev/null
+++ b/clang/test/CodeGen/RISCV/riscv-func-attr-target-mcpu-rv32.c
@@ -0,0 +1,63 @@
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv32 -target-cpu sifive-e76 -target-feature
+zifencei -target-feature +m \
+// RUN: -target-feature +a -target-feature +save-restore -target-feature -zbb
\
+// RUN: -target-feature -relax -target-feature -zfa \
+// RUN: -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define dso_local void @testDefault() #0
+void testDefault() {}
+
+// CHECK: define dso_local void @testFullArchOnly() #1
+__attribute__((target("arch=rv32imac"))) void
+testFullArchOnly() {}
+
+// CHECK: define dso_local void @testFullArchAndCpu() #2
+__attribute__((target("arch=rv32imac;cpu=sifive-e34"))) void
+testFullArchAndCpu() {}
+
+// CHECK: define dso_local void @testFullArchAndTune() #2
+__attribute__((target("arch=rv32imac;tune=sifive-e34"))) void
+testFullArchAndTune() {}
+
+// CHECK: define dso_local void @testFullArchAndCpuAndTune() #2
+__attribute__((target("arch=rv32imac;cpu=sifive-e31;tune=sifive-e34"))) void
+testFullArchAndCpuAndTune() {}
+
+// CHECK: define dso_local void @testAddExtOnly() #3
+__attribute__((target("arch=+v"))) void
+testAddExtOnly() {}
+
+// CHECK: define dso_local void @testAddExtAndCpu() #4
+__attribute__((target("arch=+v;cpu=sifive-e31"))) void
+testAddExtAndCpu() {}
+
+// CHECK: define dso_local void @testAddExtAndTune() #4
+__attribute__((target("arch=+v;tune=sifive-e31"))) void
+testAddExtAndTune() {}
+
+// CHECK: define dso_local void @testAddExtAndCpuAndTune() #5
+__attribute__((target("arch=+v;cpu=sifive-e31;tune=sifive-e34"))) void
+testAddExtAndCpuAndTune() {}
+
+// CHECK: define dso_local void @testCpuOnly() #6
+__attribute__((target("cpu=sifive-e31"))) void
+testCpuOnly() {}
+
+// CHECK: define dso_local void @testCpuAndTune() #7
+__attribute__((target("cpu=sifive-e31;tune=sifive-e34"))) void
+testCpuAndTune() {}
+
+// CHECK: define dso_local void @testTuneOnly() #8
+__attribute__((target("tune=sifive-e34"))) void
+testTuneOnly() {}
+
+// .
+// CHECK: attributes #0 = { {{.*}}"target-cpu"="sifive-e76"
"target-features"="+32bit,+a,+m,+save-restore,+zifencei,-relax,-zbb,-zfa" }
+// CHECK: attributes #1 = { {{.*}}"target-cpu"="generic-rv32"
"target-features"="+32bit,+a,+c,+m,+save-restore,-relax,-zbb,-zfa"
"tune-cpu"="sifive-e76" }
+// CHECK: attributes #2 = { {{.*}}"target-cpu"="generic-rv32"
"target-features"="+
[clang] [llvm] [RISCV] Update TargetAttr target-cpu override rule (PR #75804)
topperc wrote: Is this still needed after #77426 https://github.com/llvm/llvm-project/pull/75804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [RISCV] Update TargetAttr target-cpu override rule (PR #75804)
BeMg wrote: Close due to land https://github.com/llvm/llvm-project/pull/77426 https://github.com/llvm/llvm-project/pull/75804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [RISCV] Update TargetAttr target-cpu override rule (PR #75804)
@@ -482,5 +482,35 @@ ParsedTargetAttr
RISCVTargetInfo::parseTargetAttr(StringRef Features) const {
Ret.Tune = AttrString;
}
}
+
+ StringRef MCPU = this->getTargetOpts().CPU;
+ StringRef MTune = this->getTargetOpts().TuneCPU;
+
+ // attr-cpu override march only if arch isn't present.
+ if (FoundArch) {
+// If tune-cpu infer from CPU, then try to keep it.
+// Otherwise, just use current tune option.
+if (Ret.Tune.empty() && MTune.empty()) {
+ if (!Ret.CPU.empty())
+Ret.Tune = Ret.CPU; // Keep attr-cpu in tune-cpu
+ else if (!MCPU.empty())
+Ret.Tune = MCPU; // Keep mcpu in tune-cpu
+}
+
+// Reassign mcpu due to attr-arch= need
+// target-feature from mcpu/march.
+// Use attr-cpu will affect target-feature.
+Ret.CPU = MCPU;
+
+// arch= need keep target feature clean,
+// use the baseline cpu.
+if (llvm::find(Ret.Features, "__RISCV_TargetAttrNeedOverride") !=
+Ret.Features.end())
+ Ret.CPU =
michaelmaitland wrote:
I think this here is confusing because it is possible to specify a CPU that
gets overridden if there is a full arch string. I think this drops the
scheduler model which was not intended?
https://github.com/llvm/llvm-project/pull/75804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [RISCV] Update TargetAttr target-cpu override rule (PR #75804)
@@ -482,5 +482,35 @@ ParsedTargetAttr
RISCVTargetInfo::parseTargetAttr(StringRef Features) const {
Ret.Tune = AttrString;
}
}
+
+ StringRef MCPU = this->getTargetOpts().CPU;
+ StringRef MTune = this->getTargetOpts().TuneCPU;
+
+ // attr-cpu override march only if arch isn't present.
+ if (FoundArch) {
+// If tune-cpu infer from CPU, then try to keep it.
+// Otherwise, just use current tune option.
+if (Ret.Tune.empty() && MTune.empty()) {
+ if (!Ret.CPU.empty())
+Ret.Tune = Ret.CPU; // Keep attr-cpu in tune-cpu
+ else if (!MCPU.empty())
+Ret.Tune = MCPU; // Keep mcpu in tune-cpu
+}
+
+// Reassign mcpu due to attr-arch= need
+// target-feature from mcpu/march.
+// Use attr-cpu will affect target-feature.
+Ret.CPU = MCPU;
+
+// arch= need keep target feature clean,
+// use the baseline cpu.
+if (llvm::find(Ret.Features, "__RISCV_TargetAttrNeedOverride") !=
+Ret.Features.end())
+ Ret.CPU =
topperc wrote:
The scheduler model should come from Tune shouldn't it?
https://github.com/llvm/llvm-project/pull/75804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [RISCV] Update TargetAttr target-cpu override rule (PR #75804)
@@ -482,5 +482,35 @@ ParsedTargetAttr
RISCVTargetInfo::parseTargetAttr(StringRef Features) const {
Ret.Tune = AttrString;
}
}
+
+ StringRef MCPU = this->getTargetOpts().CPU;
+ StringRef MTune = this->getTargetOpts().TuneCPU;
+
+ // attr-cpu override march only if arch isn't present.
+ if (FoundArch) {
+// If tune-cpu infer from CPU, then try to keep it.
+// Otherwise, just use current tune option.
+if (Ret.Tune.empty() && MTune.empty()) {
+ if (!Ret.CPU.empty())
+Ret.Tune = Ret.CPU; // Keep attr-cpu in tune-cpu
+ else if (!MCPU.empty())
+Ret.Tune = MCPU; // Keep mcpu in tune-cpu
+}
+
+// Reassign mcpu due to attr-arch= need
+// target-feature from mcpu/march.
+// Use attr-cpu will affect target-feature.
+Ret.CPU = MCPU;
+
+// arch= need keep target feature clean,
+// use the baseline cpu.
+if (llvm::find(Ret.Features, "__RISCV_TargetAttrNeedOverride") !=
+Ret.Features.end())
+ Ret.CPU =
topperc wrote:
`RISCVProcessorModel` defines names that can be used for -mcpu and -mtune.
`RISCVTuneProcessorModel` defines name that can only be used for -mtune. These
just associate scheduler models with strings. You need to look at C++ code to
see how -mcpu and -mtune are propagated.
This code gets the TuneCPU from the tune-cpu function attribute if present.
Otherwise its the same as the CPU. Where CPU either comes from the target-cpu
attribute or the TargetCPU in TargetMachine. There is no TuneCPU in
TargetMachine so it can only be set differently through the tune-cpu attribute.
```
const RISCVSubtarget *
RISCVTargetMachine::getSubtargetImpl(const Function &F) const {
Attribute CPUAttr = F.getFnAttribute("target-cpu");
Attribute TuneAttr = F.getFnAttribute("tune-cpu");
Attribute FSAttr = F.getFnAttribute("target-features");
std::string CPU =
CPUAttr.isValid() ? CPUAttr.getValueAsString().str() : TargetCPU;
std::string TuneCPU =
TuneAttr.isValid() ? TuneAttr.getValueAsString().str() : CPU;
```
The lookup function for the scheduler is here
```
void MCSubtargetInfo::InitMCProcessorInfo(StringRef CPU, StringRef TuneCPU,
StringRef FS) {
FeatureBits = getFeatures(CPU, TuneCPU, FS, ProcDesc, ProcFeatures);
FeatureString = std::string(FS);
if (!TuneCPU.empty())
CPUSchedModel = &getSchedModelForCPU(TuneCPU);
else
CPUSchedModel = &MCSchedModel::Default;
}
```
https://github.com/llvm/llvm-project/pull/75804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
