[clang] [llvm] [Driver] Restore compiler-rt arch suffix for PS and Windows (PR #89775)

2024-05-01 Thread Paul T Robinson via cfe-commits

pogo59 wrote:

Worth noting that this only affects the presumed layout if the driver can't 
find either layout. In most installations, it will detect which one you have 
installed, and use it correctly. It's when the driver can't find the libs, and 
has to guess, that we run into difficulty.

I started out by making additional assumptions based on target, and people 
didn't like that either.

Another way to run this would be to invent a driver option whose default is 
derived from the CMake variable (and possibly target), that will pick the 
presumed layout instead of essentially hardcoding it. Then if you get into 
situations where the driver is guessing wrong, you have an easy (or at least 
easier, depending on your build system) fix: add a command-line option.

This is different from the option in #89642 which stops the driver from using 
the guessed lib name at all, and makes you specify the lib explicitly. My 
suggestion is an option that tells the driver which layout to use as the 
fallback.

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] [llvm] [Driver] Restore compiler-rt arch suffix for PS and Windows (PR #89775)

2024-05-01 Thread Martin Storsjö via cfe-commits

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] [llvm] [Driver] Restore compiler-rt arch suffix for PS and Windows (PR #89775)

2024-05-01 Thread Paul T Robinson via cfe-commits

pogo59 wrote:

I've redone the decision to be based on the setting of 
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR and updated the tests to work with either 
setting as much as possible. Tests that explicitly look for the per-target 
_directory_ are enabled only if the CMake variable is ON.

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] [llvm] [Driver] Restore compiler-rt arch suffix for PS and Windows (PR #89775)

2024-05-01 Thread Paul T Robinson via cfe-commits

https://github.com/pogo59 updated 
https://github.com/llvm/llvm-project/pull/89775

>From c8b04739159fcd4775656aabb4b964272add8c36 Mon Sep 17 00:00:00 2001
From: Paul Robinson 
Date: Tue, 23 Apr 2024 08:10:47 -0700
Subject: [PATCH 1/2] [Driver] Restore arch suffix for PS and Windows

Commit b876596 corrected default compiler-rt library names for many
targets, this patch restores the arch suffix for PlayStation and Windows
which both distribute these libraries with the arch suffix.
---
 clang/lib/Driver/ToolChain.cpp |  4 +++-
 clang/test/Driver/cl-link.c| 16 
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 237092ed07e5dc..d104570491203b 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -684,7 +684,9 @@ std::string ToolChain::getCompilerRT(const ArgList , 
StringRef Component,
 if (Path.empty())
   Path = P;
   }
-  if (getTriple().isOSAIX())
+  // Some targets default to the old layout.
+  if (getTriple().isOSAIX() || getTriple().isPS() ||
+  getTriple().isWindowsMSVCEnvironment())
 Path.clear();
 
   // Check the filename for the old layout if the new one does not exist.
diff --git a/clang/test/Driver/cl-link.c b/clang/test/Driver/cl-link.c
index ffd0b5ac4bade8..444f0c01b3f999 100644
--- a/clang/test/Driver/cl-link.c
+++ b/clang/test/Driver/cl-link.c
@@ -13,20 +13,20 @@
 // ASAN: link.exe
 // ASAN: "-debug"
 // ASAN: "-incremental:no"
-// ASAN: "{{[^"]*}}clang_rt.asan.lib"
-// ASAN: "-wholearchive:{{.*}}clang_rt.asan.lib"
-// ASAN: "{{[^"]*}}clang_rt.asan_cxx.lib"
-// ASAN: "-wholearchive:{{.*}}clang_rt.asan_cxx.lib"
+// ASAN: "{{[^"]*}}clang_rt.asan-i386.lib"
+// ASAN: "-wholearchive:{{.*}}clang_rt.asan-i386.lib"
+// ASAN: "{{[^"]*}}clang_rt.asan_cxx-i386.lib"
+// ASAN: "-wholearchive:{{.*}}clang_rt.asan_cxx-i386.lib"
 // ASAN: "{{.*}}cl-link{{.*}}.obj"
 
 // RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s 
-fuse-ld=link -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s
 // ASAN-MD: link.exe
 // ASAN-MD: "-debug"
 // ASAN-MD: "-incremental:no"
-// ASAN-MD: "{{.*}}clang_rt.asan_dynamic.lib"
-// ASAN-MD: "{{[^"]*}}clang_rt.asan_dynamic_runtime_thunk.lib"
+// ASAN-MD: "{{.*}}clang_rt.asan_dynamic-i386.lib"
+// ASAN-MD: "{{[^"]*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib"
 // ASAN-MD: "-include:___asan_seh_interceptor"
-// ASAN-MD: "-wholearchive:{{.*}}clang_rt.asan_dynamic_runtime_thunk.lib"
+// ASAN-MD: "-wholearchive:{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib"
 // ASAN-MD: "{{.*}}cl-link{{.*}}.obj"
 
 // RUN: %clang_cl /LD -fuse-ld=link -### /Tc%s 2>&1 | FileCheck 
--check-prefix=DLL %s
@@ -40,7 +40,7 @@
 // ASAN-DLL: "-dll"
 // ASAN-DLL: "-debug"
 // ASAN-DLL: "-incremental:no"
-// ASAN-DLL: "{{.*}}clang_rt.asan_dll_thunk.lib"
+// ASAN-DLL: "{{.*}}clang_rt.asan_dll_thunk-i386.lib"
 // ASAN-DLL: "{{.*}}cl-link{{.*}}.obj"
 
 // RUN: %clang_cl /Zi /Tc%s -fuse-ld=link -### 2>&1 | FileCheck 
--check-prefix=DEBUG %s

>From 26ff8b1d3371bc9df13769c77be4214e646eeb50 Mon Sep 17 00:00:00 2001
From: Paul Robinson 
Date: Wed, 1 May 2024 06:47:22 -0700
Subject: [PATCH 2/2] Base the decision on the CMake variable instead

---
 clang/lib/Driver/ToolChain.cpp|   3 +-
 clang/test/Driver/arm-compiler-rt.c   |  39 ++--
 clang/test/Driver/cl-link.c   |  27 +--
 clang/test/Driver/compiler-rt-unwind.c|  35 ++--
 clang/test/Driver/coverage-ld.c   |   3 +-
 clang/test/Driver/fuchsia.c   |   1 +
 clang/test/Driver/instrprof-ld.c  |   1 +
 clang/test/Driver/lit.local.cfg   |   4 +
 clang/test/Driver/mingw-sanitizers.c  |  24 ++-
 clang/test/Driver/msp430-toolchain.c  |   1 +
 .../Driver/print-libgcc-file-name-clangrt.c   |  38 ++--
 clang/test/Driver/sanitizer-ld.c  | 188 ++
 clang/test/Driver/wasm-toolchain.c|  38 ++--
 clang/test/Driver/wasm-toolchain.cpp  |  34 ++--
 clang/test/Driver/windows-cross.c |  40 ++--
 clang/test/Driver/zos-ld.c|   1 +
 clang/test/lit.site.cfg.py.in |   1 +
 llvm/include/llvm/Config/llvm-config.h.cmake  |   3 +
 18 files changed, 270 insertions(+), 211 deletions(-)

diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index d104570491203b..b266cee0a54457 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -685,8 +685,7 @@ std::string ToolChain::getCompilerRT(const ArgList , 
StringRef Component,
   Path = P;
   }
   // Some targets default to the old layout.
-  if (getTriple().isOSAIX() || getTriple().isPS() ||
-  getTriple().isWindowsMSVCEnvironment())
+  if (getTriple().isOSAIX() || !LLVM_ENABLE_PER_TARGET_RUNTIME_DIR)
 Path.clear();
 
   // Check the filename for the old layout if the new one does not exist.
diff --git