[clang] [clang][Driver] Support simplified triple versions for config files (PR #111387)
https://github.com/Bo98 created https://github.com/llvm/llvm-project/pull/111387
Currently, the config file system loads the full target triple, e.g.
`arm64-apple-darwin23.6.0.cfg`.
This is however not very useful as this is a moving target. In the case of
macOS, that target moves every 2 months.
We can improve this by adding fallbacks that simplify the version component of
the triple. This pull request adds support for loading
`arm64-apple-darwin23.cfg` and `arm64-apple-darwin.cfg`. See the included test
for a demonstration on how it works.
>From 88dd0d33147a7f46a3c9df4aed28ad4e47ef597c Mon Sep 17 00:00:00 2001
From: Bo Anderson
Date: Mon, 7 Oct 2024 15:44:23 +0100
Subject: [PATCH] [clang][Driver] Support simplified triple versions for config
files
---
clang/lib/Driver/Driver.cpp | 53 +++-
clang/test/Driver/config-file3.c | 23 ++
2 files changed, 62 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index a5d43bdac23735..26aa7f67842ce9 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1147,6 +1147,34 @@ bool Driver::loadConfigFiles() {
return false;
}
+static bool findTripleConfigFile(llvm::cl::ExpansionContext &ExpCtx,
+ SmallString<128> &ConfigFilePath,
+ llvm::Triple Triple, std::string Suffix) {
+ // First, try the full unmodified triple.
+ if (ExpCtx.findConfigFile(Triple.str() + Suffix, ConfigFilePath))
+return true;
+
+ // Don't continue if we didn't find a parsable version in the triple.
+ VersionTuple OSVersion = Triple.getOSVersion();
+ if (!OSVersion.getMinor().has_value())
+return false;
+
+ std::string BaseOSName = Triple.getOSTypeName(Triple.getOS()).str();
+
+ // Next try strip the version to only include the major component.
+ // e.g. arm64-apple-darwin23.6.0 -> arm64-apple-darwin23
+ if (OSVersion.getMajor() != 0) {
+Triple.setOSName(BaseOSName + llvm::utostr(OSVersion.getMajor()));
+if (ExpCtx.findConfigFile(Triple.str() + Suffix, ConfigFilePath))
+ return true;
+ }
+
+ // Finally, try without any version suffix at all.
+ // e.g. arm64-apple-darwin23.6.0 -> arm64-apple-darwin
+ Triple.setOSName(BaseOSName);
+ return ExpCtx.findConfigFile(Triple.str() + Suffix, ConfigFilePath);
+}
+
bool Driver::loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx) {
// Disable default config if CLANG_NO_DEFAULT_CONFIG is set to a non-empty
// value.
@@ -1158,7 +1186,7 @@ bool
Driver::loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx) {
return false;
std::string RealMode = getExecutableForDriverMode(Mode);
- std::string Triple;
+ llvm::Triple Triple;
// If name prefix is present, no --target= override was passed via CLOptions
// and the name prefix is not a valid triple, force it for backwards
@@ -1169,15 +1197,13 @@ bool
Driver::loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx) {
llvm::Triple PrefixTriple{ClangNameParts.TargetPrefix};
if (PrefixTriple.getArch() == llvm::Triple::UnknownArch ||
PrefixTriple.isOSUnknown())
- Triple = PrefixTriple.str();
+ Triple = PrefixTriple;
}
// Otherwise, use the real triple as used by the driver.
- if (Triple.empty()) {
-llvm::Triple RealTriple =
-computeTargetTriple(*this, TargetTriple, *CLOptions);
-Triple = RealTriple.str();
-assert(!Triple.empty());
+ if (Triple.str().empty()) {
+Triple = computeTargetTriple(*this, TargetTriple, *CLOptions);
+assert(!Triple.str().empty());
}
// Search for config files in the following order:
@@ -1192,21 +1218,21 @@ bool
Driver::loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx) {
// Try loading -.cfg, and return if we find a match.
SmallString<128> CfgFilePath;
- std::string CfgFileName = Triple + '-' + RealMode + ".cfg";
- if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath))
+ if (findTripleConfigFile(ExpCtx, CfgFilePath, Triple,
+ "-" + RealMode + ".cfg"))
return readConfigFile(CfgFilePath, ExpCtx);
bool TryModeSuffix = !ClangNameParts.ModeSuffix.empty() &&
ClangNameParts.ModeSuffix != RealMode;
if (TryModeSuffix) {
-CfgFileName = Triple + '-' + ClangNameParts.ModeSuffix + ".cfg";
-if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath))
+if (findTripleConfigFile(ExpCtx, CfgFilePath, Triple,
+ "-" + ClangNameParts.ModeSuffix + ".cfg"))
return readConfigFile(CfgFilePath, ExpCtx);
}
// Try loading .cfg, and return if loading failed. If a matching file
// was not found, still proceed on to try .cfg.
- CfgFileName = RealMode + ".cfg";
+ std::string CfgFileName = RealMode + ".cfg";
if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath)) {
if (readConfigFile(CfgFilePath, ExpCtx))
return true;
@@ -1218,8 +1244,7 @@ bool
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
https://github.com/Bo98 created https://github.com/llvm/llvm-project/pull/111397
`clang-cl` sets the default triple to `-pc-windows-msvc`. The host triple
was however being used instead when loading config.
Move the default triple calculation earlier to handle this.
>From a3e8b860788934d7cc1489f850f00dcfd9d8b595 Mon Sep 17 00:00:00 2001
From: Bo Anderson
Date: Mon, 7 Oct 2024 17:01:56 +0100
Subject: [PATCH] [clang][Driver] Fix triple config loading for clang-cl
---
clang/lib/Driver/Driver.cpp | 35 +--
.../config-cl/arm64ec-pc-windows-msvc.cfg | 0
.../Inputs/config-cl/config-arm64ec-arg.cfg | 1 +
clang/test/Driver/config-file.c | 10 ++
4 files changed, 35 insertions(+), 11 deletions(-)
create mode 100644
clang/test/Driver/Inputs/config-cl/arm64ec-pc-windows-msvc.cfg
create mode 100644 clang/test/Driver/Inputs/config-cl/config-arm64ec-arg.cfg
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index a5d43bdac23735..541e79ec97056b 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1247,6 +1247,19 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) {
CLOptions = std::make_unique(
ParseArgStrings(ArgList.slice(1), /*UseDriverMode=*/true,
ContainsError));
+ // We want to determine the triple early so that we load the correct config.
+ if (IsCLMode()) {
+// clang-cl targets MSVC-style Win32.
+llvm::Triple T(TargetTriple);
+T.setOS(llvm::Triple::Win32);
+T.setVendor(llvm::Triple::PC);
+T.setEnvironment(llvm::Triple::MSVC);
+T.setObjectFormat(llvm::Triple::COFF);
+if (CLOptions->hasArg(options::OPT__SLASH_arm64EC))
+ T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
+TargetTriple = T.str();
+ }
+
// Try parsing configuration file.
if (!ContainsError)
ContainsError = loadConfigFiles();
@@ -1286,6 +1299,16 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) {
appendOneArg(Args, Opt, nullptr);
}
}
+
+// The config file may have changed the architecture so apply it.
+if (HasConfigFile && Args.hasArg(options::OPT__SLASH_arm64EC)) {
+ llvm::Triple T(TargetTriple);
+ if (T.getArch() != llvm::Triple::aarch64 ||
+ T.getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) {
+T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
+TargetTriple = T.str();
+ }
+}
}
// Check for working directory option before accessing any files
@@ -1336,17 +1359,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) {
// FIXME: TargetTriple is used by the target-prefixed calls to as/ld
// and getToolChain is const.
- if (IsCLMode()) {
-// clang-cl targets MSVC-style Win32.
-llvm::Triple T(TargetTriple);
-T.setOS(llvm::Triple::Win32);
-T.setVendor(llvm::Triple::PC);
-T.setEnvironment(llvm::Triple::MSVC);
-T.setObjectFormat(llvm::Triple::COFF);
-if (Args.hasArg(options::OPT__SLASH_arm64EC))
- T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
-TargetTriple = T.str();
- } else if (IsDXCMode()) {
+ if (IsDXCMode()) {
// Build TargetTriple from target_profile option for clang-dxc.
if (const Arg *A = Args.getLastArg(options::OPT_target_profile)) {
StringRef TargetProfile = A->getValue();
diff --git a/clang/test/Driver/Inputs/config-cl/arm64ec-pc-windows-msvc.cfg
b/clang/test/Driver/Inputs/config-cl/arm64ec-pc-windows-msvc.cfg
new file mode 100644
index 00..e69de29bb2d1d6
diff --git a/clang/test/Driver/Inputs/config-cl/config-arm64ec-arg.cfg
b/clang/test/Driver/Inputs/config-cl/config-arm64ec-arg.cfg
new file mode 100644
index 00..2eb4402ddf6cd7
--- /dev/null
+++ b/clang/test/Driver/Inputs/config-cl/config-arm64ec-arg.cfg
@@ -0,0 +1 @@
+/arm64EC
diff --git a/clang/test/Driver/config-file.c b/clang/test/Driver/config-file.c
index 9df830ca4c7538..cbad0e408b9ab4 100644
--- a/clang/test/Driver/config-file.c
+++ b/clang/test/Driver/config-file.c
@@ -82,3 +82,13 @@
// CHECK-TWO-CONFIGS: -isysroot
// CHECK-TWO-CONFIGS-SAME: /opt/data
// CHECK-TWO-CONFIGS-SAME: -Wall
+
+
+//--- clang-cl loads the correct triple
+// RUN: env -u CLANG_NO_DEFAULT_CONFIG %clang_cl /arm64EC
--config-system-dir=%S/Inputs/config-cl --config-user-dir= -v 2>&1 | FileCheck
%s -check-prefix CHECK-CL --implicit-check-not 'Configuration file:'
+// CHECK-CL: Configuration file:
{{.*}}Inputs{{.}}config-cl{{.}}arm64ec-pc-windows-msvc.cfg
+
+//--- clang-cl configs support setting /arm64EC
+// RUN: %clang_cl --config-system-dir=%S/Inputs/config-cl --config-user-dir=
--config=config-arm64ec-arg.cfg -v 2>&1 | FileCheck %s -check-prefix
CHECK-CL-FLAG --implicit-check-not 'Configuration file:'
+// CHECK-CL-FLAG: Target: arm64ec-pc-windows-msvc
+// CHECK-CL-FLAG: Configuration file:
{{.*}}Inputs{{.}}config-cl{{.}}config-arm64ec-arg.cfg
___
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
https://github.com/Bo98 edited https://github.com/llvm/llvm-project/pull/111397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
https://github.com/Bo98 edited https://github.com/llvm/llvm-project/pull/111397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
https://github.com/Bo98 edited https://github.com/llvm/llvm-project/pull/111397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
https://github.com/Bo98 edited https://github.com/llvm/llvm-project/pull/111397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
https://github.com/Bo98 edited https://github.com/llvm/llvm-project/pull/111397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Support simplified triple versions for config files (PR #111387)
https://github.com/Bo98 edited https://github.com/llvm/llvm-project/pull/111387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Support simplified triple versions for config files (PR #111387)
Bo98 wrote: Ping? https://github.com/llvm/llvm-project/pull/111387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
Bo98 wrote: Ping? https://github.com/llvm/llvm-project/pull/111397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Darwin][Driver][clang] Prioritise `-isysroot` over `--sysroot` consistently (PR #115993)
Bo98 wrote: > The expected flow on Apple platforms is to only pass an `isysroot` argument > whether it's inherited from `xcrun -sdk clang` or passed > explicitly. Could homebrew instead only pass `isysroot` for Darwin targets? > Or check that `sysroot` and `isysroot` inputs are the same when creating an > invocation. It's unclear to me how homebrew gets into a situation where there > are conflicting sdks passed for `sysroot` and `isysroot` when you effectively > want to ignore whatever is passed to `sysroot`. In the past it was as simple as it happens when setting `DEFAULT_SYSROOT`. If only `isysroot` is supported on macOS, does this mean `DEFAULT_SYSROOT` should be deprecated on Apple platforms? https://github.com/llvm/llvm-project/pull/115993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Darwin][Driver][clang] Prioritise `-isysroot` over `--sysroot` consistently (PR #115993)
Bo98 wrote:
> I don't think it's deprecated in the sense we have plans to drop support for
> it. The problem here seems more like when those values conflict what should
> be expected?
>
> Today it seems expected that `sysroot` will be used for library search but
> not header search, which also matches the other platforms. I don't know if
> this adds confusion or helps unblock, but clang on Darwin also respects the
> env var `SDKROOT` which translates to `isysroot`.
I do agree with this and this does make sense. The problem has mostly come from
two expectations:
* `DEFAULT_SYSROOT` was expected by some users to support setting a default
SDK. Maybe this wasn't actually intended, but it came common enough knowledge
to eventually be explicitly recommended in parts of LLVM documentation:
https://github.com/llvm/llvm-project/blob/ee9e7867178bee16cc0976a83a8303c99c2f5326/flang/docs/GettingStarted.md?plain=1#L91-L93.
* `-isysroot` generally seems to preferred on macOS over `--sysroot`, despite
that not being the case on other platforms.
However those two points are incompatible. And `DEFAULT_SYSROOT` was, until the
introduction of config files recently, the only way to set a default as the
Apple's SDK driver is closed source. We used `DEFAULT_SYSROOT` since the SDK
changes in 10.14, but config files are becoming a possible alternative with
#111387.
---
This probably crosses into #38193, but to paint the wider picture since I agree
the problem here is more complicated than a prioritisation problem, it's
generally expected from various build scripts that something like this:
```clang
#include
int main() {
printf("Hello, World!");
return 0;
}
```
will just work out of the box without needing extra flags or to pipe it through
other commands. It works out of the box on Linux and likely most other
platforms but does not on macOS.
https://github.com/llvm/llvm-project/pull/115993
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Darwin][Driver][clang] Prioritise `-isysroot` over `--sysroot` consistently (PR #115993)
Bo98 wrote: > There is no standard location for macOS SDK, and it can be anywhere you want. > DEFAULT_SYSROOT can't fix any problem for distribution. To be clear: the build I talked about there required the CLT where the SDK location is standardised, so this isn't entirely true for the use case I had. But yes a general LLVM solution would require something more like `xcrun`. The major-fixed `MacOSX.sdk` (e.g. `MacOSX15.sdk`) distributed with the CLT was in fact something we requested from Apple a few years back and was implemented for the precise use case where downstream tools (mostly GCC but it ended up being used for LLVM too) needed a fixed location. (I don't have the `rdar` number on hand as I don't have the conversation saved but hopefully you get what I mean here) > all SDKs are backwards compatible to older OS versions, which allows you to > build software for older OS on latest version This is mostly true but there are notable exceptions such as: * `libcurl` does not have OS availability APIs and it is known that using `libcurl` headers can lead to runtime crashes on older OS * `unguarded-availability-new` is a warning by default and not all configure scripts set `-Werror` for it so will not detect feature availability properly And we have seen these break real world applications, even high profile ones like `git`. This doesn't really change anything here though. Distributing `clang` in a way that doesn't need extra flags is still an expectation the vast majority of downstream build scripts expect, regardless what SDK that is. > If you intended to make this less a problem for unsuspecting users, I would > change to pass -isysroot on Darwin, instead of using `--sysroot` for > `DEFAULT_SYSROOT`. That should be safe enough to not disrupt any users. `DEFAULT_SYSROOT` is a LLVM feature. It cannot be made to use `-isysroot`. Migrating to config files does give us the option to use `-isysroot` instead and we will do that. The v1 initial port to config files was simply a 1:1 behavioural match to the previous `DEFAULT_SYSROOT` behaviour. We've been contributing patches upstream to make config files work better for this use case (such as the separate Darwin PR linked above) and getting feedback from users from edge cases where config files still aren't being applied correctly such as `clang-tidy`. https://github.com/llvm/llvm-project/pull/115993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Darwin][Driver][clang] Prioritise `-isysroot` over `--sysroot` consistently (PR #115993)
Bo98 wrote: That proposal makes sense to me. https://github.com/llvm/llvm-project/pull/115993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
@@ -1336,17 +1359,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) {
// FIXME: TargetTriple is used by the target-prefixed calls to as/ld
// and getToolChain is const.
- if (IsCLMode()) {
-// clang-cl targets MSVC-style Win32.
-llvm::Triple T(TargetTriple);
-T.setOS(llvm::Triple::Win32);
-T.setVendor(llvm::Triple::PC);
-T.setEnvironment(llvm::Triple::MSVC);
-T.setObjectFormat(llvm::Triple::COFF);
-if (Args.hasArg(options::OPT__SLASH_arm64EC))
- T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
-TargetTriple = T.str();
- } else if (IsDXCMode()) {
+ if (IsDXCMode()) {
Bo98 wrote:
It does but it relies on argument parsing so is trickier to move and I didn't
have a DXC build on hand. I can have a look again though and see if I can get a
build going.
I guess this is the same problem as above. It would be trivial to move this to
`computeTargetTriple` but it seems this changes `TargetTriple` instead.
https://github.com/llvm/llvm-project/pull/111397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
@@ -1286,6 +1299,16 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) {
appendOneArg(Args, Opt, nullptr);
}
}
+
+// The config file may have changed the architecture so apply it.
+if (HasConfigFile && Args.hasArg(options::OPT__SLASH_arm64EC)) {
+ llvm::Triple T(TargetTriple);
+ if (T.getArch() != llvm::Triple::aarch64 ||
+ T.getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) {
+T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
+TargetTriple = T.str();
Bo98 wrote:
There's two triple systems in the driver: `TargetTriple` and the triple fed to
`ToolChain` (computed by `computeTargetTriple`). The former is feeds into the
latter but is otherwise separate.
`-m32` and `-m64` is already handled in `computeTargetTriple` and so never
affect `TargetTriple`. The code here was to keep the existing behaviour of
`/arm64EC` affecting `TargetTriple`.
To be honest: I have no idea why there is two triples. It would be simpler if
`TargetTriple` became `DefaultTriple` (and remains immutable),
`Driver.getTargetTriple()` was removed and we just moved all this to
`computeTargetTriple` but I was hesitant to change something I don't understand
the history of.
https://github.com/llvm/llvm-project/pull/111397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver] Fix triple config loading for clang-cl (PR #111397)
https://github.com/Bo98 edited https://github.com/llvm/llvm-project/pull/111397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
