[clang] [compiler-rt] [ubsan] Display correct runtime messages for negative _BitInt (PR #93612)
mstorsjo wrote: > > Indeed, Clang doesn't provide int128_t for 32 bit targets, AFAIK. > > It looks like a little bit more complex. I checked with Standalone-i386 and > AddressSanitizer-i386 targets. They do have int128_t for 32 bit targets as > soon as clang itself built as 64-bit binary. That doesn't quite reflect my experience - the bitness of the host compiler binary should not affect what features are available in the target environment: ``` $ file -L bin/clang bin/clang: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, BuildID[xxHash]=724b58c4c8ae1673, not stripped $ bin/clang -target i386-linux-gnu -E -dM - < /dev/null | grep INT128 $ bin/clang -target x86_64-linux-gnu -E -dM - < /dev/null | grep INT128 #define __SIZEOF_INT128__ 16 $ bin/clang -target i386-windows -E -dM - < /dev/null | grep INT128 $ bin/clang -target x86_64-windows -E -dM - < /dev/null | grep INT128 #define __SIZEOF_INT128__ 16 $ cat int128.c __int128_t a = 42; $ bin/clang -target x86_64-linux-gnu -c int128.c $ bin/clang -target i386-linux-gnu -c int128.c int128.c:1:1: error: unknown type name '__int128_t' 1 | __int128_t a = 42; | ^ 1 error generated. ``` I'm not quite sure what kind of test setup you have - it sounds like it has detected feature based on the compiler's default target, regardless of what the target really supports? https://github.com/llvm/llvm-project/pull/93612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [ubsan] Display correct runtime messages for negative _BitInt (PR #93612)
mstorsjo wrote: > @mstorsjo It seems your compiler build does not have int128_t enabled. Indeed, Clang doesn't provide int128_t for 32 bit targets, AFAIK. > Could you please test #96240 in your environment to verify it has no such > problem? That PR does seem to work fine, in such an environment, in a quick adhoc test setup - thanks! https://github.com/llvm/llvm-project/pull/93612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Set a predefined __GXX_TYPEINFO_EQUALITY_INLINE=0 for… (PR #96062)
mstorsjo wrote: > Do you already know in which Clang versions this will be fixed? Also thx for > fixing this The fix will be in the upcoming 19.x which is going to release candidates in August and might be released in September/October. (Although, downstreams may pick it up and backport the patch to earlier releases - msys2 does this if the fix is relevant to them.) https://github.com/llvm/llvm-project/pull/96062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Set a predefined __GXX_TYPEINFO_EQUALITY_INLINE=0 for… (PR #96062)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/96062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [ubsan] Display correct runtime messages for negative _BitInt (PR #93612)
mstorsjo wrote: In addition to the issue noted on buildbots, this also caused failed tests on i386: https://github.com/mstorsjo/llvm-mingw/actions/runs/9606458336/job/26504129718#step:8:1501 There seem to be a couple of different errors there: ``` bit-int.c:72:19: runtime error: implicit conversion from type 'unsigned _BitInt(37)' of value [21707795506135039](tel:21707795506135039) (64-bit, unsigned) to type '_BitInt(37)' changed the value to -1 (64-bit, signed) AddressSanitizer: CHECK failed: ubsan_value.cpp:87 "((0 && "libclang_rt.ubsan was built without __int128 support")) != (0)" (0x0, 0x0) (tid=2296) ``` https://github.com/llvm/llvm-project/pull/93612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [llvm] [runtimes] remove workaround for old CMake when setting `--unwindlib=none` (PR #93429)
mstorsjo wrote: > Thanks a lot for the fix, I've pulled it into the PR (also rebased on main > while I'm at it) - and CI didn't fail as previously, so this looks great! > Thank you! Ok, good! Btw, it might be good to add a reference to https://gitlab.kitware.com/cmake/cmake/-/issues/23454 in the new commit message too (or perhaps even in a comment in libunwind/CMakeLists.txt?), to clarify that this is indeed not a bug but expected behaviour. https://github.com/llvm/llvm-project/pull/93429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [libcxx] [libunwind] [llvm] [openmp] [runtimes] remove workaround for old CMake when setting `--unwindlib=none` (PR #93429)
mstorsjo wrote: > Hm, this seems to affect some feature detection (and constraints) later on, > though I don't see why/how removing the specification of `--unwindlib=none` > to the driver (leaving it only to the linker) would cause that: > > https://github.com/llvm/llvm-project/blob/1c046ca3f3254944483251bdc9c843e72d7f7796/libunwind/src/CMakeLists.txt#L99-L106 I actually tested doing this cleanup back around when I wrote this - sorry I never got around to amending the comment here explaining what has to be done. The case with libunwind is a bit non-obvious. I rebased my test change from 2022, see https://github.com/mstorsjo/llvm-project/commit/runtimes-link-options-unwindlib-none. Can you try this out and see if it fixes the issue you're seeing? https://github.com/llvm/llvm-project/pull/93429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Set a predefined __GXX_TYPEINFO_EQUALITY_INLINE=0 for… (PR #96062)
@@ -926,6 +926,12 @@ static void InitializePredefinedMacros(const TargetInfo
,
if (LangOpts.GNUCVersion && LangOpts.CPlusPlus11)
Builder.defineMacro("__GXX_EXPERIMENTAL_CXX0X__");
+ if (TI.getTriple().isWindowsGNUEnvironment() && LangOpts.CPlusPlus) {
mstorsjo wrote:
Ok, done. Yeah while it doesn't serve much purpose in non-c++ mode, it doesn't
hurt either and simplifies things a little bit.
https://github.com/llvm/llvm-project/pull/96062
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Set a predefined __GXX_TYPEINFO_EQUALITY_INLINE=0 for… (PR #96062)
https://github.com/mstorsjo updated
https://github.com/llvm/llvm-project/pull/96062
From 7e6050d7e6274fe2f9b66ebecc92152f6363f025 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Wed, 19 Jun 2024 14:34:12 +0300
Subject: [PATCH] [clang] [MinGW] Set a predefined
__GXX_TYPEINFO_EQUALITY_INLINE=0 for MinGW targets
libstdc++ requires this define to match what is predefined in
GCC for the ABI of this platform; GCC hardcodes this define
for all mingw configurations in gcc/config/i386/cygming.h.
(It also defines __GXX_MERGED_TYPEINFO_NAMES=0, but that happens
to match the defaults in libstdc++ headers, so there's no similar
need to define it in Clang.)
This fixes a Clang/libstdc++ interop issue discussed at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110572.
---
clang/lib/Frontend/InitPreprocessor.cpp | 6 ++
clang/test/Preprocessor/predefined-win-macros.c | 5 +
2 files changed, 11 insertions(+)
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp
b/clang/lib/Frontend/InitPreprocessor.cpp
index e8c8a5175f8f4..ad1849b1a30f9 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -926,6 +926,12 @@ static void InitializePredefinedMacros(const TargetInfo
,
if (LangOpts.GNUCVersion && LangOpts.CPlusPlus11)
Builder.defineMacro("__GXX_EXPERIMENTAL_CXX0X__");
+ if (TI.getTriple().isWindowsGNUEnvironment()) {
+// Set ABI defining macros for libstdc++ for MinGW, where the
+// default in libstdc++ differs from the defaults for this target.
+Builder.defineMacro("__GXX_TYPEINFO_EQUALITY_INLINE", "0");
+ }
+
if (LangOpts.ObjC) {
if (LangOpts.ObjCRuntime.isNonFragile()) {
Builder.defineMacro("__OBJC2__");
diff --git a/clang/test/Preprocessor/predefined-win-macros.c
b/clang/test/Preprocessor/predefined-win-macros.c
index 14e2f584bd093..7d29e45c7d5ac 100644
--- a/clang/test/Preprocessor/predefined-win-macros.c
+++ b/clang/test/Preprocessor/predefined-win-macros.c
@@ -116,6 +116,7 @@
// CHECK-X86-MINGW: #define WINNT 1
// CHECK-X86-MINGW: #define _WIN32 1
// CHECK-X86-MINGW-NOT: #define _WIN64 1
+// CHECK-X86-MINGW: #define __GXX_TYPEINFO_EQUALITY_INLINE 0
// RUN: %clang_cc1 -triple thumbv7-windows-gnu %s -E -dM -o - \
// RUN: | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM-MINGW
@@ -125,6 +126,7 @@
// CHECK-ARM-MINGW: #define WINNT 1
// CHECK-ARM-MINGW: #define _WIN32 1
// CHECK-ARM-MINGW-NOT: #define _WIN64 1
+// CHECK-ARM-MINGW: #define __GXX_TYPEINFO_EQUALITY_INLINE 0
// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - \
// RUN: | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-MINGW
@@ -134,6 +136,7 @@
// CHECK-AMD64-MINGW: #define WINNT 1
// CHECK-AMD64-MINGW: #define _WIN32 1
// CHECK-AMD64-MINGW: #define _WIN64 1
+// CHECK-AMD64-MINGW: #define __GXX_TYPEINFO_EQUALITY_INLINE 0
// RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
// RUN: | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
@@ -145,6 +148,7 @@
// CHECK-ARM64-MINGW: #define _WIN32 1
// CHECK-ARM64-MINGW: #define _WIN64 1
// CHECK-ARM64-MINGW: #define __GCC_ASM_FLAG_OUTPUTS__ 1
+// CHECK-ARM64-MINGW: #define __GXX_TYPEINFO_EQUALITY_INLINE 0
// CHECK-ARM64-MINGW: #define __aarch64__ 1
// RUN: %clang_cc1 -triple arm64ec-windows-gnu %s -E -dM -o - \
@@ -157,6 +161,7 @@
// CHECK-ARM64EC-MINGW: #define _WIN32 1
// CHECK-ARM64EC-MINGW: #define _WIN64 1
// CHECK-ARM64EC-MINGW: #define __GCC_ASM_FLAG_OUTPUTS__ 1
+// CHECK-ARM64EC-MINGW: #define __GXX_TYPEINFO_EQUALITY_INLINE 0
// CHECK-ARM64EC-MINGW-NOT: #define __aarch64__ 1
// CHECK-ARM64EC-MINGW: #define __amd64 1
// CHECK-ARM64EC-MINGW: #define __amd64__ 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Set a predefined __GXX_TYPEINFO_EQUALITY_INLINE=0 for… (PR #96062)
mstorsjo wrote: CC @jwakely https://github.com/llvm/llvm-project/pull/96062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Set a predefined __GXX_TYPEINFO_EQUALITY_INLINE=0 for… (PR #96062)
https://github.com/mstorsjo created
https://github.com/llvm/llvm-project/pull/96062
… MinGW targets
libstdc++ requires this define to match what is predefined in GCC for the ABI
of this platform; GCC hardcodes this define for all mingw configurations in
gcc/config/i386/cygming.h.
(It also defines __GXX_MERGED_TYPEINFO_NAMES=0, but that happens to match the
defaults in libstdc++ headers, so there's no similar need to define it in
Clang.)
This fixes a Clang/libstdc++ interop issue discussed at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110572.
From a0dc374a76d68179c2b7475a7d1e0e55f1622401 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Wed, 19 Jun 2024 14:34:12 +0300
Subject: [PATCH] [clang] [MinGW] Set a predefined
__GXX_TYPEINFO_EQUALITY_INLINE=0 for MinGW targets
libstdc++ requires this define to match what is predefined in
GCC for the ABI of this platform; GCC hardcodes this define
for all mingw configurations in gcc/config/i386/cygming.h.
(It also defines __GXX_MERGED_TYPEINFO_NAMES=0, but that happens
to match the defaults in libstdc++ headers, so there's no similar
need to define it in Clang.)
This fixes a Clang/libstdc++ interop issue discussed at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110572.
---
clang/lib/Frontend/InitPreprocessor.cpp | 6 ++
clang/test/Preprocessor/predefined-win-macros.c | 10 ++
2 files changed, 16 insertions(+)
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp
b/clang/lib/Frontend/InitPreprocessor.cpp
index e8c8a5175f8f4..2d20b56966186 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -926,6 +926,12 @@ static void InitializePredefinedMacros(const TargetInfo
,
if (LangOpts.GNUCVersion && LangOpts.CPlusPlus11)
Builder.defineMacro("__GXX_EXPERIMENTAL_CXX0X__");
+ if (TI.getTriple().isWindowsGNUEnvironment() && LangOpts.CPlusPlus) {
+// Set ABI defining macros for libstdc++ for MinGW, where the
+// default in libstdc++ differs from the defaults for this target.
+Builder.defineMacro("__GXX_TYPEINFO_EQUALITY_INLINE", "0");
+ }
+
if (LangOpts.ObjC) {
if (LangOpts.ObjCRuntime.isNonFragile()) {
Builder.defineMacro("__OBJC2__");
diff --git a/clang/test/Preprocessor/predefined-win-macros.c
b/clang/test/Preprocessor/predefined-win-macros.c
index 14e2f584bd093..12e705875b123 100644
--- a/clang/test/Preprocessor/predefined-win-macros.c
+++ b/clang/test/Preprocessor/predefined-win-macros.c
@@ -163,3 +163,13 @@
// CHECK-ARM64EC-MINGW: #define __arm64ec__ 1
// CHECK-ARM64EC-MINGW: #define __x86_64 1
// CHECK-ARM64EC-MINGW: #define __x86_64__ 1
+
+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - \
+// RUN: | FileCheck -match-full-lines %s --check-prefix=CHECK-MINGW-C
+
+// CHECK-MINGW-C-NOT: #define __GXX_TYPEINFO_EQUALITY_INLINE
+
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -x c++ %s -E -dM -o - \
+// RUN: | FileCheck -match-full-lines %s --check-prefix=CHECK-MINGW-CXX
+
+// CHECK-MINGW-CXX: #define __GXX_TYPEINFO_EQUALITY_INLINE 0
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [llvm][AArch64] Support -mcpu=apple-m4 (PR #95478)
@@ -1010,6 +1034,9 @@ def : ProcessorModel<"apple-a16", CycloneModel, ProcessorFeatures.AppleA16, [TuneAppleA16]>; def : ProcessorModel<"apple-a17", CycloneModel, ProcessorFeatures.AppleA17, [TuneAppleA17]>; +def : ProcessorModel<"apple-m4", CycloneModel, ProcessorFeatures.AppleM4, mstorsjo wrote: Ah, right - sorry I forgot about that distinction. (I guess it'll end up in macs at some point too, and I know you can't comment on that - so until then this is indeed the right sorting.) https://github.com/llvm/llvm-project/pull/95478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [llvm][AArch64] Support -mcpu=apple-m4 (PR #95478)
@@ -1010,6 +1034,9 @@ def : ProcessorModel<"apple-a16", CycloneModel, ProcessorFeatures.AppleA16, [TuneAppleA16]>; def : ProcessorModel<"apple-a17", CycloneModel, ProcessorFeatures.AppleA17, [TuneAppleA17]>; +def : ProcessorModel<"apple-m4", CycloneModel, ProcessorFeatures.AppleM4, mstorsjo wrote: Shouldn't this go below, under `// Mac CPUs`? https://github.com/llvm/llvm-project/pull/95478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add winsysroot alias to the GNU driver (PR #95320)
mstorsjo wrote: What's the deal with constantly force-pushing an update to the PR, with the exact same contents? With the massive inflow of commits in LLVM, you can't rebase your branch constantly on top of the latest one many times per day, I would say. And what's going on when #94731 was closed and now this one is reopened? https://github.com/llvm/llvm-project/pull/95320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [test] Skip a test that sets PATH= on Windows (PR #95096)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/95096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [test] Skip a test that sets PATH= on Windows (PR #95096)
https://github.com/mstorsjo edited https://github.com/llvm/llvm-project/pull/95096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [test] Skip a test that sets PATH= on Windows (PR #95096)
@@ -77,13 +77,3 @@
// XTOR: {{llvm-spirv.*"}}
// BACKEND-NOT: {{llvm-spirv.*"}}
-
-//-
-// Check llvm-spirv- is used if it is found in PATH.
-// RUN: mkdir -p %t/versioned
-// RUN: touch %t/versioned/llvm-spirv-%llvm-version-major \
-// RUN: && chmod +x %t/versioned/llvm-spirv-%llvm-version-major
-// RUN: env "PATH=%t/versioned" %clang -### --target=spirv64 -x cl -c %s 2>&1 \
mstorsjo wrote:
Ok, updated the PR to use `%if !system-windows` instead - tested that it does
seem to work as expected here.
https://github.com/llvm/llvm-project/pull/95096
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [test] Skip a test that sets PATH= on Windows (PR #95096)
https://github.com/mstorsjo updated
https://github.com/llvm/llvm-project/pull/95096
From 11b577825a15bcd6139863adb047ef5f7b161a36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Tue, 11 Jun 2024 13:14:47 +0300
Subject: [PATCH] [clang] [test] Skip a test that sets PATH= on Windows
The same has been done in a couple other existing tests, that also
are skipped on Windows (e.g. ld-path.c). Some tests that really
do want to test setting the path on Windows does it differently,
see e.g. ps4-ps5-linker-win.c.
Since a65771fce4a2f25f16d4b3918ad6a11370637f7b, the spirv-toolchain.cl
test does one test where PATH is set. Setting PATH does work in
some build configurations - however, if built with e.g. llvm-mingw,
the built Clang executable depends on libc++.dll (and libunwind.dll)
which are found in PATH. If the PATH is overridden, the newly built
Clang executable no longer can run.
Split the test that requires setting PATH to a separate file,
and mark it as unsupported on Windows.
---
clang/test/Driver/spirv-toolchain.cl | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/clang/test/Driver/spirv-toolchain.cl
b/clang/test/Driver/spirv-toolchain.cl
index de818177cb19f..eff02f809ce83 100644
--- a/clang/test/Driver/spirv-toolchain.cl
+++ b/clang/test/Driver/spirv-toolchain.cl
@@ -80,10 +80,15 @@
//-
// Check llvm-spirv- is used if it is found in PATH.
+//
+// This test uses the PATH environment variable; on Windows, we may need to
retain
+// the original path for the built Clang binary to be able to execute (as it is
+// used for locating dependent DLLs). Therefore, skip this test on
system-windows.
+//
// RUN: mkdir -p %t/versioned
// RUN: touch %t/versioned/llvm-spirv-%llvm-version-major \
// RUN: && chmod +x %t/versioned/llvm-spirv-%llvm-version-major
-// RUN: env "PATH=%t/versioned" %clang -### --target=spirv64 -x cl -c %s 2>&1 \
-// RUN: | FileCheck -DVERSION=%llvm-version-major --check-prefix=VERSIONED %s
+// RUN: %if !system-windows %{ env "PATH=%t/versioned" %clang -###
--target=spirv64 -x cl -c %s 2>&1 \
+// RUN: | FileCheck -DVERSION=%llvm-version-major --check-prefix=VERSIONED
%s %}
// VERSIONED: {{.*}}llvm-spirv-[[VERSION]]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [test] Skip a test that sets PATH= on Windows (PR #95096)
https://github.com/mstorsjo edited https://github.com/llvm/llvm-project/pull/95096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [test] Skip a test that sets PATH= on Windows (PR #95096)
https://github.com/mstorsjo updated
https://github.com/llvm/llvm-project/pull/95096
From 8e5e09f12124a361c06d833a9ade75bbb338be4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Tue, 11 Jun 2024 13:14:47 +0300
Subject: [PATCH] [clang] [test] Skip a test that sets PATH= on Windows
The same has been done in a couple other existing tests, that also
are skipped on Windows (e.g. ld-path.c). Some tests that really
do want to test setting the path on Windows does it differently,
see e.g. ps4-ps5-linker-win.c.
Since a65771fce4a2f25f16d4b3918ad6a11370637f7b, the spirv-toolchain.cl
test does one test where PATH is set. Setting PATH does work in
some build configurations - however, if built with e.g. llvm-mingw,
the built Clang executable depends on libc++.dll (and libunwind.dll)
which are found in PATH. If the PATH is overridden, the newly built
Clang executable no longer can run.
Split the test that requires setting PATH to a separate file,
and mark it as unsupported on Windows.
---
clang/test/Driver/spirv-toolchain-version.cl | 14 ++
clang/test/Driver/spirv-toolchain.cl | 10 --
2 files changed, 14 insertions(+), 10 deletions(-)
create mode 100644 clang/test/Driver/spirv-toolchain-version.cl
diff --git a/clang/test/Driver/spirv-toolchain-version.cl
b/clang/test/Driver/spirv-toolchain-version.cl
new file mode 100644
index 0..b23c2b9ef5558
--- /dev/null
+++ b/clang/test/Driver/spirv-toolchain-version.cl
@@ -0,0 +1,14 @@
+/// This test uses the PATH environment variable; on Windows, we may need to
retain
+/// the original path for the built Clang binary to be able to execute (as it
is
+/// used for locating dependent DLLs).
+// UNSUPPORTED: system-windows
+
+//-
+// Check llvm-spirv- is used if it is found in PATH.
+// RUN: mkdir -p %t/versioned
+// RUN: touch %t/versioned/llvm-spirv-%llvm-version-major \
+// RUN: && chmod +x %t/versioned/llvm-spirv-%llvm-version-major
+// RUN: env "PATH=%t/versioned" %clang -### --target=spirv64 -x cl -c %s 2>&1 \
+// RUN: | FileCheck -DVERSION=%llvm-version-major --check-prefix=VERSIONED %s
+
+// VERSIONED: {{.*}}llvm-spirv-[[VERSION]]
diff --git a/clang/test/Driver/spirv-toolchain.cl
b/clang/test/Driver/spirv-toolchain.cl
index de818177cb19f..db3ee4d3fe02f 100644
--- a/clang/test/Driver/spirv-toolchain.cl
+++ b/clang/test/Driver/spirv-toolchain.cl
@@ -77,13 +77,3 @@
// XTOR: {{llvm-spirv.*"}}
// BACKEND-NOT: {{llvm-spirv.*"}}
-
-//-
-// Check llvm-spirv- is used if it is found in PATH.
-// RUN: mkdir -p %t/versioned
-// RUN: touch %t/versioned/llvm-spirv-%llvm-version-major \
-// RUN: && chmod +x %t/versioned/llvm-spirv-%llvm-version-major
-// RUN: env "PATH=%t/versioned" %clang -### --target=spirv64 -x cl -c %s 2>&1 \
-// RUN: | FileCheck -DVERSION=%llvm-version-major --check-prefix=VERSIONED %s
-
-// VERSIONED: {{.*}}llvm-spirv-[[VERSION]]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [test] Skip a test that sets PATH= on Windows (PR #95096)
mstorsjo wrote: > Could the single test with the PATH manipulation be moved to a separate test > file which is disabled on Windows? That way we could keep running the other > tests on Windows. That's certainly an option, too. https://github.com/llvm/llvm-project/pull/95096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [test] Skip a test that sets PATH= on Windows (PR #95096)
mstorsjo wrote: CC @linehill https://github.com/llvm/llvm-project/pull/95096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [test] Skip a test that sets PATH= on Windows (PR #95096)
https://github.com/mstorsjo created https://github.com/llvm/llvm-project/pull/95096 The same has been done in a couple other existing tests, that also are skipped on Windows (e.g. ld-path.c). Some tests that really do want to test setting the path on Windows does it differently, see e.g. ps4-ps5-linker-win.c. Since a65771fce4a2f25f16d4b3918ad6a11370637f7b, the spirv-toolchain.cl test does one test where PATH is set. Setting PATH does work in some build configurations - however, if built with e.g. llvm-mingw, the built Clang executable depends on libc++.dll (and libunwind.dll) which are found in PATH. If the PATH is overridden, the newly built Clang executable no longer can run. From 1608c828eaeb6ca37702817384930fa44b0612f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Tue, 11 Jun 2024 13:14:47 +0300 Subject: [PATCH] [clang] [test] Skip a test that sets PATH= on Windows The same has been done in a couple other existing tests, that also are skipped on Windows (e.g. ld-path.c). Some tests that really do want to test setting the path on Windows does it differently, see e.g. ps4-ps5-linker-win.c. Since a65771fce4a2f25f16d4b3918ad6a11370637f7b, the spirv-toolchain.cl test does one test where PATH is set. Setting PATH does work in some build configurations - however, if built with e.g. llvm-mingw, the built Clang executable depends on libc++.dll (and libunwind.dll) which are found in PATH. If the PATH is overridden, the newly built Clang executable no longer can run. --- clang/test/Driver/spirv-toolchain.cl | 5 + 1 file changed, 5 insertions(+) diff --git a/clang/test/Driver/spirv-toolchain.cl b/clang/test/Driver/spirv-toolchain.cl index de818177cb19f..2c9f9db80cad6 100644 --- a/clang/test/Driver/spirv-toolchain.cl +++ b/clang/test/Driver/spirv-toolchain.cl @@ -1,3 +1,8 @@ +/// One test uses the PATH environment variable; on Windows, we may need to retain +/// the original path for the built Clang binary to be able to execute (as it is +/// used for locating dependent DLLs). +// UNSUPPORTED: system-windows + // Check object emission. // RUN: %clang -### --target=spirv64 -x cl -c %s 2>&1 | FileCheck --check-prefix=SPV64 %s // RUN: %clang -### --target=spirv64 %s 2>&1 | FileCheck --check-prefix=SPV64 %s ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Don't use -Wno-nested-anon-types on GCC (PR #95029)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/95029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Don't use -Wno-nested-anon-types on GCC (PR #95029)
https://github.com/mstorsjo created
https://github.com/llvm/llvm-project/pull/95029
GCC usually doesn't warn about unrecognized -Wno- options, if no
diagnostics are printed. However if some diagnostics are printed, it also
mentions that there were unrecognized -Wno- options.
Before 4feae05c6abda364a9295aecfa600d7d4e7dfeb6, we checked for whether
-Wnested-anon-types was supported, and added the -Wno- form if the
positive form of the option was supported.
As of GCC 14, -Wnested-anon-types isn't supported, thus limit the use of the
option to actual Clang (and still only while using the GCC compatible driver).
This avoids unnecessary mentions about unrecognized -Wno- options when
building with GCC.
From 4af34cd1a03064a3f5966f8f33bccdec940def89 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Mon, 10 Jun 2024 22:30:59 +0300
Subject: [PATCH] [clang] Don't use -Wno-nested-anon-types on GCC
GCC usually doesn't warn about unrecognized -Wno- options,
if no diagnostics are printed. However if some diagnostics are
printed, it also mentions that there were unrecognized -Wno-
options.
Before 4feae05c6abda364a9295aecfa600d7d4e7dfeb6, we checked
for whether -Wnested-anon-types was supported, and added the
-Wno- form if the positive form of the option was supported.
As of GCC 14, -Wnested-anon-types isn't supported, thus limit
the use of the option to actual Clang (and still only while using
the GCC compatible driver).
---
clang/CMakeLists.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 2ac0bccb42f50..2b5269f2ce2e7 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -350,7 +350,9 @@ if (LLVM_COMPILER_IS_GCC_COMPATIBLE)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pedantic -Wno-long-long")
endif ()
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-nested-anon-types" )
+ if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-nested-anon-types" )
+ endif ()
endif ()
# Determine HOST_LINK_VERSION on Darwin.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Fix a test issue in mingw configurations (PR #92737)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/92737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Fix a test issue in mingw configurations (PR #92737)
https://github.com/mstorsjo created
https://github.com/llvm/llvm-project/pull/92737
On Windows, long is always 32 bit, thus one can't use long for casting pointers
to integers, on 64 bit architectures.
Instead use long long, which should be large enough.
This avoids errors like "error: cast from pointer to smaller type 'long' loses
information" in this testcase.
This condition only seems to be an error in mingw mode; in MSVC mode
(clang-cl), this is only a warning.
From 392c23804e3531be2786b5ec9712409e222c3dba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Sun, 19 May 2024 00:33:47 +0300
Subject: [PATCH] [analyzer] Fix a test issue in mingw configurations
On Windows, long is always 32 bit, thus one can't use long for
casting pointers to integers, on 64 bit architectures.
Instead use long long, which should be large enough.
This avoids errors like "error: cast from pointer to smaller type
'long' loses information" in this testcase.
This condition only seems to be an error in mingw mode; in MSVC
mode (clang-cl), this is only a warning.
---
clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
index 03aee56a200f6..b13e7123ee524 100644
--- a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
+++ b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
@@ -90,7 +90,7 @@ void reportDescriptiveName(int *p);
extern int* ptr;
extern int array[3];
void top() {
- reportDescriptiveName([(long)ptr]);
+ reportDescriptiveName([(long long)ptr]);
})cpp";
std::string Output;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Driver] Restore compiler-rt arch suffix for PS and Windows (PR #89775)
mstorsjo wrote: One reason why I’m not entirely thrilled (not firmly against, but not thrilled - but I’m not sure if there are much better options either) with this direction, is that it becomes messy when cross compiling. LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is automatically set based on the host OS, e.g. if building on Linux, it defaults to true, while if building on Windows, it defaults to false. When building Clang separately from the runtimes, this decision comes even more as a surprise; if I first build a Linux Clang, then later build the runtimes for Windows (for a Linux hosted cross toolchain targeting Windows), this might mismatch. For the change that currently is suggested, this mismatch might be benign and not matter for this particular setup/direction, but if we extend this pattern to skip all sorts of probing and just assume one layout or the other, it will matter. I guess the solution to that will be that when I build the clang binary first, I need to set LLVM_ENABLE_PER_TARGET_RUNTIME_DIR based on how I’m going to build my runtimes later. That’s probably acceptable, even though less convenient than now. However, if I instead have a Clang binary from e.g. a Linux distribution, and I want to build Windows runtimes to use with it, I would need a way to query which layout is hardcoded into the binary. There are various options for querying paths from the Clang executable, but it’s not very obvious to figure out which layout it will require, or whether it might support both. https://github.com/llvm/llvm-project/pull/89775 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Restore compiler-rt arch suffix for PS and Windows (PR #89775)
mstorsjo wrote: > > The recent changes, #81037 and #87866, were (as far as I know) only > > intended to change what is printed as error messages, when neither is > > found, to help users correct their setup for the new style layout. But > > those changes also seem to have unexpected effects in changing e.g. what > > gets emitted as embedded directive, when the compiler doesn't see either of > > them at compile time (with e.g. distributed build systems), while they > > might be available at link time. > > This matches my understanding. I am not aware of the embedded directive? Do > you embed a path to a compiler-rt library to the generated object files? Clang does this, in a number of cases. In the MSVC ecosystem, one usually invokes `link` or `lld-link` directly, instead of using `clang` to drive the link - therefore, in order to pass implicit libraries to link, like asan/profile, directives are embedded into the generated object files, that tells the linker to link in those libraries. > I think while technically a new clang can use an old compiler-rt file > hierarchy, technically it is unsupported: kinda like that a very old libc++ > may not be built with a new Clang. I don't think anybody is arguing that a new clang should use an old compiler-rt install, but it should be able to use a new install with the layout according to `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` disabled. > If we avoid hard-coded library names in compiler-generated relocatable files > (just called "object files" on Windows?). there should be no distributed > build system concern. We can't avoid this - we already have this situation. See https://github.com/llvm/llvm-project/pull/87866#issuecomment-2072626122 - #87866 changed the output of the embedded directives when building with a distributed build system, where the compiler doesn't have access to inspect the lib directory that is going to be used to link things in the end. https://github.com/llvm/llvm-project/pull/89775 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > > I would suggest we revert this - and at least collect all the observed side > > effects and declare them before considering relanding it. > > That sounds good to me. Do you have a list of PRs to revert? Not sure if there are follow-up fixes, sorry, but the discussed PRs are this one, and #81037 (where #89775 says the latter one changed functional behaviour, but it sounds mostly like your issue, i.e. caused by this one). > > ... then we do still get the old name embedded. > > Sure. The motivation on our side is a distributed compile service where the > library doesn't exist on the remote end. This patch means we'll have to add > knowledge about path layouts at link time to the remote setup at compile > time. That's certainly doable, but kind of janky. Right, I see. FWIW, with the embedding of directives, which kind of depends on knowing the runtime layout style, I guess this is an issue that needs to be tackled at one point or another, for distributed builds (especially as long as the intent is to change default towards the per-triple directories instead of per-os directories). > (What we'll actually do is use the flag from #89642 to disable all this > guessing of libs and just explicitly pass the path to the lib at link time. > So this won't actually affect us then, but reverting and relanding in one > commit with a list of side effects still sounds like a good thing independent > of that.) Ah, that probably sounds like a good flag to have in any case, for that kind of distributed setup! https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Restore compiler-rt arch suffix for PS and Windows (PR #89775)
mstorsjo wrote: > Commit > [b876596](https://github.com/llvm/llvm-project/commit/b876596a76cdc183439b36455d26883b67f8ee51) > corrected default compiler-rt library names for many targets Are you sure it's this change? There are reports of similar changes showing up in https://github.com/llvm/llvm-project/pull/87866#issuecomment-2072684950, caused by ccdebbae4d77d3efc236af92c22941de5d437e01 (#87866) > It's been like that for maybe 2-3 years now and no one has complained about it The old status quo has been that you can build the runtimes with either layout, and clang will use whichever it finds, when invoking the linker. The recent changes, #81037 and #87866, were (as far as I know) only intended to change what is printed as error messages, when neither is found, to help users correct their setup for the new style layout. But those changes also seem to have unexpected effects in changing e.g. what gets emitted as embedded directive, when the compiler doesn't see either of them at compile time (with e.g. distributed build systems), while they might be available at link time. > but last time it was discussed I think @MaskRay was against a new variable, > but since we might need to have some different behaviour it might be > warrented. Not sure who might have been against it; my take on it is at least that the build time cmake variables are tricky, when e.g. one clang binary might be for multiple different targets (e.g. for native compilation on linux, with per-target runtime directories there, but also for cross compilation for windows targets with a different setup for the target runtimes). I'm not against them, as long as both setups remain usable though. https://github.com/llvm/llvm-project/pull/89775 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > I now did build clang at > [ccdebba](https://github.com/llvm/llvm-project/commit/ccdebbae4d77d3efc236af92c22941de5d437e01) > and > [ccdebba](https://github.com/llvm/llvm-project/commit/ccdebbae4d77d3efc236af92c22941de5d437e01)^. > > [ccdebba](https://github.com/llvm/llvm-project/commit/ccdebbae4d77d3efc236af92c22941de5d437e01) > has `/DEFAULTLIB:clang_rt.profile.lib` in its output, > [ccdebba](https://github.com/llvm/llvm-project/commit/ccdebbae4d77d3efc236af92c22941de5d437e01)^ > has /DEFAULTLIB:clang_rt.profile-x86_64.lib`. So definitely due to this > change. > > It sounds like you're saying that's not intentional? I didn't author this, but as far as I understood, the intent of this patch was only to make sure to print the new style path to ease disambiguation for the cases when both are missing - i.e. the same as #81037, for some cases that wasn't covered by the former. I never saw this PR as one that had intended functional effects. At this point, with more and more functional effects popping up, that aren't mentioned as intended within the commit message, I would suggest we revert this - and at least collect all the observed side effects and declare them before considering relanding it. > Will the contents of `empty.asm` correct if > `lib//clang_rt.profile.lib` doesn't exist? I mean, will `empty.asm` > contains `/DEFAULTLIB:clang_rt.profile-x86_64.lib` then? This is in a case where this file does not exist, and neither does the new one. @nico wrote: > `foo` is a directly that contains just these two clang binaries I.e. it is a directory that contains these two binaries and nothing else. I do note that if `lib/clang/19/lib/windows/clang_rt.profile-x86_64.lib` does exist, i.e. if we add a `mkdir -p foo/lib/clang/19/lib/windows && touch foo/lib/clang/19/lib/windows/clang_rt.profile-x86_64.lib`, then we do still get the old name embedded. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > This is a behavior change: In distributed build environments, neither lib > file exists at compile time. Previously, this would result in the "old" > style, now (together with #81037) it results in the "new" style (which we > disable everywhere since it causes all kinds of issues – from what I can > tell, we're not alone in this). Hmm, in which cases does this PR change anything of what happens at compile time? The only functional behaviour difference I'm aware of, is that `clang --print-runtime-dir` now always prints the new style path, even if the old style path exists and was expected to be used. There have been talks about embedding directives for autolinking compiler-rt builtins for msvc mode builds, and switching between old and new path style depending on what files exist on disk (which would be a problem for the kind of distributed build environment you have, admittedly!), but that's not implemented yet. Or is this already the case for embedded directives for asan? And based on your linked issue, apparently also for profiling? Can you distill out a minimal standalone command that showcases compiling, and shows the embedded directive that gets changed by this PR? https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > > Is it expected now that `clang --print-runtime-dir` will always have the > > clang host triple appended even if `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is > > off? I guess I was expecting to see `lib/linux` instead of > > `lib/x86_64-unknown-linux-gnu`. > > https://reviews.llvm.org/D98868 introduced `--print-runtime-dir`. The > question is whether `--print-runtime-dir` should print the legacy `lib/linux` > when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is off. Personally I'd hope that > `--print-runtime-dir` does not try to be smart and users should be able to > expect that it always prints the new hierarchy. > > With the old hierarchy, the user is expected to know how to derive > `libclang_rt.builtins-aarch64.a` from `libclang_rt.builtins.a`. In this case, > they can extract the directory name from `clang > --print-file-name=libclang_rt.builtins-aarch64.a`. That change doesn’t seem to explicitly say that this only is intended to be used with the new layout, and it seems like a number of different users have taken up use of this option in this way, with the old layout. So this change does seem to affect a number of previously seemingly fully legitimate configurations in practice. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > @aeubanks The problem is that in your configure, the libclang_rt is placed in > `/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a`, > instead of > `/lib/clang/19/lib/arm-unknown-linux-android/libclang_rt.builtins.a`. The point is that both locations were supposed to be accepted, as they were before - this PR was not supposed to be a policy change that affects that. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > This seems to have had an unexpected effect. In a build where I don't use the > new path style, I used to get the old path style returned like this: > > ``` > $ clang -target x86_64-w64-mingw32 -print-runtime-dir > /home/martin/clang-nightly/lib/clang/19/lib/windows > ``` > > However after this change, now I'm getting the new style path, even if it > doesn't exist, and if the old one actually did exist: > > ``` > $ clang -target x86_64-w64-mingw32 -print-runtime-dir > /home/martin/clang-nignhtly/lib/clang/19/lib/x86_64-w64-windows-gnu > ``` > > I'm ok with changing the default if the old path style doesn't exist - but if > it does exist, we should still return that. (I haven't dig into it to see why > this is, yet.) The reason for this seems to lie here: https://github.com/llvm/llvm-project/blob/ccdebbae4d77d3efc236af92c22941de5d437e01/clang/lib/Driver/Driver.cpp#L2213-L2219 Previously, this picked the `TC.getCompilerRTPath()` case, while this now always ends up going with `TC.getRuntimePath()` as it's never empty. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: This seems to have had an unexpected effect. In a build where I don't use the new path style, I used to get the old path style returned like this: ``` $ clang -target x86_64-w64-mingw32 -print-runtime-dir /home/martin/clang-nightly/lib/clang/19/lib/windows ``` However after this change, now I'm getting the new style path, even if it doesn't exist, and if the old one actually did exist: ``` $ clang -target x86_64-w64-mingw32 -print-runtime-dir /home/martin/clang-nignhtly/lib/clang/19/lib/x86_64-w64-windows-gnu ``` I'm ok with changing the default if the old path style doesn't exist - but if it does exist, we should still return that. (I haven't dig into it to see why this is, yet.) https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Compile the asm as well as the C++ source (PR #86351)
mstorsjo wrote: > I'm sorry to hear that. Reading through > https://libcxx.llvm.org/BuildingLibcxx.html now to see if I can make > ENABLE_RUNTIMES behave itself under cross compilation. I'm familiar with this > in the context of the "bootstrapping" build from the docs, the build clang > first one, which drops (most) arguments passed to cmake. Hopefully building > runtimes directly doesn't involve that quirk. Indeed, pointing cmake at `llvm-project/runtimes` and specifying libunwind in ENABLE_RUNTIMES should work pretty much the same as pointing cmake at libunwind directly, just with a bunch of boilerplate centralized. I agree that it probably would be good to keep the error message explaining this, especially if it otherwise seems to almost work. https://github.com/llvm/llvm-project/pull/86351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Compile the asm as well as the C++ source (PR #86351)
mstorsjo wrote: This build configuration was explicitly made disallowed in 6f17768e11480063f4c2bcbeea559505fee3ea19, with an error message explaining the situation. However that error message was later removed in 0a22dfcb11c05cbd4f654c8ef1868a4bc6085140. https://github.com/llvm/llvm-project/pull/86351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] [libcxxabi] [libunwind] [libcxx, libcxxabi, libunwind] Prefer -fvisibility-global-new-delete=force-hidden (PR #84917)
https://github.com/mstorsjo created
https://github.com/llvm/llvm-project/pull/84917
27ce26b06655cfece3d54b30e442ef93d3e78ac7 added the new option
`-fvisibility-global-new-delete=`, where
`-fvisibility-global-new-delete=force-hidden` is equivalent to the old option
`-fvisibility-global-new-delete-hidden`. At the same time, the old option was
deprecated.
Test for and use the new option form first; if unsupported, try using the old
form.
This avoids warnings in the MinGW builds, if built with Clang 18 or newer.
From 4145880f327ed3ad76f7693193ba31ec2d2332ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Tue, 12 Mar 2024 10:42:16 +0200
Subject: [PATCH] [libcxx, libcxxabi, libunwind] Prefer
-fvisibility-global-new-delete=force-hidden
27ce26b06655cfece3d54b30e442ef93d3e78ac7 added the new option
`-fvisibility-global-new-delete=`, where
`-fvisibility-global-new-delete=force-hidden` is equivalent to the
old option `-fvisibility-global-new-delete-hidden`. At the same
time, the old option was deprecated.
Test for and use the new option form first; if unsupported, try
using the old form.
This avoids warnings in the MinGW builds, if built with Clang 18
or newer.
---
libcxx/src/CMakeLists.txt| 5 -
libcxxabi/src/CMakeLists.txt | 5 -
libunwind/src/CMakeLists.txt | 5 -
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 07ffc8bfdaae3d..1110a79ddcacd5 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -301,7 +301,10 @@ if (LIBCXX_ENABLE_STATIC)
# then its code shouldn't declare them with hidden visibility. They might
# actually be provided by a shared library at link time.
if (LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
- append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS
-fvisibility-global-new-delete-hidden)
+ append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS
-fvisibility-global-new-delete=force-hidden)
+ if (NOT CXX_SUPPORTS_FVISIBILITY_GLOBAL_NEW_DELETE_EQ_FORCE_HIDDEN_FLAG)
+append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS
-fvisibility-global-new-delete-hidden)
+ endif()
endif()
target_compile_options(cxx_static PRIVATE ${CXX_STATIC_LIBRARY_FLAGS})
# _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS can be defined in __config_site
diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
index 0af4dc1448e91a..c8cc93de50777b 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -268,7 +268,10 @@ if(LIBCXXABI_HERMETIC_STATIC_LIBRARY)
# then its code shouldn't declare them with hidden visibility. They might
# actually be provided by a shared library at link time.
if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
-target_add_compile_flags_if_supported(cxxabi_static_objects PRIVATE
-fvisibility-global-new-delete-hidden)
+target_add_compile_flags_if_supported(cxxabi_static_objects PRIVATE
-fvisibility-global-new-delete=force-hidden)
+if (NOT CXX_SUPPORTS_FVISIBILITY_GLOBAL_NEW_DELETE_EQ_FORCE_HIDDEN_FLAG)
+ target_add_compile_flags_if_supported(cxxabi_static_objects PRIVATE
-fvisibility-global-new-delete-hidden)
+endif()
endif()
# _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS can be defined in libcxx's
# __config_site too. Define it in the same way here, to avoid redefinition
diff --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index 9c6f5d908b0945..780430ba70ba60 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -201,7 +201,10 @@ set_target_properties(unwind_static_objects
if(LIBUNWIND_HIDE_SYMBOLS)
target_add_compile_flags_if_supported(unwind_static_objects PRIVATE
-fvisibility=hidden)
- target_add_compile_flags_if_supported(unwind_static_objects PRIVATE
-fvisibility-global-new-delete-hidden)
+ target_add_compile_flags_if_supported(unwind_static_objects PRIVATE
-fvisibility-global-new-delete=force-hidden)
+ if (NOT CXX_SUPPORTS_FVISIBILITY_GLOBAL_NEW_DELETE_EQ_FORCE_HIDDEN_FLAG)
+target_add_compile_flags_if_supported(unwind_static_objects PRIVATE
-fvisibility-global-new-delete-hidden)
+ endif()
target_compile_definitions(unwind_static_objects PRIVATE
_LIBUNWIND_HIDE_SYMBOLS)
endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Update documentation and release notes for llvm-profgen COFF support (PR #84864)
@@ -2410,20 +2410,35 @@ usual build cycle when using sample profilers for optimization: 1. Build the code with source line table information. You can use all the usual build flags that you always build your application with. The only - requirement is that you add ``-gline-tables-only`` or ``-g`` to the - command line. This is important for the profiler to be able to map - instructions back to source line locations. + requirement is that DWARF debug info including source line information is + generated. This DWARF information is important for the profiler to be able + to map instructions back to source line locations. + + On Linux, ``-g`` or just ``-gline-tables-only`` is sufficient: .. code-block:: console $ clang++ -O2 -gline-tables-only code.cc -o code + It is also possible to include DWARF in Windows binaries: mstorsjo wrote: Codeview is the standard in MSVC style environments, while DWARF is the standard in MinGW too. https://github.com/llvm/llvm-project/pull/84864 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [TargetParser][AArch64] Add alias for FEAT_RDM. (PR #80540)
mstorsjo wrote: FYI, see 4b8d9abca7d0280878fb12de331e688ee85d7cd8 for another existing case where we already support both `rdm` and `rdma`. But I don't think that case can share any of the aliasing logic from here. https://github.com/llvm/llvm-project/pull/80540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
https://github.com/mstorsjo approved this pull request. If nobody else wants to chime in here, I guess I'll go ahead and approve it. LGTM! https://github.com/llvm/llvm-project/pull/80527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Improve error when a compiler-rt library is not found (PR #81037)
https://github.com/mstorsjo approved this pull request. LGTM, thanks. In principle, we're just trading surprises in one case into surprises in another case, but I guess this case is the one with more non-obvious details (and this can be argued is the future direction we should be heading towards, even if I'm not personally a huge fan of it), so it seems reasonable and valuable. https://github.com/llvm/llvm-project/pull/81037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [asan][windows] Eliminate the static asan runtime on windows (PR #81677)
mstorsjo wrote: > > Not to distract from the direction taken here (which I do agree seems > > reasonable, even if I haven't had time to look closer at the PR yet) - but, > > when using the static CRT in general, doesn't the same concern apply there > > as well? I.e. when using the static CRT, care must be taken that the same > > CRT instances does frees as did the allocation. But I guess there are other > > asan-related aspects that makes it even more of a mess. > > Yes, it does, but my understanding is that this requirement is slightly > weaker than what asan needs. As long as you _do_ match mallocs and frees > things work fine, because the data that's shared is shared through > kernel32.dll, ntdll.dll, or the kernel and page table. If asan allowed this > it would need some way of mapping memory to the correct global structures, > and some way of figuring out who gets to handle the shadow memory area and > where to get the stack information for a given allocation upon an invalid > access. We can't rely on "matching" allocations with invalid accesses because > it's far from uncommon for a memory error to relate to memory allocated in > another DLL. Maybe the static-linked asans could have some voting mechanism > to settle on a "lead" copy of asan or something. It's not impossible but > making things work well is a ton of work for something the loader can do > anyway. Before this change we still relied on only one copy of asan being > around, but the functions would be exported from the main exe with the "dll > thunk" using GetProcAddress to call them. Ah, right, these are all very good explanations for why this is harder than the preexisting cases of multiple statically linked CRTs within the same process. Thanks! > > Also secondly, when discussing these details - how the asan runtime is > > built in one, specific way, while it is used for applications that can use > > a different CRT configuration > > It might be possible to support copies of the asan runtime built with the > static CRT, it's just not that useful and we'd rather keep one configuration > and spend time making it work perfectly for all instrumented programs. Yeah, I totally agree, there's no practical need for allowing asan to be linked in another way. Especially as it works for both MSVC and mingw environments. > It occurs to me that this probably requires changes to the gyp build files. No, the extra build systems doesn't need to be updated in general, that's up to whoever maintains them to sync them afterwards. Regular contributors only need to make sure the cmake build works. (And unless one is able to test that the changes to the third party build systems actually is entirely right, it's usually better to leave them untouched, so it can be done cleanly in one commit, rather than do a half-broken update to the files.) https://github.com/llvm/llvm-project/pull/81677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [COFF][Aarch64] Add _InterlockedAdd64 intrinsic (PR #81849)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/81849 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [COFF][Aarch64] Add _InterlockedAdd64 intrinsic (PR #81849)
mstorsjo wrote: > Done Thanks, now this looks good to merge! https://github.com/llvm/llvm-project/pull/81849 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [COFF][Aarch64] Add _InterlockedAdd64 intrinsic (PR #81849)
mstorsjo wrote: It looks like your github account is set to keep your email address private - can you please turn that off, so we get a proper email address (when the commit is rewritten, as we do merges with "squash and merge" here)? See the "keep my email addresses private" setting at https://github.com/settings/emails, and https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it for the reasoning about this. https://github.com/llvm/llvm-project/pull/81849 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [asan][windows] Eliminate the static asan runtime on windows (PR #81677)
mstorsjo wrote: > The core reasoning is that asan is a "only one allowed per process" type > thing (you can't have one copy of the asan runtime handling a malloc while a > different one handles the corresponding free). Not to distract from the direction taken here (which I do agree seems reasonable, even if I haven't had time to look closer at the PR yet) - but, when using the static CRT in general, doesn't the same concern apply there as well? I.e. when using the static CRT, care must be taken that the same CRT instances does frees as did the allocation. But I guess there are other asan-related aspects that makes it even more of a mess. Also secondly, when discussing these details - how the asan runtime is built in one, specific way, while it is used for applications that can use a different CRT configuration - would you like to chime in on https://github.com/microsoft/vcpkg/pull/34123#issuecomment-1760396486? There, some people involved questioned and weren't convinced that one part of a project (compiler-rt in llvm) should be allowed to override the CRT choice that is made for the whole vcpkg build. I tried to argue that compiler-rt is a very special case and doesn't interact with the rest of the libraries built in vcpkg like that, and compiler-rt is free to override the CRT to use internally. https://github.com/llvm/llvm-project/pull/81677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix build dllexport/dllimport issues when doing a shared build for Windows using GCC (PR #66881)
mstorsjo wrote: > This is superseded by https://github.com/llvm/llvm-project/pull/71393 which > was merged now. I'll close this one for now, as I believe the issue has been fixed differently. https://github.com/llvm/llvm-project/pull/66881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix build dllexport/dllimport issues when doing a shared build for Windows using GCC (PR #66881)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/66881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
mstorsjo wrote: From my point of view, I think this sounds fine - it looks like you've looked through this and thought deeper about it than I have at least. I'm not deep enough into this to comfortably press approved on this for now though, so hopefully someone else can chime in as well. https://github.com/llvm/llvm-project/pull/80527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Driver] Improve error when a compiler-rt library is not found (PR #81037)
mstorsjo wrote: I would, generally, prefer to not hardcode `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` (which only affects how runtimes are installed) into Clang. Runtimes may or may not be built at the same time as Clang, and one build of Clang can be used for a multitude of targets with different setups. I wrote a more lengthy response on discourse, see https://discourse.llvm.org/t/runtime-directory-fallback/76860/7. https://github.com/llvm/llvm-project/pull/81037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Handle linking ARM64EC code (PR #78912)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/78912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Handle linking ARM64EC code (PR #78912)
https://github.com/mstorsjo approved this pull request. LGTM, thanks for adding the test! https://github.com/llvm/llvm-project/pull/78912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Handle linking ARM64EC code (PR #78912)
https://github.com/mstorsjo commented: Code wise, this seems good, but I think we'd like to have a testcase for it. https://github.com/llvm/llvm-project/pull/78912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
mstorsjo wrote: > > I don't really know more about the issue that requires --long-plt at the > > moment and why it's only needed for clang-repl > > clang-repl binary size is ~3.7G in debug mode and this seems to exceed the > branch range of default ARM PLT slots. The instruction sequence that's > necessary for long-range PLT slots is likely more expensive. I guess this > situation is too much of an edge-case to make the effort and detect it > upfront, so they just bail out if it happens an ask for the flag. Right, I see. Though - why is this only an issue for the clang-repl binary and not the other clang-based ones? Does it happen to hit some maximum when it uses both everything that is in normal clang, plus all the JIT stuff? (Normally I would expect LLDB to be the largest one, as it includes much of Clang, but that's also always split into a separate liblldb.so.) https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
mstorsjo wrote: > Oh, I usually don't do that, but it's certainly a valid point. Can you think > of a better way to express the condition here? We need `-Wl,--long-plt` for > ARM targets whenever the used linker supports it. Otherwise we have to assume > that it emits such PLTs by default. Not sure; architecture detection in CMake generally is problematic. It's possible to infer the actual real destination architecture with a compiler test (like compiling and testing if `__arm__` is defined). CMake doesn't really provide anything out of the box for that though (it does provide `CMAKE_C_COMPILER_ARCHITECTURE_ID` on MSVC like compilers, but not elsewhere, see https://gitlab.kitware.com/cmake/cmake/-/issues/17702), and it feels like overkill to add such a compile test here. Also note that `CMAKE_SYSTEM_PROCESSOR` is unclear in Darwin universal builds anyway - if I'm compiling with `-DCMAKE_OSX_ARCHITECTURES=x86_64;arm64` what will be set in `CMAKE_SYSTEM_PROCESSOR`? :-) I think this current form of the check might be ok enough (I don't really know more about the issue that requires `--long-plt` at the moment and why it's only needed for clang-repl). If you'd want it to hit more universally, perhaps just remove the architecture check - if we test that the linker does support `--long-plt` without erroring out, we probably can add it, even if we don't really know the exact architecture we're linking for? https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
mstorsjo wrote: > > When cross compiling LLVM, I never have set `CMAKE_SYSTEM_PROCESSOR` so > > far, since we don't really have anything that uses it (before this), which > > means that this expands to an empty string. I guess I should set it still > > though. > > Yes, I am just getting used to it as well. I think it's worth noting that > this should be set in a toolchain file, because CMake seems to have special > handling for them. When I pass `-DCMAKE_SYSTEM_PROCESSOR=ARM` at > configuration time, it gets overwritten with my host arch. I think that's (somewhat?) expected. When you're not cross compiling, cmake explicitly sets `CMAKE_SYSTEM_PROCESSOR` to `CMAKE_HOST_SYSTEM_PROCESSOR`, ignoring whatever you set manually. (Dunno if it would make any difference if you'd set it in a toolchain file.) Only if you're cross compiling (which CMake defines as if you're setting `CMAKE_SYSTEM_NAME`, regardless if cross compiling from Linux to Linux on another arch), it doesn't set it and passes whatever you set originally through. This actually makes `CMAKE_SYSTEM_PROCESSOR` a bit problematic; if you're on x86_64 but targeting i386, or on aarch64 but targeting arm, you might not want to consider it a cross build (as you can execute the build products) but CMake then will hide whatever you're really targeting with the info gathered from the host environment. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] e3d73ad - [clang-repl] Fix CMake errors when cross compiling
Author: Martin Storsjö
Date: 2024-01-23T13:42:24+02:00
New Revision: e3d73ad58c41b945d9fc5d5fb16ea44850ccf652
URL:
https://github.com/llvm/llvm-project/commit/e3d73ad58c41b945d9fc5d5fb16ea44850ccf652
DIFF:
https://github.com/llvm/llvm-project/commit/e3d73ad58c41b945d9fc5d5fb16ea44850ccf652.diff
LOG: [clang-repl] Fix CMake errors when cross compiling
These stem from 4821c90c24d52d4a42990fd9371caedb157bc58b.
When cross compiling, CMAKE_SYSTEM_PROCESSOR is empty, if the
target processor hasn't been set when setting up the cross
compilation. Ideally, when setting up cross compilation with
CMake, the user is supposed to set CMAKE_SYSTEM_PROCESSOR, but
so far, compilation has worked just fine even without it.
Quote the string where CMAKE_SYSTEM_PROCESSOR is expanded, to
avoid argument errors when it expands to an empty string.
Added:
Modified:
clang/tools/clang-repl/CMakeLists.txt
Removed:
diff --git a/clang/tools/clang-repl/CMakeLists.txt
b/clang/tools/clang-repl/CMakeLists.txt
index 2a0f617a2c0ff6..031dcaba5e4468 100644
--- a/clang/tools/clang-repl/CMakeLists.txt
+++ b/clang/tools/clang-repl/CMakeLists.txt
@@ -23,7 +23,7 @@ if(CLANG_PLUGIN_SUPPORT)
export_executable_symbols_for_plugins(clang-repl)
endif()
-string(TOUPPER ${CMAKE_SYSTEM_PROCESSOR} system_processor)
+string(TOUPPER "${CMAKE_SYSTEM_PROCESSOR}" system_processor)
if(system_processor MATCHES "ARM")
set(FLAG_LONG_PLT "-Wl,--long-plt")
llvm_check_linker_flag(CXX ${FLAG_LONG_PLT} LINKER_HAS_FLAG_LONG_PLT)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][AArch64] Define __USER_LABEL_PREFIX__ to # for ARM64EC (PR #78913)
@@ -1462,10 +1462,12 @@ WindowsARM64TargetInfo::WindowsARM64TargetInfo(const
llvm::Triple ,
}
void WindowsARM64TargetInfo::setDataLayout() {
- resetDataLayout(Triple.isOSBinFormatMachO()
- ? "e-m:o-i64:64-i128:128-n32:64-S128"
- : "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128",
- Triple.isOSBinFormatMachO() ? "_" : "");
+ if (Triple.isOSBinFormatMachO()) {
+resetDataLayout("e-m:o-i64:64-i128:128-n32:64-S128", "_");
mstorsjo wrote:
AFAIK Windows+MachO is used in some cases for JIT scenarios, because the
runtime linking support in the JIT is much more complete for MachO than it is
for PE/COFF.
https://github.com/llvm/llvm-project/pull/78913
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [Driver] Treat MuslEABIHF as a hardfloat environment wrt multiarch directories (PR #77536)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/77536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [Driver] Treat MuslEABIHF as a hardfloat environment wrt multiarch directories (PR #77536)
mstorsjo wrote: > `bool isEABIHF` from clang/lib/CodeGen/Targets/ARM.cpp can probably be > factored. Yep - any suggestion on where we could move it? Up to the `Triple` class? https://github.com/llvm/llvm-project/pull/77536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Convert a few options from CACHE PATH to CACHE STRING (PR #77534)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/77534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [Driver] Treat MuslEABIHF as a hardfloat environment wrt multiarch directories (PR #77536)
https://github.com/mstorsjo created
https://github.com/llvm/llvm-project/pull/77536
If using multiarch directories with musl, the multiarch directory still uses
*-linux-gnu triples - which may or may not be intentional, while it is somewhat
consistent at least.
However, for musl armhf targets, make sure that this also picks
arm-linux-gnueabihf, rather than arm-linux-gnueabi.
From f715cfc716e5a4bacdc0ef041f6a53c6821e81cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Mon, 8 Jan 2024 12:35:36 +0200
Subject: [PATCH] [clang] [Driver] Treat MuslEABIHF as a hardfloat environment
wrt multiarch directories
If using multiarch directories with musl, the multiarch directory
still uses *-linux-gnu triples - which may or may not be intentional,
while it is somewhat consistent at least.
However, for musl armhf targets, make sure that this also picks
arm-linux-gnueabihf, rather than arm-linux-gnueabi.
---
clang/lib/Driver/ToolChains/Gnu.cpp | 8 ++--
clang/lib/Driver/ToolChains/Linux.cpp | 8 ++--
clang/test/Driver/linux-ld.c | 9 +
3 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 24681dfdc99c03..771240dac7a83e 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2668,7 +2668,9 @@ void
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
case llvm::Triple::arm:
case llvm::Triple::thumb:
LibDirs.append(begin(ARMLibDirs), end(ARMLibDirs));
-if (TargetTriple.getEnvironment() == llvm::Triple::GNUEABIHF) {
+if (TargetTriple.getEnvironment() == llvm::Triple::GNUEABIHF ||
+TargetTriple.getEnvironment() == llvm::Triple::MuslEABIHF ||
+TargetTriple.getEnvironment() == llvm::Triple::EABIHF) {
TripleAliases.append(begin(ARMHFTriples), end(ARMHFTriples));
} else {
TripleAliases.append(begin(ARMTriples), end(ARMTriples));
@@ -2677,7 +2679,9 @@ void
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
case llvm::Triple::armeb:
case llvm::Triple::thumbeb:
LibDirs.append(begin(ARMebLibDirs), end(ARMebLibDirs));
-if (TargetTriple.getEnvironment() == llvm::Triple::GNUEABIHF) {
+if (TargetTriple.getEnvironment() == llvm::Triple::GNUEABIHF ||
+TargetTriple.getEnvironment() == llvm::Triple::MuslEABIHF ||
+TargetTriple.getEnvironment() == llvm::Triple::EABIHF) {
TripleAliases.append(begin(ARMebHFTriples), end(ARMebHFTriples));
} else {
TripleAliases.append(begin(ARMebTriples), end(ARMebTriples));
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp
b/clang/lib/Driver/ToolChains/Linux.cpp
index 735af54f114cef..4300a2bdff1791 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -61,12 +61,16 @@ std::string Linux::getMultiarchTriple(const Driver ,
case llvm::Triple::thumb:
if (IsAndroid)
return "arm-linux-androideabi";
-if (TargetEnvironment == llvm::Triple::GNUEABIHF)
+if (TargetEnvironment == llvm::Triple::GNUEABIHF ||
+TargetEnvironment == llvm::Triple::MuslEABIHF ||
+TargetEnvironment == llvm::Triple::EABIHF)
return "arm-linux-gnueabihf";
return "arm-linux-gnueabi";
case llvm::Triple::armeb:
case llvm::Triple::thumbeb:
-if (TargetEnvironment == llvm::Triple::GNUEABIHF)
+if (TargetEnvironment == llvm::Triple::GNUEABIHF ||
+TargetEnvironment == llvm::Triple::MuslEABIHF ||
+TargetEnvironment == llvm::Triple::EABIHF)
return "armeb-linux-gnueabihf";
return "armeb-linux-gnueabi";
case llvm::Triple::x86:
diff --git a/clang/test/Driver/linux-ld.c b/clang/test/Driver/linux-ld.c
index 15643d6491ae52..d5cc3103a3a746 100644
--- a/clang/test/Driver/linux-ld.c
+++ b/clang/test/Driver/linux-ld.c
@@ -541,6 +541,15 @@
// RUN: --gcc-toolchain="" \
// RUN: --sysroot=%S/Inputs/ubuntu_12.04_LTS_multiarch_tree \
// RUN: | FileCheck --check-prefix=CHECK-UBUNTU-12-04-ARM-HF %s
+//
+// Check that musleabihf is treated as a hardfloat config, with respect to
+// multiarch directories.
+//
+// RUN: %clang -### %s -no-pie 2>&1 \
+// RUN: --target=arm-unknown-linux-musleabihf -rtlib=platform
--unwindlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/ubuntu_12.04_LTS_multiarch_tree \
+// RUN: | FileCheck --check-prefix=CHECK-UBUNTU-12-04-ARM-HF %s
// CHECK-UBUNTU-12-04-ARM-HF: "{{.*}}ld{{(.exe)?}}"
"--sysroot=[[SYSROOT:[^"]+]]"
// CHECK-UBUNTU-12-04-ARM-HF:
"{{.*}}/usr/lib/arm-linux-gnueabihf{{/|}}crt1.o"
// CHECK-UBUNTU-12-04-ARM-HF:
"{{.*}}/usr/lib/arm-linux-gnueabihf{{/|}}crti.o"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Convert a few options from CACHE PATH to CACHE STRING (PR #77534)
https://github.com/mstorsjo created
https://github.com/llvm/llvm-project/pull/77534
This applies the same change as in
760261a3daf98882ccbd177e3133fb4a058f47ad (where they were applied to libcxxabi
and libcxx) to libunwind as well.
These options can reasonably be set either as an absolute or relative path, but
if set as type PATH, they are rewritten from relative into absolute relative to
the build directory, while the relative form is intended to be relative to the
install prefix.
From f4d654ff157f7a1ae3237d22dbbc541e6c083c4f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Mon, 8 Jan 2024 14:32:47 +0200
Subject: [PATCH] [libunwind] Convert a few options from CACHE PATH to CACHE
STRING
This applies the same change as in
760261a3daf98882ccbd177e3133fb4a058f47ad (where they were applied
to libcxxabi and libcxx) to libunwind as well.
These options can reasonably be set either as an absolute or
relative path, but if set as type PATH, they are rewritten from
relative into absolute relative to the build directory, while
the relative form is intended to be relative to the install prefix.
---
libunwind/CMakeLists.txt | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index 248e888619e40b..bb1b052f61d875 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -105,9 +105,9 @@ set(CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake"
${CMAKE_MODULE_PATH})
-set(LIBUNWIND_INSTALL_INCLUDE_DIR "${CMAKE_INSTALL_INCLUDEDIR}" CACHE PATH
+set(LIBUNWIND_INSTALL_INCLUDE_DIR "${CMAKE_INSTALL_INCLUDEDIR}" CACHE STRING
"Path where built libunwind headers should be installed.")
-set(LIBUNWIND_INSTALL_RUNTIME_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+set(LIBUNWIND_INSTALL_RUNTIME_DIR "${CMAKE_INSTALL_BINDIR}" CACHE STRING
"Path where built libunwind runtime libraries should be installed.")
set(LIBUNWIND_SHARED_OUTPUT_NAME "unwind" CACHE STRING "Output name for the
shared libunwind runtime library.")
@@ -115,7 +115,7 @@ set(LIBUNWIND_STATIC_OUTPUT_NAME "unwind" CACHE STRING
"Output name for the stat
if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
set(LIBUNWIND_LIBRARY_DIR
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
- set(LIBUNWIND_INSTALL_LIBRARY_DIR
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE} CACHE PATH
+ set(LIBUNWIND_INSTALL_LIBRARY_DIR
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE} CACHE STRING
"Path where built libunwind libraries should be installed.")
if(LIBCXX_LIBDIR_SUBDIR)
string(APPEND LIBUNWIND_LIBRARY_DIR /${LIBUNWIND_LIBDIR_SUBDIR})
@@ -127,7 +127,7 @@ else()
else()
set(LIBUNWIND_LIBRARY_DIR
${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX})
endif()
- set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX} CACHE PATH
+ set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX} CACHE STRING
"Path where built libunwind libraries should be installed.")
endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Don't look for a GCC in path if the install base has a proper mingw sysroot (PR #76949)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/76949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Don't look for a GCC in path if the install base has a proper mingw sysroot (PR #76949)
mstorsjo wrote: > > Although, on a second thought, it might actually still be good to adjust it > > in sync. If we're invoking Clang with `-m32` and deciding on whether to use > > i386/i586/i686, and we end up using the install base as sysroot, without > > inferring any triple from there, we shouldn't go on and check another > > unrelated GCC in path in order to influence this. Therefore, I think we > > perhaps should amend this with the following: > > Yes, I think that it would be better to avoid any decisions based on an > unrelated GCC and the additional check looks good to me. Ok, updated the PR with the patch that way; will merge it sometime later if nobody has any further objections to this. https://github.com/llvm/llvm-project/pull/76949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Don't look for a GCC in path if the install base has a proper mingw sysroot (PR #76949)
https://github.com/mstorsjo updated
https://github.com/llvm/llvm-project/pull/76949
From ce2a49c1a052b30fb1df91f3a7293e89e0a8726d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Tue, 19 Dec 2023 15:53:21 +0200
Subject: [PATCH] [clang] [MinGW] Don't look for a GCC in path if the install
base has a proper mingw sysroot
This fixes uses of the MSYS2 clang64 environment compilers, if
another set of GCC based compilers are available further back
in PATH (which may be explicitly added, or inherited unintentionally
from other software installed).
(The issue in the clang64 environment can be worked around somewhat
by installing *-gcc-compat packages which present aliases named
-gcc within the clang64 environment as well.)
This fixes https://github.com/msys2/MINGW-packages/issues/11495
and https://github.com/msys2/MINGW-packages/issues/19279.
---
clang/lib/Driver/ToolChains/MinGW.cpp | 25 ++--
clang/test/Driver/mingw-sysroot.cpp | 28 +++
2 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp
b/clang/lib/Driver/ToolChains/MinGW.cpp
index 65512f16357d04..18fc9d4b6807e3 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -471,12 +471,23 @@ findClangRelativeSysroot(const Driver , const
llvm::Triple ,
return make_error_code(std::errc::no_such_file_or_directory);
}
+static bool looksLikeMinGWSysroot(const std::string ) {
+ StringRef Sep = llvm::sys::path::get_separator();
+ if (!llvm::sys::fs::exists(Directory + Sep + "include" + Sep + "_mingw.h"))
+return false;
+ if (!llvm::sys::fs::exists(Directory + Sep + "lib" + Sep + "libkernel32.a"))
+return false;
+ return true;
+}
+
toolchains::MinGW::MinGW(const Driver , const llvm::Triple ,
const ArgList )
: ToolChain(D, Triple, Args), CudaInstallation(D, Triple, Args),
RocmInstallation(D, Triple, Args) {
getProgramPaths().push_back(getDriver().getInstalledDir());
+ std::string InstallBase =
+ std::string(llvm::sys::path::parent_path(getDriver().getInstalledDir()));
// The sequence for detecting a sysroot here should be kept in sync with
// the testTriple function below.
llvm::Triple LiteralTriple = getLiteralTriple(D, getTriple());
@@ -487,13 +498,17 @@ toolchains::MinGW::MinGW(const Driver , const
llvm::Triple ,
else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot(
getDriver(), LiteralTriple, getTriple(), SubdirName))
Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
+ // If the install base of Clang seems to have mingw sysroot files directly
+ // in the toplevel include and lib directories, use this as base instead of
+ // looking for a triple prefixed GCC in the path.
+ else if (looksLikeMinGWSysroot(InstallBase))
+Base = InstallBase;
else if (llvm::ErrorOr GPPName =
findGcc(LiteralTriple, getTriple()))
Base = std::string(llvm::sys::path::parent_path(
llvm::sys::path::parent_path(GPPName.get(;
else
-Base = std::string(
-llvm::sys::path::parent_path(getDriver().getInstalledDir()));
+Base = InstallBase;
Base += llvm::sys::path::get_separator();
findGccLibDir(LiteralTriple);
@@ -778,9 +793,15 @@ static bool testTriple(const Driver , const llvm::Triple
,
if (D.SysRoot.size())
return true;
llvm::Triple LiteralTriple = getLiteralTriple(D, Triple);
+ std::string InstallBase =
+ std::string(llvm::sys::path::parent_path(D.getInstalledDir()));
if (llvm::ErrorOr TargetSubdir =
findClangRelativeSysroot(D, LiteralTriple, Triple, SubdirName))
return true;
+ // If the install base itself looks like a mingw sysroot, we'll use that
+ // - don't use any potentially unrelated gcc to influence what triple to use.
+ if (looksLikeMinGWSysroot(InstallBase))
+return false;
if (llvm::ErrorOr GPPName = findGcc(LiteralTriple, Triple))
return true;
// If we neither found a colocated sysroot or a matching gcc executable,
diff --git a/clang/test/Driver/mingw-sysroot.cpp
b/clang/test/Driver/mingw-sysroot.cpp
index 911dab4927073d..50152b2ca210d2 100644
--- a/clang/test/Driver/mingw-sysroot.cpp
+++ b/clang/test/Driver/mingw-sysroot.cpp
@@ -14,6 +14,12 @@
// RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32
%T/testroot-clang/x86_64-w64-mingw32
// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32
%T/testroot-clang/i686-w64-mingw32
+// RUN: rm -rf %T/testroot-clang-native
+// RUN: mkdir -p %T/testroot-clang-native/bin
+// RUN: ln -s %clang %T/testroot-clang-native/bin/clang
+// RUN: mkdir -p %T/testroot-clang-native/include/_mingw.h
+// RUN: mkdir -p %T/testroot-clang-native/lib/libkernel32.a
+
// RUN: rm -rf %T/testroot-custom-triple
// RUN: mkdir -p %T/testroot-custom-triple/bin
// RUN: ln -s %clang %T/testroot-custom-triple/bin/clang
@@ -58,6
[clang] [clang] [MinGW] Don't look for a GCC in path if the install base has a proper mingw sysroot (PR #76949)
mstorsjo wrote: > > Looks mostly good to me, but I wonder if we should change testTriple as > > well. > > I thought so too based on the comment, but reviewing the code it seems > `testTriple` is trying to find evidence that a given triple (and more > specifically arch for things like `i386` vs `i686`) is valid. The evidence > found by `looksLikeMinGWSysroot` does not provide any hint about what the > triple or arch name should be, so I don't think it helps `testTriple`. Thanks, this explanation is absolutely spot on. (In all honesty, I had forgot about `testTriple` when I wrote this patch - and despite the comment right next to the code I touched, I didn't remember to check it...) Although, on a second thought, it might actually still be good to adjust it in sync. If we're invoking Clang with `-m32` and deciding on whether to use i386/i586/i686, and we end up using the install base as sysroot, without inferring any triple from there, we shouldn't go on and check another unrelated GCC in path in order to influence this. Therefore, I think we perhaps should amend this with the following: ```diff diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp b/clang/lib/Driver/ToolChains/MinGW.cpp index e2965a08a0b9..18fc9d4b6807 100644 --- a/clang/lib/Driver/ToolChains/MinGW.cpp +++ b/clang/lib/Driver/ToolChains/MinGW.cpp @@ -793,9 +793,15 @@ static bool testTriple(const Driver , const llvm::Triple , if (D.SysRoot.size()) return true; llvm::Triple LiteralTriple = getLiteralTriple(D, Triple); + std::string InstallBase = + std::string(llvm::sys::path::parent_path(D.getInstalledDir())); if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot(D, LiteralTriple, Triple, SubdirName)) return true; + // If the install base itself looks like a mingw sysroot, we'll use that + // - don't use any potentially unrelated gcc to influence what triple to use. + if (looksLikeMinGWSysroot(InstallBase)) +return false; if (llvm::ErrorOr GPPName = findGcc(LiteralTriple, Triple)) return true; // If we neither found a colocated sysroot or a matching gcc executable, ``` Actually, for such cases, we could potentially check for the per-target runtime installs, e.g. looking for `/include/` or `/lib/`. Switching between multiple arches with e.g. `-m32` in such a setup could work, if all the arch specific files are in `/lib/`, but I'm not sure if anyone does full mingw setups with such a hierarchy (yet). So this would be a separate potential future step. https://github.com/llvm/llvm-project/pull/76949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Don't look for a GCC in path if the install base has a proper mingw sysroot (PR #76949)
https://github.com/mstorsjo updated
https://github.com/llvm/llvm-project/pull/76949
From c67187043168b79e57c0e4f3261293d799852e90 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Tue, 19 Dec 2023 15:53:21 +0200
Subject: [PATCH] [clang] [MinGW] Don't look for a GCC in path if the install
base has a proper mingw sysroot
This fixes uses of the MSYS2 clang64 environment compilers, if
another set of GCC based compilers are available further back
in PATH (which may be explicitly added, or inherited unintentionally
from other software installed).
(The issue in the clang64 environment can be worked around somewhat
by installing *-gcc-compat packages which present aliases named
-gcc within the clang64 environment as well.)
This fixes https://github.com/msys2/MINGW-packages/issues/11495
and https://github.com/msys2/MINGW-packages/issues/19279.
---
clang/lib/Driver/ToolChains/MinGW.cpp | 19 --
clang/test/Driver/mingw-sysroot.cpp | 28 +++
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp
b/clang/lib/Driver/ToolChains/MinGW.cpp
index 65512f16357d04..e2965a08a0b9a2 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -471,12 +471,23 @@ findClangRelativeSysroot(const Driver , const
llvm::Triple ,
return make_error_code(std::errc::no_such_file_or_directory);
}
+static bool looksLikeMinGWSysroot(const std::string ) {
+ StringRef Sep = llvm::sys::path::get_separator();
+ if (!llvm::sys::fs::exists(Directory + Sep + "include" + Sep + "_mingw.h"))
+return false;
+ if (!llvm::sys::fs::exists(Directory + Sep + "lib" + Sep + "libkernel32.a"))
+return false;
+ return true;
+}
+
toolchains::MinGW::MinGW(const Driver , const llvm::Triple ,
const ArgList )
: ToolChain(D, Triple, Args), CudaInstallation(D, Triple, Args),
RocmInstallation(D, Triple, Args) {
getProgramPaths().push_back(getDriver().getInstalledDir());
+ std::string InstallBase =
+ std::string(llvm::sys::path::parent_path(getDriver().getInstalledDir()));
// The sequence for detecting a sysroot here should be kept in sync with
// the testTriple function below.
llvm::Triple LiteralTriple = getLiteralTriple(D, getTriple());
@@ -487,13 +498,17 @@ toolchains::MinGW::MinGW(const Driver , const
llvm::Triple ,
else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot(
getDriver(), LiteralTriple, getTriple(), SubdirName))
Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
+ // If the install base of Clang seems to have mingw sysroot files directly
+ // in the toplevel include and lib directories, use this as base instead of
+ // looking for a triple prefixed GCC in the path.
+ else if (looksLikeMinGWSysroot(InstallBase))
+Base = InstallBase;
else if (llvm::ErrorOr GPPName =
findGcc(LiteralTriple, getTriple()))
Base = std::string(llvm::sys::path::parent_path(
llvm::sys::path::parent_path(GPPName.get(;
else
-Base = std::string(
-llvm::sys::path::parent_path(getDriver().getInstalledDir()));
+Base = InstallBase;
Base += llvm::sys::path::get_separator();
findGccLibDir(LiteralTriple);
diff --git a/clang/test/Driver/mingw-sysroot.cpp
b/clang/test/Driver/mingw-sysroot.cpp
index 911dab4927073d..50152b2ca210d2 100644
--- a/clang/test/Driver/mingw-sysroot.cpp
+++ b/clang/test/Driver/mingw-sysroot.cpp
@@ -14,6 +14,12 @@
// RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32
%T/testroot-clang/x86_64-w64-mingw32
// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32
%T/testroot-clang/i686-w64-mingw32
+// RUN: rm -rf %T/testroot-clang-native
+// RUN: mkdir -p %T/testroot-clang-native/bin
+// RUN: ln -s %clang %T/testroot-clang-native/bin/clang
+// RUN: mkdir -p %T/testroot-clang-native/include/_mingw.h
+// RUN: mkdir -p %T/testroot-clang-native/lib/libkernel32.a
+
// RUN: rm -rf %T/testroot-custom-triple
// RUN: mkdir -p %T/testroot-custom-triple/bin
// RUN: ln -s %clang %T/testroot-custom-triple/bin/clang
@@ -58,6 +64,28 @@
// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%"
%T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32
-rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck
-check-prefix=CHECK_TESTROOT_GCC %s
+// If we're executing clang from a directory with what looks like a mingw
sysroot,
+// with headers in /include and libs in /lib, use that rather than
looking
+// for another GCC in the path.
+//
+// Note, this test has a surprising quirk: We're testing with an install
directory,
+// testroot-clang-native, which lacks the "x86_64-w64-mingw32" subdirectory,
it only
+// has the include and lib subdirectories without any triple prefix.
+//
+// Since commit fd15cb935d7aae25ad62bfe06fe9f17cea585978, we avoid using the
+// /include and /lib
[clang] [clang] [MinGW] Don't look for a GCC in path if the install base has a proper mingw sysroot (PR #76949)
mstorsjo wrote: CC @mati865 @jeremyd2019 @huangqinjin https://github.com/llvm/llvm-project/pull/76949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [MinGW] Don't look for a GCC in path if the install base has a proper mingw sysroot (PR #76949)
https://github.com/mstorsjo created
https://github.com/llvm/llvm-project/pull/76949
This fixes uses of the MSYS2 clang64 environment compilers, if another set of
GCC based compilers are available further back in PATH (which may be explicitly
added, or inherited unintentionally from other software installed).
(The issue in the clang64 environment can be worked around somewhat by
installing *-gcc-compat packages which present aliases named -gcc
within the clang64 environment as well.)
This fixes https://github.com/msys2/MINGW-packages/issues/11495 and
https://github.com/msys2/MINGW-packages/issues/19279.
From 355e2530e855249adf9657c58d4e1a6727d969bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?=
Date: Tue, 19 Dec 2023 15:53:21 +0200
Subject: [PATCH] [clang] [MinGW] Don't look for a GCC in path if the install
base has a proper mingw sysroot
This fixes uses of the MSYS2 clang64 environment compilers, if
another set of GCC based compilers are available further back
in PATH (which may be explicitly added, or inherited unintentionally
from other software installed).
(The issue in the clang64 environment can be worked around somewhat
by installing *-gcc-compat packages which present aliases named
-gcc within the clang64 environment as well.)
This fixes https://github.com/msys2/MINGW-packages/issues/11495
and https://github.com/msys2/MINGW-packages/issues/19279.
---
clang/lib/Driver/ToolChains/MinGW.cpp | 19 --
clang/test/Driver/mingw-sysroot.cpp | 28 +++
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp
b/clang/lib/Driver/ToolChains/MinGW.cpp
index 65512f16357d04..3868657659602e 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -471,12 +471,23 @@ findClangRelativeSysroot(const Driver , const
llvm::Triple ,
return make_error_code(std::errc::no_such_file_or_directory);
}
+static bool looksLikeMinGWSysroot(const std::string ) {
+ StringRef Sep = llvm::sys::path::get_separator();
+ if (!llvm::sys::fs::exists(Directory + Sep + "include" + Sep + "_mingw.h"))
+return false;
+ if (!llvm::sys::fs::exists(Directory + Sep + "lib" + Sep + "libkernel32.a"))
+return false;
+ return true;
+}
+
toolchains::MinGW::MinGW(const Driver , const llvm::Triple ,
const ArgList )
: ToolChain(D, Triple, Args), CudaInstallation(D, Triple, Args),
RocmInstallation(D, Triple, Args) {
getProgramPaths().push_back(getDriver().getInstalledDir());
+ std::string InstallBase = std::string(
+llvm::sys::path::parent_path(getDriver().getInstalledDir()));
// The sequence for detecting a sysroot here should be kept in sync with
// the testTriple function below.
llvm::Triple LiteralTriple = getLiteralTriple(D, getTriple());
@@ -487,13 +498,17 @@ toolchains::MinGW::MinGW(const Driver , const
llvm::Triple ,
else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot(
getDriver(), LiteralTriple, getTriple(), SubdirName))
Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
+ // If the install base of Clang seems to have mingw sysroot files directly
+ // in the toplevel include and lib directories, use this as base instead of
+ // looking for a triple prefixed GCC in the path.
+ else if (looksLikeMinGWSysroot(InstallBase))
+Base = InstallBase;
else if (llvm::ErrorOr GPPName =
findGcc(LiteralTriple, getTriple()))
Base = std::string(llvm::sys::path::parent_path(
llvm::sys::path::parent_path(GPPName.get(;
else
-Base = std::string(
-llvm::sys::path::parent_path(getDriver().getInstalledDir()));
+Base = InstallBase;
Base += llvm::sys::path::get_separator();
findGccLibDir(LiteralTriple);
diff --git a/clang/test/Driver/mingw-sysroot.cpp
b/clang/test/Driver/mingw-sysroot.cpp
index 911dab4927073d..50152b2ca210d2 100644
--- a/clang/test/Driver/mingw-sysroot.cpp
+++ b/clang/test/Driver/mingw-sysroot.cpp
@@ -14,6 +14,12 @@
// RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32
%T/testroot-clang/x86_64-w64-mingw32
// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32
%T/testroot-clang/i686-w64-mingw32
+// RUN: rm -rf %T/testroot-clang-native
+// RUN: mkdir -p %T/testroot-clang-native/bin
+// RUN: ln -s %clang %T/testroot-clang-native/bin/clang
+// RUN: mkdir -p %T/testroot-clang-native/include/_mingw.h
+// RUN: mkdir -p %T/testroot-clang-native/lib/libkernel32.a
+
// RUN: rm -rf %T/testroot-custom-triple
// RUN: mkdir -p %T/testroot-custom-triple/bin
// RUN: ln -s %clang %T/testroot-custom-triple/bin/clang
@@ -58,6 +64,28 @@
// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%"
%T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32
-rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck
-check-prefix=CHECK_TESTROOT_GCC %s
+// If we're
[clang] 71b3ead - [clang][dataflow] Remove a redundant trailing semicolon. NFC.
Author: Martin Storsjö
Date: 2024-01-04T15:01:17+02:00
New Revision: 71b3ead870107e39e998f6480e545eb01d9d28be
URL:
https://github.com/llvm/llvm-project/commit/71b3ead870107e39e998f6480e545eb01d9d28be
DIFF:
https://github.com/llvm/llvm-project/commit/71b3ead870107e39e998f6480e545eb01d9d28be.diff
LOG: [clang][dataflow] Remove a redundant trailing semicolon. NFC.
This silences the following warning with GCC:
llvm-project/llvm/tools/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:89:4:
warning: extra ‘;’ [-Wpedantic]
89 | };
|^
|-
Added:
Modified:
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
Removed:
diff --git
a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 8d481788af208a..fe5ba5ab5426f7 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -86,7 +86,7 @@ class DataflowAnalysisTest : public Test {
const std::optional = BlockStates[Block->getBlockID()];
assert(MaybeState.has_value());
return *MaybeState;
- };
+ }
std::unique_ptr AST;
std::unique_ptr CFCtx;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lld] [llvm] [libcxxabi] [compiler-rt] [libc] [openmp] [mlir] [clang-tools-extra] [clang] [lldb] [libcxx] [flang] [builtins][arm64] Build __init_cpu_features_resolver on Apple platforms (PR #73685)
mstorsjo wrote: > > BTW, when compiling the file I also get a bunch of warnings in this style: > > @mstorsjo maybe `unsigned long` is 32 bits on that platform... what's the > target triple? Ah, indeed - yes, Windows has 32 bit `long`s. The triples are `aarch64-windows-gnu` or `aarch64-windows-msvc`. https://github.com/llvm/llvm-project/pull/73685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang-tools-extra] [libc] [compiler-rt] [libcxx] [openmp] [mlir] [lldb] [flang] [libcxxabi] [lld] [builtins][arm64] Build __init_cpu_features_resolver on Apple platforms (PR #73685)
mstorsjo wrote:
This commit broken building compiler-rt builtins for Windows on aarch64;
building now hits these errors:
```
llvm-project/compiler-rt/lib/builtins/cpu_model.c:1192:2: error: No support for
checking for lse atomics on this platfrom yet.
1192 | #error No support for checking for lse atomics on this platfrom yet.
| ^
llvm-project/compiler-rt/lib/builtins/cpu_model.c:1571:2: error: No support for
checking hwcap on this platform yet.
1571 | #error No support for checking hwcap on this platform yet.
| ^
2 errors generated.
```
Before this change, most of this whole file was ifdeffed out when building on
Windows (and Apple platforms, I would presume), but now most of it is included,
then hitting this `#error`.
I guess it could work to just remove the `#error` cases, but this file suffers
from a pretty deep ifdef nesting jungle, so I'm not sure if that's the best
solution. (FWIW, if we wanted to add aarch64 CPU feature detection for Windows
here, the code would be more of a separate codepath just like the Apple case,
it doesn't share the linux/BSD HWCAP style.)
I can push a quick fix, either removing the `#error` or reverting this commit,
later during the day.
BTW, when compiling the file I also get a bunch of warnings in this style:
```
llvm-project/compiler-rt/lib/builtins/cpu_model.c:1448:36: warning: value size
does not match register size specified by the constraint and modifier
[-Wasm-operand-widths]
1448 | getCPUFeature(ID_AA64PFR1_EL1, ftr);
|^
llvm-project/compiler-rt/lib/builtins/cpu_model.c:1448:5: note: use constraint
modifier "w"
1448 | getCPUFeature(ID_AA64PFR1_EL1, ftr);
| ^
llvm-project/compiler-rt/lib/builtins/cpu_model.c:1345:45: note: expanded from
macro 'getCPUFeature'
1345 | #define getCPUFeature(id, ftr) __asm__("mrs %0, " #id : "=r"(ftr))
|
```
https://github.com/llvm/llvm-project/pull/73685
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Cygwin] Cygwin driver (PR #74933)
mstorsjo wrote: > > @carlo-bramini has spent some effort on using Clang in Cygwin environments > > before, so as far as I know, it does work in general from before. So this > > change, which adds an entirely new driver for Cygwin environments, would > > need to be explained why it does that (I don't disagree, it's probably the > > right thing to do in general), how things worked before and how this > > changes things. And I would like to have @carlo-bramini's eye on this (and > > all the related Cygwin patches from @xu-chiheng). > > And changes like this need some general tests, have a look at > > `clang/test/Driver` for how other drivers are tested. > > The Cygwin driver is basically the same as MinGW driver with minor difference. Right. What are the actual observable differences to what was executed before? (I presume this before used the `Generic_GCC` toolchain?) Also, I wonder if it would make sense to share the MinGW driver code with Cygwin, and just add exceptions where necessary, instead of duplicating it into a separate one? Maybe, but perhaps not. https://github.com/llvm/llvm-project/pull/74933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [MinGW] MinGW dynamicbase (PR #74979)
mstorsjo wrote: > > Also > > In Cygwin with binutils 2.41, --dynamicbase make a difference, so I thought > MinGW also need it. No, MinGW does not need it, as it has been enabled by default since binutils 2.36. Apparently that change, https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=514b4e191d5f46de8e142fe216e677a35fa9c4bb, didn't apply to Cygwin but only to MinGW. But if Cygwin works with dynamicbase (I think it might have issues with it but I'm not sure?) then perhaps binutils should be changed to enable dynamicbase by default there, instead of changing compilers to pass the option by default. https://github.com/llvm/llvm-project/pull/74979 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [MinGW] Fix the regression caused by commit 592e935e115ffb451eb9b782376711dab6558fe0, that, in MinGW, Clang can't be built by system Clang 15.0.4. (PR #74982)
mstorsjo wrote: > I have build scripts and patches at: https://github.com/xu-chiheng/Note This does not answer the question. You need to explain what is broken, and why, and how this fixes it. And address the concern that this actually breaks functionality in some cases. I guess this partially answers the question on in what exact environment the issue occurs, although it would require me to dissect your build script environment to figure it out. https://github.com/llvm/llvm-project/pull/74982 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [MinGW] MinGW dynamicbase (PR #74979)
https://github.com/mstorsjo requested changes to this pull request. No, you do not need to do this. There's no need to add `--dynamicbase` manually in Clang. As I already posted, both ld.bfd and ld.lld default to `--dynamicbase` enabled since 2020. https://github.com/llvm/llvm-project/pull/74979 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [MinGW] Fix the regression caused by commit 592e935e115ffb451eb9b782376711dab6558fe0, that, in MinGW, Clang can't be built by system Clang 15.0.4. (PR #74982)
mstorsjo wrote: I don't know what issue/regression you're referring to. Please explain, in detail, what the issue is and all the relevant aspects of your configuration. Also explain what the suggested fix does, and how it handles the various cases (I just tested building latest llvm-project main with Clang 15 and LLD, for a mingw target, and it worked just fine, both as a regular non-dylib build, and with `LLVM_LINK_LLVM_DYLIB` enabled.) I also believe that the suggested patch would break actual use of clang-repl; if `LLVM_BUILD_LLVM_DYLIB` or `LLVM_BUILD_SHARED_LIBS` aren't defined, then those symbols wouldn't be dllexported at all. This causes them to not be found at runtime when the JIT runtime tries to locate them. https://github.com/llvm/llvm-project/pull/74982 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Cygwin] Cygwin driver (PR #74933)
mstorsjo wrote: @carlo-bramini has spent some effort on using Clang in Cygwin environments before, so as far as I know, it does work in general from before. So this change, which adds an entirely new driver for Cygwin environments, would need to be explained why it does that (I don't disagree, it's probably the right thing to do in general), how things worked before and how this changes things. And I would like to have @carlo-bramini's eye on this (and all the related Cygwin patches from @xu-chiheng). And changes like this need some general tests, have a look at `clang/test/Driver` for how other drivers are tested. https://github.com/llvm/llvm-project/pull/74933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [MinGW] MinGW pthread (PR #74981)
mstorsjo wrote: This breaks bootstrapping llvm-mingw. Not all mingw environments use or require pthreads; llvm-mingw is one such environment, and the clang64 environment in msys2 is another one. While llvm-mingw does contain winpthreads, it is built later in the build process, and if this patch is applied, the setup procedure is broken; one would need to reorder how these libraries are linked, or create a dummy empty `libpthread.a` to make sure that linking works until the read winpthreads library is built. Note that within msys2, they do apply a patch that does exactly what this patch does, for the mingw64 environment, where the system libstdc++ and similar does require winpthreads. The fact that this is patched for the GCC environments isn't ideal, but any attempt to modify this needs to first acknowledge the current state of things and not just blindly barge ahead with a breaking change like this. Also do note that the upcoming GCC 14 will have the win32 thread model supporting C++11, so it is quite possible for GCC based environments to stop relying so much on winpthreads, which would reduce the need for this patch. CC @mati865 @lazka @jeremyd2019 https://github.com/llvm/llvm-project/pull/74981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [MinGW] MinGW dynamicbase (PR #74979)
https://github.com/mstorsjo requested changes to this pull request. This is not necessary. Since 514b4e191d5f46de8e142fe216e677a35fa9c4bb in binutils (https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=514b4e191d5f46de8e142fe216e677a35fa9c4bb), dynamicbase is enabled by default. Also since e72403f96de7f1c681acd5968f72aa986412dfce in llvm-project, LLD also does the same. https://github.com/llvm/llvm-project/pull/74979 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Stub out gcc_struct attribute (PR #71148)
mstorsjo wrote: > Right, I'd just like to make sure that we're not deepening a divergence here. > It would be good to get agreement from the GCC devs that they think > `ms_struct` probably ought to do something on e.g. ARM MinGW targets and that > they consider this a bug (in a feature that they may not really support, > which is fine). But if they think _we're_ wrong and that this really should > only have effect on x86, I would like to know that. I'm not a GCC developer, but I would not think that GCC would consider this an x86-only feature. It just so happens that (upstream) GCC only supports Windows on x86. But MSVC does their own funky bitfield packing on all architectures - so it seems reasonable to want to be able to match it on all architectures. https://github.com/llvm/llvm-project/pull/71148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [clang] [clang][DebugInfo] Revert "emit definitions for constant-initialized static data-members" (PR #74580)
mstorsjo wrote: Could we please land this now? https://github.com/llvm/llvm-project/pull/74580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Stub out gcc_struct attribute (PR #71148)
mstorsjo wrote:
> Okay, @mstorsjo @MaskRay, what is the way forward?
I'm totally not authoritative for these things, but...
> Am I right that, as for the user-facing changes, `[[gcc_struct]]` cancelling
> implicit `-mms-bitfilds` on MinGW is fine
Sounds quite fine for me
> and silently ignoring `-m{no-}ms-bitfields` on `windows-msvc` is not?
Silently ignoring options is clearly not good IMO, so either we warn about them
or implement them
> Should we (and if yes, when exactly) disallow `-m{no-,}ms-bitfields`? Should
> the aforementioned `--target=x86_64-pc-windows-msvc -fc++-abi=itanium
> -mms-bitfields` be accepted?
FWIW I wasn't even aware that it was possible to pick a nondefault C++ ABI, so
I don't have a strong opinion on this matter. If it works and doesn't create
inconsistencies, then I don't mind, but I guess the regular Clang maintainers
have more of a final say on that.
> Is it fine to provide `[[gcc_struct]]` on MSVC because of the reasons I
> outlined before?
I would be totally fine with that.
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Stub out gcc_struct attribute (PR #71148)
mstorsjo wrote: > One more thing. Re binary compatibility concerns: `-mno-ms-bitfields` on > MinGW is an equally-sized footgun as on MSVC. Without proper header > annotation with `#pragma ms_struct on`, either of them will silently make an > ABI mismatch. However, for some reason, supporting `-mno-ms-bitfields` on > MinGW is not argued upon. I guess this is for historical reasons. Originally the MinGW target had this as an opt-in, and this was indeed a silent ABI mismatch. Users who needed to use affected structs and interact with Microsoft APIs had to remember to build their code with `-mms-bitfields` (and hope they don't link in some code that is built without it, with affected structs in their interface). It became the default only by GCC 4.7 (in 2012), and we picked up on this way much later in Clang, in Clang 11 in 2020. https://github.com/llvm/llvm-project/pull/71148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Stub out gcc_struct attribute (PR #71148)
mstorsjo wrote:
> Microsoft bit-field layout didn't break an overly-specific regression test
> but rendered unusable double to string conversion. The culprit was the
> following snippet:
>
> ```c++
> union Extractor {
> double value;
> struct {
> bool sign : 1;
> u32 exponent : 11;
> u64 mantissa : 52;
> };
> };
> ```
>
> According to MSVC ABI, there should be padding between fields. I hope you
> agree that this is not an intuitive and expected behavior.
It is indeed an unexpected thing. However it is possible to work around it for
all these cases; if you declare the inner structure like this:
```
struct {
u64 sign : 1;
u64 exponent : 11;
u64 mantissa : 52;
};
```
Then there will be no extra padding between the fields.
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Stub out gcc_struct attribute (PR #71148)
mstorsjo wrote:
> `-mms-bitfields` is a GCC x86 specific option (`aarch64-linux-gnu-gcc
> -mms-bitfields -xc /dev/null -E` => `error: unrecognized command-line option
> ‘-mms-bitfields’`).
While it is implemented as an x86 specific option in GCC right now, that
doesn't mean that it only is supposed to have an effect for x86. Upstream GCC
only supports Windows on x86, and my recollection is that lots of the Windows
specific logic is located in x86 specific files, even if the same logic also
should apply for Windows on any other architecture - it's just not implemented
(yet).
As implemented in Clang, the option works for any MinGW target.
As an example:
```c
struct field {
unsigned char a : 4;
unsigned int b : 4;
};
int size = sizeof(struct field);
```
```console
$ clang -target x86_64-windows-gnu -S -o - bitfields.c | grep -A1 size
.globl size# @size
.p2align2, 0x0
size:
.long 8 # 0x8
$ clang -target x86_64-windows-gnu -S -o - bitfields.c -mno-ms-bitfields | grep
-A1 size
.globl size# @size
.p2align2, 0x0
size:
.long 4 # 0x4
$ clang -target aarch64-windows-gnu -S -o - bitfields.c | grep -A1 size
.globl size// @size
.p2align2, 0x0
size:
.word 8 // 0x8
$ clang -target aarch64-windows-gnu -S -o - bitfields.c -mno-ms-bitfields |
grep -A1 size
.globl size// @size
.p2align2, 0x0
size:
.word 4 // 0x4
```
---
> https://gitlab.com/qemu-project/qemu/-/issues/1782#note_1495842591 seems like
> ignored `gcc_struct` for windows-gnu targets (feature request #24757).
> I agree that if there are real needs and the needs seem genuine, Clang should
> support `gcc_struct`.
Yes, this would clearly be good to have. For orthogonality, it would be good to
have both `gcc_struct` and `ms_struct` available. GCC does support them on
non-windows targets as well; I think that can be useful for implementing things
like Wine.
---
As noted somewhere (I don't see the comment to quote right now), the MS
bitfield packing logic (as enabled by `-mms-bitfields`) is enabled by default
when using the MSVC C++ ABI, but not when using the Itanium C++ ABI. But as
referenced, since https://reviews.llvm.org/D81795, we do enable the option
`-mms-bitfields` automatically for MinGW targets which otherwise do use the
Itanium C++ ABI. Being able to override this back to the default format for
individual structs would be great.
I don't know and can't speculate about what the implications would be for being
able to switch to GCC struct layout in the MSVC C++ ABI, though. For individual
structs, I guess it should be doable (I'm only thinking for trivial-ish structs
like the one in my example above, right now though). For anything relating to
C++ classes and the mechanisms behind them, I'm pretty sure one doesn't want to
change how anything of that works. Therefore I don't think it's too relevant to
implement `-mno-ms-bitfields` for MSVC targets (as I guess it could open a huge
can of worms).
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Fix an inconsistent indentation (NFC) (PR #72314)
https://github.com/mstorsjo approved this pull request. LGTM, thanks! (I have no idea how I botched that previous fix commit...) https://github.com/llvm/llvm-project/pull/72314 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
mstorsjo wrote: > Hi Phoebe, starting seeing this error on rather old codes after this patch > landed . is there a particular flag you recommend i should compile with to > get previous behavior ? > > error: always_inline function '_mm_setzero_pd' requires target feature > 'evex512', but would be inlined into function '_mm_getexp_pd' that is > compiled without support for 'evex512' I also ran into something similar, when compiling Qt; I filed https://github.com/llvm/llvm-project/issues/72106 with a different reproducer. https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix build dllexport/dllimport issues when doing a shared build for Windows using GCC (PR #66881)
mstorsjo wrote: This is superseded by #71393 which was merged now. https://github.com/llvm/llvm-project/pull/66881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix BUILD_SHARED_LIBS symbols from libclangInterpreter on MinGW (PR #71393)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/71393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Reapply #2 [clang-repl] [test] Make an XFAIL more precise (PR #71168)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/71168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Reapply #2 [clang-repl] [test] Make an XFAIL more precise (PR #71168)
https://github.com/mstorsjo edited https://github.com/llvm/llvm-project/pull/71168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Reapply #2 [clang-repl] [test] Make an XFAIL more precise (PR #71168)
https://github.com/mstorsjo edited https://github.com/llvm/llvm-project/pull/71168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix build dllexport/dllimport issues when doing a shared build for Windows using GCC (PR #66881)
mstorsjo wrote: Thanks, I wasn't aware of this issue (I don't routinely try building with `-DBUILD_SHARED_LIBS=ON`, which I presume is what you've done to trigger this). See 592e935e115ffb451eb9b782376711dab6558fe0 for earlier context on this issue; therefore I'd prefer to fix this as I do in #71393; can you confirm if that change works for you as well? https://github.com/llvm/llvm-project/pull/66881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix BUILD_SHARED_LIBS symbols from libclangInterpreter on MinGW (PR #71393)
mstorsjo wrote: CC @brechtsanders, this is an alternative to #66881. https://github.com/llvm/llvm-project/pull/71393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix BUILD_SHARED_LIBS symbols from libclangInterpreter on MinGW (PR #71393)
https://github.com/mstorsjo created https://github.com/llvm/llvm-project/pull/71393 A few symbols within libclangInterpreter have got explicit dllexport attributes, in order to make them exported (and thus visible at runtime) in any build, not only when they are part of e.g. a DLL libclang-cpp, but also when they are part of a plain .exe. Due to the explicit dllexports, these symbols would sidestep the regular MinGW logic of exporting all symbols if there are no dllexports. Therefore, for libclang-cpp, a separate fix was made in 592e935e115ffb451eb9b782376711dab6558fe0, to pass --export-all-symbols to the build of libclang-cpp. If building with BUILD_SHARED_LIBS enabled, then the same issue appears in libclangInterpreter; pass the same flag --export-all-symbols there as well, to make sure all symbols are visible, not only the ones that are explicitly marked as dllexport. From 9de9617b46bd3c2ff4abd9446afc72c6e91f2493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Mon, 6 Nov 2023 15:42:42 +0200 Subject: [PATCH] [clang-repl] Fix BUILD_SHARED_LIBS symbols from libclangInterpreter on MinGW A few symbols within libclangInterpreter have got explicit dllexport attributes, in order to make them exported (and thus visible at runtime) in any build, not only when they are part of e.g. a DLL libclang-cpp, but also when they are part of a plain .exe. Due to the explicit dllexports, these symbols would sidestep the regular MinGW logic of exporting all symbols if there are no dllexports. Therefore, for libclang-cpp, a separate fix was made in 592e935e115ffb451eb9b782376711dab6558fe0, to pass --export-all-symbols to the build of libclang-cpp. If building with BUILD_SHARED_LIBS enabled, then the same issue appears in libclangInterpreter; pass the same flag --export-all-symbols there as well, to make sure all symbols are visible, not only the ones that are explicitly marked as dllexport. --- clang/lib/Interpreter/CMakeLists.txt | 11 +++ 1 file changed, 11 insertions(+) diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt index 84f6ca5271d2ab0..9065f998f73c473 100644 --- a/clang/lib/Interpreter/CMakeLists.txt +++ b/clang/lib/Interpreter/CMakeLists.txt @@ -38,3 +38,14 @@ add_clang_library(clangInterpreter clangSema clangSerialization ) + +if ((MINGW OR CYGWIN) AND BUILD_SHARED_LIBS) + # The DLLs are supposed to export all symbols (except for ones that are + # explicitly hidden). Normally, this is what happens anyway, but if there + # are symbols that are marked explicitly as dllexport, we'd only export them + # and nothing else. The Interpreter contains a few cases of such dllexports + # (for symbols that need to be exported even from standalone exe files); + # therefore, add --export-all-symbols to make sure we export all symbols + # despite potential dllexports. + target_link_options(clangInterpreter PRIVATE LINKER:--export-all-symbols) +endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
