Re: [PATCH] D16929: add support for -print-imm-hex for AArch64
pawosm01 added a comment. You're right, this patch smuggles elimination of two dead functions. I've prepared alternative version of this patch which does not do that. Repository: rL LLVM http://reviews.llvm.org/D16929 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Accept -fallow-argument-mismatch with a warning instead of rejecting it (PR #91611)
pawosm-arm wrote: The problem with `-fallow-argument-mismatch` has already been reported to the MPICH project maintainers, https://github.com/pmodels/mpich/issues/6869 therefore I'm going to close this PR. https://github.com/llvm/llvm-project/pull/91611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Accept -fallow-argument-mismatch with a warning instead of rejecting it (PR #91611)
https://github.com/pawosm-arm closed https://github.com/llvm/llvm-project/pull/91611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Enable config file options (PR #100343)
https://github.com/pawosm-arm approved this pull request. This is great, thanks for contributing it. https://github.com/llvm/llvm-project/pull/100343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [lld] [flang] Generate main only when a Fortran program statement is present (PR #89938)
https://github.com/pawosm-arm approved this pull request. The -fno-fortran-main flag should disappear before it finds any widespread use. https://github.com/llvm/llvm-project/pull/89938 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] support -fno-openmp (PR #107087)
https://github.com/pawosm-arm commented: As I'm not seeing how linker is invoked, I just need to ask (for the sake of completeness) will it also prevent addition of `-lomp` to the linker's command line? https://github.com/llvm/llvm-project/pull/107087 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] support -fno-openmp (PR #107087)
https://github.com/pawosm-arm approved this pull request. https://github.com/llvm/llvm-project/pull/107087 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Accept -fallow-argument-mismatch with a warning instead of rejecting it (PR #91611)
https://github.com/pawosm-arm created https://github.com/llvm/llvm-project/pull/91611 Many autotools-utilizing projects (mpich among them) fail to complete the `configure` step since it tries to invoke the (unknown to them) Fortran compiler always with the `-fallow-argument-mismatch` flag. As the use of this flag has no impact on flang, there is no definite need to reject it. It would be better to accept it with a warning and reject the opposite flag `-fno-allow-argument-mismatch` with an error. With this patch, mpich compiles and works. >From afeb4498c0f5b9f4e41e486b28caa0603740ae2c Mon Sep 17 00:00:00 2001 From: Paul Osmialowski Date: Thu, 9 May 2024 16:23:38 +0100 Subject: [PATCH] [flang][driver] Accept -fallow-argument-mismatch with a warning instead of rejecting it Many autotools-utilizing projects (mpich among them) fail to complete the `configure` step since it tries to invoke the (unknown to them) Fortran compiler always with the `-fallow-argument-mismatch` flag. As the use of this flag has no impact on flang, there is no definite need to reject it. It would be better to accept it with a warning and reject the opposite flag `-fno-allow-argument-mismatch` with an error. --- clang/include/clang/Driver/Options.td | 2 ++ flang/test/Driver/allow-argument-mismatch.f90 | 15 +++ 2 files changed, 17 insertions(+) create mode 100644 flang/test/Driver/allow-argument-mismatch.f90 diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 73a2518480e9b..eaa17fdec0f8b 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2540,6 +2540,8 @@ def fsanitize_undefined_strip_path_components_EQ : Joined<["-"], "fsanitize-unde } // end -f[no-]sanitize* flags +def fallow_argument_mismatch : Flag<["-"], "fallow-argument-mismatch">, Visibility<[FlangOption, FC1Option]>; + def funsafe_math_optimizations : Flag<["-"], "funsafe-math-optimizations">, Group, Visibility<[ClangOption, CC1Option]>, HelpText<"Allow unsafe floating-point math optimizations which may decrease precision">, diff --git a/flang/test/Driver/allow-argument-mismatch.f90 b/flang/test/Driver/allow-argument-mismatch.f90 new file mode 100644 index 0..2adf9e67daeab --- /dev/null +++ b/flang/test/Driver/allow-argument-mismatch.f90 @@ -0,0 +1,15 @@ +! Test that -fallow-argument-mismatch flag is accepted with warning, while +! -fno-allow-argument-mismatch is rejected with error + +! RUN: %flang -S %s -fallow-argument-mismatch -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK-WARNING +! RUN: %flang_fc1 -S %s -fallow-argument-mismatch -o /dev/null +! RUN: not %flang -S %s -fno-allow-argument-mismatch -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR +! RUN: not %flang_fc1 -S %s -fno-allow-argument-mismatch -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR + +! CHECK-WARNING: warning: +! CHECK_WARNING-SAME: argument unused +! CHECK_WARNING-SAME: -fallow-argument-mismatch + +! CHECK-ERROR: error: +! CHECK-ERROR-SAME: unknown argument +! CHECK-ERROR-SAME: -fno-allow-argument-mismatch ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Accept -fallow-argument-mismatch with a warning instead of rejecting it (PR #91611)
pawosm-arm wrote: > > It sounds like an issue with the `mpich` build script rather than LLVM Flang. > Why would `mpich` and other projects use this flag to begin with? Do other > Fortran compilers support it? > I don't know why they do that, I suppose they think they know better. I don't suspect they are aware of the pain they brought to the users (of various Fortran compilers). > From GFrotran docs > (https://gcc.gnu.org/onlinedocs/gfortran/Fortran-Dialect-Options.html): > > > Using this option is strongly discouraged. > As I said, I suppose they think they know better. > In general, I'm hesitant to allow such dummy flags that: > > * do nothing, > * are there merely to prevent issues in build scripts. > > Are there no other ways around it? We obviously should make it possible for > LLVM Flang to build all projects, but I really want to avoid Flang containing > multiple dummy flags only to work around build scripts that assume a > particular compiler is used. > The alternative is to instruct the users how to patch the `configure` scripts. There is always a trade-off between disgust (caused by introduction of ugly flags) and battered user experience (caused by the compiler rejecting a flag that someone in some widely used project insisted that should be accepted). We could document the problem and describe how to work around it, however we must carefully consider the maintenance cost of this extra bit of documentation. > We might be missing some "gfortran compatibility mode" in Flang (on top of > `FlangMode`). Such mode could allow unsupported GFortran flags and warn > whenever those are used. `flang-new` would still "default" to `FlangMode` and > just generate an error. > Yes, we agreed long time ago that gfortran isn't the gold standard of how the Fortran compiler should behave. But the reality always knows it better. And things like mpich or netcdf are widely used in the exact environment we want to target with our compiler. https://github.com/llvm/llvm-project/pull/91611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix lookup of dependent operator= named by using-declaration (PR #91503)
pawosm-arm wrote: The number of C++ workloads failing to build after #90152 is rather impressive, even this PR do not help. Is there a plan to address that? https://github.com/llvm/llvm-project/pull/91503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix lookup of dependent operator= named by using-declaration (PR #91503)
pawosm-arm wrote: I'm sorry, I didn't notice that there is more discussion after #90152 covering also my issue at hand. The obvious advice I can get from those is the 'this is not valid c++, fix your code' suggestion. Trouble starts when there is a lot of it to fix, even if it mostly involve deletion of the functions that aren't instantiated. https://github.com/llvm/llvm-project/pull/91503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] When linking with the Fortran runtime, the `addArchSpecificRPath()` should be called too (PR #114837)
https://github.com/pawosm-arm created https://github.com/llvm/llvm-project/pull/114837 When linking with other runtimes (OpenMP, sanitizers), the addArchSpecificRPath() is being called. The same thing should happen when linking with the Fortran runtime, this will improve user experience massively. >From 5f6614172bd634e871c5e2c837e4310a0de7f98f Mon Sep 17 00:00:00 2001 From: Paul Osmialowski Date: Mon, 4 Nov 2024 11:50:57 + Subject: [PATCH] [flang][Driver] When linking with the Fortran runtime, the addArchSpecificRPath() should be called too When linking with other runtimes (OpenMP, sanitizers), the addArchSpecificRPath() is being called. The same thing should happen when linking with the Fortran runtime, this will improve user experience massively. --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 1 + flang/test/Driver/arch-specific-libdir-rpath.f95 | 7 +++ 2 files changed, 8 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 1c3c8c816594e5..ca06fb115dfa11 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1293,6 +1293,7 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, } CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); +addArchSpecificRPath(TC, Args, CmdArgs); } // libomp needs libatomic for atomic operations if using libgcc diff --git a/flang/test/Driver/arch-specific-libdir-rpath.f95 b/flang/test/Driver/arch-specific-libdir-rpath.f95 index cc09938f7d1e28..23fb52abfbd574 100644 --- a/flang/test/Driver/arch-specific-libdir-rpath.f95 +++ b/flang/test/Driver/arch-specific-libdir-rpath.f95 @@ -16,6 +16,13 @@ ! ! Test that -rpath is added ! +! Add LIBPATH, RPATH +! +! RUN: %flang %s -### --target=x86_64-linux \ +! RUN: -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir \ +! RUN: -frtlib-add-rpath 2>&1 \ +! RUN: | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s +! ! Add LIBPATH, RPATH for OpenMP ! ! RUN: %flang %s -### --target=x86_64-linux -fopenmp \ ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Transforms][Utils][PromoteMem2Reg] Propagate nnan flag on par with the nsz flag (PR #114271)
pawosm-arm wrote: > We should probably also handle `no-infs-fp-math` to save the next person the > trouble. I'd leave it to a separate PR, as this one has a specific story behind the proposed change, anything extra would go beyond it. https://github.com/llvm/llvm-project/pull/114271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Transforms][Utils][PromoteMem2Reg] Propagate nnan flag on par with the nsz flag (PR #114271)
pawosm-arm wrote: > > Yet we see a value in the change proposed here too hence a good reason for > > merging it. > > I've clearly misunderstood the context of the rebase. What is the value in > merging this change if the original issue has already been resolved? I guess > this fix is simpler and thus might help compile time but then that wasn't the > original intent. There are two views at this situation: One, I can imagine many ways the `nnan` flag could not be retrieved if it isn't ensured (the way my patch is doing it), Dave's patch solves the problem more elaborate way we know it works in specific situations, yet how can we be sure there aren't other places in which `nnan` should be easily visible if a transformation should succeed? The other is that there's a general movement away from having the function attributes (in favor of using only the instruction flags) and thus by adding another use we make this transition harder. My PR will catch all cases and has very low complexity. Seems I need @nikic opinion on the subject, whether we should land it or abandon it. https://github.com/llvm/llvm-project/pull/114271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] When linking with the Fortran runtime, the `addArchSpecificRPath()` should be called too (PR #114837)
pawosm-arm wrote: > `addSanitizerRuntime` and `addOpenMPRuntime` are already calling > `addArchSpecificRPath`. It it a problem if rpath is added multiple times? > I've been testing it with and without -fopenmp and didn't observe any problem > Compiler-rt libs (`libclang_rt.*.a`) are added as absolute paths, shouldn't > the Fortran-runtime do the same? This makes sense for static libraries, Fortran-runtime can be built static or shared, and in case of shared lib, the rpath issue starts to kick in. https://github.com/llvm/llvm-project/pull/114837 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Transforms][Utils][PromoteMem2Reg] Propagate nnan flag on par with the nsz flag (PR #114271)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/114271 >From a426e17ed3c3cad6b3c813bd996e649a2e544591 Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Wed, 30 Oct 2024 14:30:08 + Subject: [PATCH] [Transforms][Utils][PromoteMem2Reg] Propagate nnan and ninf flags on par with the nsz flag Following the change introduced by the PR #83381, this patch extends it with the same treatment of the nnan and ninf fast-math flags. This is to address the performance drop caused by PR#83200 which prevented vital InstCombine transformation due to the lack of the relevant fast-math flag. The PromoteMem2Reg utility is used by the SROA pass, where Phi nodes are being created. Proposed change allows propagation of the nnan and ninf flags down to these Phi nodes. --- clang/test/Headers/__clang_hip_math.hip | 8 +- .../Utils/PromoteMemoryToRegister.cpp | 17 +++ .../SROA/propagate-fast-math-flags-on-phi.ll | 134 ++ 3 files changed, 155 insertions(+), 4 deletions(-) diff --git a/clang/test/Headers/__clang_hip_math.hip b/clang/test/Headers/__clang_hip_math.hip index e4254d1e64bec9..f64165e2e4bc62 100644 --- a/clang/test/Headers/__clang_hip_math.hip +++ b/clang/test/Headers/__clang_hip_math.hip @@ -1727,7 +1727,7 @@ extern "C" __device__ double test_j1(double x) { // FINITEONLY-NEXT:[[EXITCOND_NOT:%.*]] = icmp eq i32 [[INC_I]], [[X]] // FINITEONLY-NEXT:br i1 [[EXITCOND_NOT]], label [[_ZL3JNFIF_EXIT]], label [[FOR_BODY_I]], !llvm.loop [[LOOP14:![0-9]+]] // FINITEONLY: _ZL3jnfif.exit: -// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi float [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] +// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi nnan ninf float [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] // FINITEONLY-NEXT:ret float [[RETVAL_0_I]] // // APPROX-LABEL: @test_jnf( @@ -1830,7 +1830,7 @@ extern "C" __device__ float test_jnf(int x, float y) { // FINITEONLY-NEXT:[[EXITCOND_NOT:%.*]] = icmp eq i32 [[INC_I]], [[X]] // FINITEONLY-NEXT:br i1 [[EXITCOND_NOT]], label [[_ZL2JNID_EXIT]], label [[FOR_BODY_I]], !llvm.loop [[LOOP15:![0-9]+]] // FINITEONLY: _ZL2jnid.exit: -// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi double [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] +// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi nnan ninf double [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] // FINITEONLY-NEXT:ret double [[RETVAL_0_I]] // // APPROX-LABEL: @test_jn( @@ -4461,7 +4461,7 @@ extern "C" __device__ double test_y1(double x) { // FINITEONLY-NEXT:[[EXITCOND_NOT:%.*]] = icmp eq i32 [[INC_I]], [[X]] // FINITEONLY-NEXT:br i1 [[EXITCOND_NOT]], label [[_ZL3YNFIF_EXIT]], label [[FOR_BODY_I]], !llvm.loop [[LOOP24:![0-9]+]] // FINITEONLY: _ZL3ynfif.exit: -// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi float [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] +// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi nnan ninf float [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] // FINITEONLY-NEXT:ret float [[RETVAL_0_I]] // // APPROX-LABEL: @test_ynf( @@ -4564,7 +4564,7 @@ extern "C" __device__ float test_ynf(int x, float y) { // FINITEONLY-NEXT:[[EXITCOND_NOT:%.*]] = icmp eq i32 [[INC_I]], [[X]] // FINITEONLY-NEXT:br i1 [[EXITCOND_NOT]], label [[_ZL2YNID_EXIT]], label [[FOR_BODY_I]], !llvm.loop [[LOOP25:![0-9]+]] // FINITEONLY: _ZL2ynid.exit: -// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi double [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] +// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi nnan ninf double [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] // FINITEONLY-NEXT:ret double [[RETVAL_0_I]] // // APPROX-LABEL: @test_yn( diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp index 656bb1ebd1161e..d8657de3e58b09 100644 --- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp +++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp @@ -394,6 +394,12 @@ struct PromoteMem2Reg { /// Whether the function has the no-signed-zeros-fp-math attribute set. bool NoSignedZeros = false; + /// Whether the function has the no-nans-fp-math attribute set. + bool NoNaNs = false; + + /// Whether th
[clang] [llvm] [Transforms][Utils][PromoteMem2Reg] Propagate nnan flag on par with the nsz flag (PR #114271)
pawosm-arm wrote: > > I've rebased my PR so this branch could also contain phase-ordering test > > for vectorizing predicated selects introduced by commit > > [577c7dd](https://github.com/llvm/llvm-project/commit/577c7dd7cc4c5a9f62f9654cfa30ee9d55709426) > > Do I understand correctly that this PR doesn't change the PhaseOrdering test > because the motivating issue was already addressed by > [0f91944](https://github.com/llvm/llvm-project/commit/0f919444adfa74b46df50aa2da12e7137c23a02c)? Yes, the original problem that I've faced could be addressed various ways, and 0f91944 indeed fixes it too. Yet we see a value in the change proposed here too hence a good reason for merging it. https://github.com/llvm/llvm-project/pull/114271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] When linking with the Fortran runtime, the `addArchSpecificRPath()` should be called too (PR #114837)
pawosm-arm wrote: If there is no objections by others, I'll merge it some time tomorrow. https://github.com/llvm/llvm-project/pull/114837 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] When linking with the Fortran runtime, the `addArchSpecificRPath()` should be called too (PR #114837)
pawosm-arm wrote: > `addSanitizerRuntime` and `addOpenMPRuntime` are already calling > `addArchSpecificRPath`. It it a problem if rpath is added multiple times? For a reference, in case of Clang and C code utilizing OpenMP, -rpath is given at least twice: `"/usr/bin/ld" "-rpath" "/opt/llvm/lib/clang/20/lib/aarch64-unknown-linux-gnu" "-rpath" "/opt/llvm/bin/../lib/aarch64-unknown-linux-gnu" "-L/opt/llvm/lib"` https://github.com/llvm/llvm-project/pull/114837 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Transforms][Utils][PromoteMem2Reg] Propagate nnan flag on par with the nsz flag (PR #114271)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/114271 >From 45a02d4dd6102bf825943e738a15adf64361f976 Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Wed, 30 Oct 2024 14:30:08 + Subject: [PATCH] [Transforms][Utils][PromoteMem2Reg] Propagate nnan and ninf flags on par with the nsz flag Following the change introduced by the PR #83381, this patch extends it with the same treatment of the nnan and ninf fast-math flags. This is to address the performance drop caused by PR#83200 which prevented vital InstCombine transformation due to the lack of the relevant fast-math flag. The PromoteMem2Reg utility is used by the SROA pass, where Phi nodes are being created. Proposed change allows propagation of the nnan and ninf flags down to these Phi nodes. --- clang/test/Headers/__clang_hip_math.hip | 8 +- .../Utils/PromoteMemoryToRegister.cpp | 33 - .../SROA/propagate-fast-math-flags-on-phi.ll | 134 ++ 3 files changed, 164 insertions(+), 11 deletions(-) diff --git a/clang/test/Headers/__clang_hip_math.hip b/clang/test/Headers/__clang_hip_math.hip index e4254d1e64bec9..f64165e2e4bc62 100644 --- a/clang/test/Headers/__clang_hip_math.hip +++ b/clang/test/Headers/__clang_hip_math.hip @@ -1727,7 +1727,7 @@ extern "C" __device__ double test_j1(double x) { // FINITEONLY-NEXT:[[EXITCOND_NOT:%.*]] = icmp eq i32 [[INC_I]], [[X]] // FINITEONLY-NEXT:br i1 [[EXITCOND_NOT]], label [[_ZL3JNFIF_EXIT]], label [[FOR_BODY_I]], !llvm.loop [[LOOP14:![0-9]+]] // FINITEONLY: _ZL3jnfif.exit: -// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi float [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] +// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi nnan ninf float [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] // FINITEONLY-NEXT:ret float [[RETVAL_0_I]] // // APPROX-LABEL: @test_jnf( @@ -1830,7 +1830,7 @@ extern "C" __device__ float test_jnf(int x, float y) { // FINITEONLY-NEXT:[[EXITCOND_NOT:%.*]] = icmp eq i32 [[INC_I]], [[X]] // FINITEONLY-NEXT:br i1 [[EXITCOND_NOT]], label [[_ZL2JNID_EXIT]], label [[FOR_BODY_I]], !llvm.loop [[LOOP15:![0-9]+]] // FINITEONLY: _ZL2jnid.exit: -// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi double [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] +// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi nnan ninf double [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] // FINITEONLY-NEXT:ret double [[RETVAL_0_I]] // // APPROX-LABEL: @test_jn( @@ -4461,7 +4461,7 @@ extern "C" __device__ double test_y1(double x) { // FINITEONLY-NEXT:[[EXITCOND_NOT:%.*]] = icmp eq i32 [[INC_I]], [[X]] // FINITEONLY-NEXT:br i1 [[EXITCOND_NOT]], label [[_ZL3YNFIF_EXIT]], label [[FOR_BODY_I]], !llvm.loop [[LOOP24:![0-9]+]] // FINITEONLY: _ZL3ynfif.exit: -// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi float [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] +// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi nnan ninf float [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] // FINITEONLY-NEXT:ret float [[RETVAL_0_I]] // // APPROX-LABEL: @test_ynf( @@ -4564,7 +4564,7 @@ extern "C" __device__ float test_ynf(int x, float y) { // FINITEONLY-NEXT:[[EXITCOND_NOT:%.*]] = icmp eq i32 [[INC_I]], [[X]] // FINITEONLY-NEXT:br i1 [[EXITCOND_NOT]], label [[_ZL2YNID_EXIT]], label [[FOR_BODY_I]], !llvm.loop [[LOOP25:![0-9]+]] // FINITEONLY: _ZL2ynid.exit: -// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi double [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] +// FINITEONLY-NEXT:[[RETVAL_0_I:%.*]] = phi nnan ninf double [ [[CALL_I20_I]], [[IF_THEN_I]] ], [ [[CALL_I22_I]], [[IF_THEN2_I]] ], [ [[CALL_I21_I]], [[IF_END4_I]] ], [ [[SUB_I]], [[FOR_BODY_I]] ] // FINITEONLY-NEXT:ret double [[RETVAL_0_I]] // // APPROX-LABEL: @test_yn( diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp index 656bb1ebd1161e..aa3f825265f816 100644 --- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp +++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp @@ -394,6 +394,12 @@ struct PromoteMem2Reg { /// Whether the function has the no-signed-zeros-fp-math attribute set. bool NoSignedZeros = false; + /// Whether the function has the no-nans-fp-math attribute set. + bool NoNaNs = false; + + /// Whether
[clang] [flang] [flang][driver] Make -stdlib= option visible to flang and silently ignored by it (PR #110598)
pawosm-arm wrote: There was a change of plans, so I don't need it now. But I expect the subject will return sooner or later raised by someone else who stumble upon this just like me. Closing it for now. https://github.com/llvm/llvm-project/pull/110598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Make -stdlib= option visible to flang and silently ignored by it (PR #110598)
https://github.com/pawosm-arm closed https://github.com/llvm/llvm-project/pull/110598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang-Repl] Add support for out-of-process execution. (PR #110418)
pawosm-arm wrote: Ah, so it isn't failing only for me. Apparently, it blows away all the environments where ORC is deliberately disabled. https://github.com/llvm/llvm-project/pull/110418 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Make -stdlib= option visible to flang and silently ignored by it (PR #110598)
pawosm-arm wrote: > I don't think it is correct to say that the option was ignored. It may still > be read by the compiler driver (which generates the link line). In fact it > probably _should_ be processed by the compiler driver in order to set the C++ > standard library correctly - otherwise you could just not set this in your > `LDFLAGS`. But this (ignoring) is exactly that is happening. Contrary to classic flang, flang-new cannot be used to mix C++ and Fortran code in one compiler invocation, and working on this patch I've even found a test case guarding that controversial decision (`driver-error-cc1.cpp`). This is something that would probably trigger further discussion as angry users may soon emerge as flang-new's popularity grows, but for now, I've found that even if I have a project for which its Makefile builds C++ and Fortran sources separately, it fails at link time if I'm trying to use specific standard C++ library by setting designated CMake flag (which in turn, sets C++ and linker flags for the project globally). I could go through all CMake generated link.txt files and adjust the flags, but this exposes a terrible user experience as: 1) the user must be experienced enough to know how this must be modified 2) it introduces annoying extra step between cmake invocation and make/ninja invocation. Using this flag for compilation of a Fortran code will display a warining: ``` flang-new: warning: argument unused during compilation: '-stdlib=platform' [-Wunused-command-line-argument] ``` ...and this is expected. Using this flag for linking will not do anything, looking at `-###` output with and without using this flag shows no difference, ergo it is being ingored. https://github.com/llvm/llvm-project/pull/110598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Make -stdlib= option visible to flang and silently ignored by it (PR #110598)
@@ -0,0 +1,48 @@ +// REQUIRES: system-linux pawosm-arm wrote: As I stated in my other comment, the `-###` output is not affected by the `-stdlib=` flag, so a test which looks at its output would not provide a great value. The integration test from the other hand guards that the real-life scenario that I've encountered could be resolved with `flang-new`. https://github.com/llvm/llvm-project/pull/110598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Make -stdlib= option visible to flang and silently ignored by it (PR #110598)
pawosm-arm wrote: > If this is completely ignored and does not effect the link line, are you sure > the resulting binary actually uses the right c++ library? > > Edit: flang will not link a C++ library. It is not a C++ compiler. As we discussed it offline, in case of this project it does not matter, no calls to the library are actually being made from the C++ code, and it was more important what CMake did with CXXFLAGS (so the headers from the right C++ standard library were used). Also the integration test I've added depicts that flang-new doesn't really need to know anything about the language and standard libraries they use. The trouble here is that linker flags were blindly propagated by CMake (when asked to specify C++ standard library) no matter if it has any use or not. In smaller project, where we know that C++ the part isn't actually calling anything from library, the -stdlib flang should not be used during linking at all, but here we're talking about large scale project, where the Flang/C++ part is just a small part so the project maintainers didn't really care about making an exception in the flags being used when linking just that part. https://github.com/llvm/llvm-project/pull/110598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Make -stdlib= option visible to flang and silently ignored by it (PR #110598)
pawosm-arm wrote: > Note that the test will fail when `LLVM_DEFAULT_TARGET_TRIPLE` is set to > anything else than the host platform. Because of this. Clang tests never > invoke the linker, but check the output of `-###`. However, Flang already has > tests with the same issue. I wonder how the other C++ integration test in this directory (`iso-fortran-binding.cpp`) is not affected, it is not only linking but also executing what has been linked, which I wanted to avoid, I didn't want to run into the issues with RPaths. I also wanted to bring your attention to the subject in context of your work on extracting Fortran Runtime, what I've noticed is that using `-DLLVM_ENABLE_LIBCXX=True` CMake flag has unexpected effect when it comes to C++/Fortran mixed code. https://github.com/llvm/llvm-project/pull/110598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Make -stdlib= option visible to flang and silently ignored by it (PR #110598)
pawosm-arm wrote: > (and there isn't even a warning that the option is unused) > > @pawosm-arm Wasn't this ever an issue? Should those to be consistent? Interesting, clang would do the same as flang-new patched with my patch. Maybe addition of the warning would be a good idea, if it isn't an overkill. https://github.com/llvm/llvm-project/pull/110598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add the ability to specify that RPATH should be added by default (PR #115163)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/115163 >From 2c4b767437c55245391eb15da94a97b34a63f834 Mon Sep 17 00:00:00 2001 From: Paul Osmialowski Date: Tue, 5 Nov 2024 16:38:22 + Subject: [PATCH] [clang][Driver] Add the ability to specify that RPATH should be added by default The `-frtlib-add-rpath` is very convenient when linking against various runtimes (OpenMP, Fortran, sanitizers), so much so we could argue that this should be a default behavior. This patch adds the ability to specify (at CMake level) that RPATH should be added by default. --- clang/CMakeLists.txt | 2 + clang/include/clang/Config/config.h.cmake | 3 + clang/lib/Driver/ToolChains/CommonArgs.cpp| 4 + clang/test/CMakeLists.txt | 1 + .../arch-specific-libdir-rpath-by-default.c | 105 ++ .../test/Driver/arch-specific-libdir-rpath.c | 2 + clang/test/lit.cfg.py | 3 + clang/test/lit.site.cfg.py.in | 1 + flang/test/CMakeLists.txt | 1 + .../arch-specific-libdir-rpath-by-default.f95 | 40 +++ .../Driver/arch-specific-libdir-rpath.f95 | 3 +- flang/test/Driver/linker-flags.f90| 3 +- flang/test/lit.cfg.py | 4 + flang/test/lit.site.cfg.py.in | 1 + 14 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 clang/test/Driver/arch-specific-libdir-rpath-by-default.c create mode 100644 flang/test/Driver/arch-specific-libdir-rpath-by-default.f95 diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 27e8095534a65c..fb902cd65a31da 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -216,6 +216,8 @@ endif() set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld") +set(ENABLE_LINKER_RPATH_BY_DEFAULT OFF CACHE BOOL "pass -rpath to the linker by default") + set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "enable x86 relax relocations by default") diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 27ed69e21562bf..a7f185d88c68cc 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -69,6 +69,9 @@ /* pass --build-id to ld */ #cmakedefine ENABLE_LINKER_BUILD_ID +/* pass -rpath to the linker by default */ +#cmakedefine ENABLE_LINKER_RPATH_BY_DEFAULT + /* enable x86 relax relocations by default */ #cmakedefine01 ENABLE_X86_RELAX_RELOCATIONS diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index ca06fb115dfa11..e62f59f56241cd 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1183,7 +1183,11 @@ void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC, void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args, ArgStringList &CmdArgs) { if (!Args.hasFlag(options::OPT_frtlib_add_rpath, +#ifdef ENABLE_LINKER_RPATH_BY_DEFAULT +options::OPT_fno_rtlib_add_rpath, true)) +#else options::OPT_fno_rtlib_add_rpath, false)) +#endif return; SmallVector CandidateRPaths(TC.getArchSpecificLibPaths()); diff --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt index 5369dc92f69e8a..e439e973ce6f81 100644 --- a/clang/test/CMakeLists.txt +++ b/clang/test/CMakeLists.txt @@ -11,6 +11,7 @@ llvm_canonicalize_cmake_booleans( CLANG_SPAWN_CC1 CLANG_ENABLE_CIR ENABLE_BACKTRACES + ENABLE_LINKER_RPATH_BY_DEFAULT LLVM_BUILD_EXAMPLES LLVM_BYE_LINK_INTO_TOOLS LLVM_ENABLE_PLUGINS diff --git a/clang/test/Driver/arch-specific-libdir-rpath-by-default.c b/clang/test/Driver/arch-specific-libdir-rpath-by-default.c new file mode 100644 index 00..20c3f2fa560905 --- /dev/null +++ b/clang/test/Driver/arch-specific-libdir-rpath-by-default.c @@ -0,0 +1,105 @@ +// Test that the driver adds an arch-specific subdirectory in +// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' +// +// REQUIRES: enable_rpath_by_default +// +// Test the default behavior when neither -frtlib-add-rpath nor +// -fno-rtlib-add-rpath is specified, which is to add -rpath +// RUN: %clang %s -### --target=x86_64-linux \ +// RUN: -fsanitize=address -shared-libasan \ +// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir 2>&1 \ +// RUN: | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s +// +// Test that -rpath is not added under -fno-rtlib-add-rpath even if other +// conditions are met. +// RUN: %clang %s -### --target=x86_64-linux \ +// RUN: -fsanitize=address -shared-libasan \ +// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ +// RUN: -fno-rtlib-add-rpath 2>&1 \ +// RUN: | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s +// +// Test that -rpath is ad
[clang] [flang] [clang][Driver] Add the ability to specify that RPATH should be added by default (PR #115163)
pawosm-arm wrote: We worked out this patch just because we dropped the idea of adding configuration file to our toolchain and for no other reason... https://github.com/llvm/llvm-project/pull/115163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add RPATH by default (PR #115286)
pawosm-arm wrote: > Some distributions (e.g. Fedora) have policies against DT_RUNPATH for system > packages. For as long as I remember, distros (and Fedora is a good example of this phenomenon) were patching the software packages heavily, so adding one more to ensure their well established policies wouldn't make a huge difference. https://github.com/llvm/llvm-project/pull/115286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Transforms][Utils][PromoteMem2Reg] Propagate nnan and ninf flags on par with the nsz flag (PR #114271)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/114271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add the ability to specify that RPATH should be added by default (PR #115163)
https://github.com/pawosm-arm created https://github.com/llvm/llvm-project/pull/115163 The `-frtlib-add-rpath` is very convenient when linking against various runtimes (OpenMP, Fortran, sanitizers), so much so we could argue that this should be a default behavior. This patch adds the ability to specify (at CMake level) that RPATH should be added by default. >From 6a63fbed36fea9b7728e53979d3a84e816110f06 Mon Sep 17 00:00:00 2001 From: Paul Osmialowski Date: Tue, 5 Nov 2024 16:38:22 + Subject: [PATCH] [clang][Driver] Add the ability to specify that RPATH should be added by default The `-frtlib-add-rpath` is very convenient when linking against various runtimes (OpenMP, Fortran, sanitizers), so much so we could argue that this should be a default behavior. This patch adds the ability to specify (at CMake level) that RPATH should be added by default. --- clang/CMakeLists.txt | 2 + clang/include/clang/Config/config.h.cmake | 3 + clang/lib/Driver/ToolChains/CommonArgs.cpp| 4 + clang/test/CMakeLists.txt | 1 + .../arch-specific-libdir-rpath-by-default.c | 105 ++ .../test/Driver/arch-specific-libdir-rpath.c | 2 + clang/test/lit.cfg.py | 3 + clang/test/lit.site.cfg.py.in | 1 + flang/test/CMakeLists.txt | 1 + .../arch-specific-libdir-rpath-by-default.f95 | 34 ++ .../Driver/arch-specific-libdir-rpath.f95 | 2 + flang/test/Driver/linker-flags.f90| 3 +- flang/test/lit.cfg.py | 4 + flang/test/lit.site.cfg.py.in | 1 + 14 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 clang/test/Driver/arch-specific-libdir-rpath-by-default.c create mode 100644 flang/test/Driver/arch-specific-libdir-rpath-by-default.f95 diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 27e8095534a65c..fb902cd65a31da 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -216,6 +216,8 @@ endif() set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld") +set(ENABLE_LINKER_RPATH_BY_DEFAULT OFF CACHE BOOL "pass -rpath to the linker by default") + set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "enable x86 relax relocations by default") diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 27ed69e21562bf..a7f185d88c68cc 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -69,6 +69,9 @@ /* pass --build-id to ld */ #cmakedefine ENABLE_LINKER_BUILD_ID +/* pass -rpath to the linker by default */ +#cmakedefine ENABLE_LINKER_RPATH_BY_DEFAULT + /* enable x86 relax relocations by default */ #cmakedefine01 ENABLE_X86_RELAX_RELOCATIONS diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index ca06fb115dfa11..e62f59f56241cd 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1183,7 +1183,11 @@ void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC, void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args, ArgStringList &CmdArgs) { if (!Args.hasFlag(options::OPT_frtlib_add_rpath, +#ifdef ENABLE_LINKER_RPATH_BY_DEFAULT +options::OPT_fno_rtlib_add_rpath, true)) +#else options::OPT_fno_rtlib_add_rpath, false)) +#endif return; SmallVector CandidateRPaths(TC.getArchSpecificLibPaths()); diff --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt index 5369dc92f69e8a..e439e973ce6f81 100644 --- a/clang/test/CMakeLists.txt +++ b/clang/test/CMakeLists.txt @@ -11,6 +11,7 @@ llvm_canonicalize_cmake_booleans( CLANG_SPAWN_CC1 CLANG_ENABLE_CIR ENABLE_BACKTRACES + ENABLE_LINKER_RPATH_BY_DEFAULT LLVM_BUILD_EXAMPLES LLVM_BYE_LINK_INTO_TOOLS LLVM_ENABLE_PLUGINS diff --git a/clang/test/Driver/arch-specific-libdir-rpath-by-default.c b/clang/test/Driver/arch-specific-libdir-rpath-by-default.c new file mode 100644 index 00..20c3f2fa560905 --- /dev/null +++ b/clang/test/Driver/arch-specific-libdir-rpath-by-default.c @@ -0,0 +1,105 @@ +// Test that the driver adds an arch-specific subdirectory in +// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' +// +// REQUIRES: enable_rpath_by_default +// +// Test the default behavior when neither -frtlib-add-rpath nor +// -fno-rtlib-add-rpath is specified, which is to add -rpath +// RUN: %clang %s -### --target=x86_64-linux \ +// RUN: -fsanitize=address -shared-libasan \ +// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir 2>&1 \ +// RUN: | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s +// +// Test that -rpath is not added under -fno-rtlib-add-rpath even if other +// conditions are met. +// RUN: %clang %s -### --target=x86_64-linu
[clang] [flang] [clang][Driver] Add RPATH by default (PR #115286)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/115286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add the ability to specify that RPATH should be added by default (PR #115163)
pawosm-arm wrote: > For what it’s worth, as a user I’m always surprised that this _isn’t_ the > default. It’s quite annoying to eg have to set LD_LIBRARY_PATH when using > openmp with clang when they’re built alongside each other. Is there a > specific reason this isn’t the default or is it just historical? OK, so let's try to make it default behavior, see #115286 https://github.com/llvm/llvm-project/pull/115163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add RPATH by default (PR #115286)
https://github.com/pawosm-arm created https://github.com/llvm/llvm-project/pull/115286 The `-frtlib-add-rpath` is very convenient when linking against various runtimes (OpenMP, Fortran, sanitizers), so much so we could argue that this should be a default behavior. This patch makes adding RPATH the default behavior that could be prevented by explicitly using the `-fno-rtlib-add-rpath` flag. >From f50c075b706b691bedab835ad66330741e08fe76 Mon Sep 17 00:00:00 2001 From: Paul Osmialowski Date: Thu, 7 Nov 2024 09:33:54 + Subject: [PATCH] [clang][Driver] Add RPATH by default The `-frtlib-add-rpath` is very convenient when linking against various runtimes (OpenMP, Fortran, sanitizers), so much so we could argue that this should be a default behavior. This patch makes adding RPATH the default behavior that could be prevented by explicitly using the `-fno-rtlib-add-rpath` flag. --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 2 +- clang/test/Driver/arch-specific-libdir-rpath.c | 4 ++-- flang/test/Driver/arch-specific-libdir-rpath.f95 | 5 ++--- flang/test/Driver/linker-flags.f90 | 3 ++- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index ca06fb115dfa11..6983ab4567f9a6 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1183,7 +1183,7 @@ void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC, void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args, ArgStringList &CmdArgs) { if (!Args.hasFlag(options::OPT_frtlib_add_rpath, -options::OPT_fno_rtlib_add_rpath, false)) +options::OPT_fno_rtlib_add_rpath, true)) return; SmallVector CandidateRPaths(TC.getArchSpecificLibPaths()); diff --git a/clang/test/Driver/arch-specific-libdir-rpath.c b/clang/test/Driver/arch-specific-libdir-rpath.c index 1e6bbbc5929ac2..3277da9287233f 100644 --- a/clang/test/Driver/arch-specific-libdir-rpath.c +++ b/clang/test/Driver/arch-specific-libdir-rpath.c @@ -2,11 +2,11 @@ // {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' // // Test the default behavior when neither -frtlib-add-rpath nor -// -fno-rtlib-add-rpath is specified, which is to skip -rpath +// -fno-rtlib-add-rpath is specified, which is to add -rpath // RUN: %clang %s -### --target=x86_64-linux \ // RUN: -fsanitize=address -shared-libasan \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir 2>&1 \ -// RUN: | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s +// RUN: | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s // // Test that -rpath is not added under -fno-rtlib-add-rpath even if other // conditions are met. diff --git a/flang/test/Driver/arch-specific-libdir-rpath.f95 b/flang/test/Driver/arch-specific-libdir-rpath.f95 index 23fb52abfbd574..5ce57dab96c260 100644 --- a/flang/test/Driver/arch-specific-libdir-rpath.f95 +++ b/flang/test/Driver/arch-specific-libdir-rpath.f95 @@ -1,12 +1,11 @@ -! REQUIRES: x86-registered-target ! Test that the driver adds an arch-specific subdirectory in ! {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' ! ! Test the default behavior when neither -frtlib-add-rpath nor -! -fno-rtlib-add-rpath is specified, which is to skip -rpath +! -fno-rtlib-add-rpath is specified, which is to add -rpath ! RUN: %flang %s -### --target=x86_64-linux \ ! RUN: -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir 2>&1 \ -! RUN: | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s +! RUN: | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s ! ! Test that -rpath is not added under -fno-rtlib-add-rpath ! RUN: %flang %s -### --target=x86_64-linux \ diff --git a/flang/test/Driver/linker-flags.f90 b/flang/test/Driver/linker-flags.f90 index ac9500d7c45cec..c4ee26c5dacab8 100644 --- a/flang/test/Driver/linker-flags.f90 +++ b/flang/test/Driver/linker-flags.f90 @@ -33,7 +33,8 @@ ! SOLARIS-F128NONE-NOT: FortranFloat128Math ! UNIX-F128LIBQUADMATH-SAME: "-lFortranFloat128Math" "--as-needed" "-lquadmath" "--no-as-needed" ! SOLARIS-F128LIBQUADMATH-SAME: "-lFortranFloat128Math" "-z" "ignore" "-lquadmath" "-z" "record" -! UNIX-SAME: "-lFortranRuntime" "-lFortranDecimal" "-lm" +! UNIX-SAME: "-lFortranRuntime" "-lFortranDecimal" +! UNIX-SAME: "-lm" ! COMPILER-RT: "{{.*}}{{\\|/}}libclang_rt.builtins.a" ! DARWIN-LABEL: "{{.*}}ld{{(\.exe)?}}" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add RPATH by default (PR #115286)
pawosm-arm wrote: @paulwalker-arm Can you share your rationale here? https://github.com/llvm/llvm-project/pull/115286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add the ability to specify that RPATH should be added by default (PR #115163)
@@ -0,0 +1,34 @@ +! REQUIRES: x86-registered-target pawosm-arm wrote: removed those requirements https://github.com/llvm/llvm-project/pull/115163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
pawosm-arm wrote: As I prepared a piece of code which makes the `-l` flags being treated properly when read from a config file (so it won't result in unresolved symbols e.g. when used with `-static`), I realized that going the config file way still creates a problem: with `-fveclib=ArmPL -lamath` put into a config file, it will always be linked with `libamath`, even if the user decides to use `-fveclib=none` to override this. And situation of this kind can only be solved properly by the patch discussed in this PR. And there's another rationale behind it I can think of: using `-fopenmp` does not require the user to add `-lomp` to the command line, it just happens automatically. The sheer logic demands that `-fveclib=ArmPL` should result in the same treatment. There is no other provider for ArmPL's veclib functions these days than libamath. https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add the ability to specify that RPATH should be added by default (PR #115163)
pawosm-arm wrote: No, config files today ARE NOT sufficient to do that. There is no way to attach a role to the flags in clang's config file, all flags are being treated the same. So putting -lamath in there will put it before object files that would make use of it, it will result in unresolved symbols. https://github.com/llvm/llvm-project/pull/115163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/116432 >From 5614595f48dc2a6b4087c992c108874ca6c787ee Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Fri, 15 Nov 2024 15:22:21 + Subject: [PATCH] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath Using `-fveclib=ArmPL` without libamath likely effects in the link-time errors. --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 11 +++ clang/lib/Driver/ToolChains/MSVC.cpp | 8 clang/test/Driver/fveclib.c| 14 ++ flang/test/Driver/fveclib.f90 | 14 ++ 4 files changed, 47 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index cbba4289eb9450..94143d68d39649 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -490,6 +490,17 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { + const llvm::Triple &Triple = TC.getTriple(); + StringRef V = A->getValue(); + if ((V == "ArmPL") && (Triple.isOSLinux() || Triple.isOSDarwin())) { +CmdArgs.push_back(Args.MakeArgString("-lamath")); +CmdArgs.push_back(Args.MakeArgString("-lm")); +addArchSpecificRPath(TC, Args, CmdArgs); + } +} + } } void tools::addLinkerCompressDebugSectionsOption( diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp index 80799d1e715f07..dc5d8d46585ef3 100644 --- a/clang/lib/Driver/ToolChains/MSVC.cpp +++ b/clang/lib/Driver/ToolChains/MSVC.cpp @@ -84,6 +84,14 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA, else if (TC.getTriple().isWindowsArm64EC()) CmdArgs.push_back("-machine:arm64ec"); + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { + StringRef V = A->getValue(); + if (V == "ArmPL") +CmdArgs.push_back(Args.MakeArgString("--dependent-lib=amath")); +} + } + if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles) && !C.getDriver().IsCLMode() && !C.getDriver().IsFlangMode()) { CmdArgs.push_back("-defaultlib:libcmt"); diff --git a/clang/test/Driver/fveclib.c b/clang/test/Driver/fveclib.c index 8b233b0023398f..31d44ee06e1594 100644 --- a/clang/test/Driver/fveclib.c +++ b/clang/test/Driver/fveclib.c @@ -102,3 +102,17 @@ /* Verify no warning when math-errno is re-enabled for a different veclib (that does not imply -fno-math-errno). */ // RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL -fmath-errno -fveclib=LIBMVEC %s 2>&1 | FileCheck --check-prefix=CHECK-REPEAT-VECLIB %s // CHECK-REPEAT-VECLIB-NOT: math errno enabled + +/* Verify that vectorized routines library is being linked in. */ +// RUN: %clang -### --target=aarch64-pc-windows-msvc -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL-MSVC %s +// RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +// RUN: %clang -### --target=arm64-apple-darwin -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +// CHECK-LINKING-ARMPL: "-lamath" +// CHECK-LINKING-ARMPL-SAME: "-lm" +// CHECK-LINKING-ARMPL-MSVC: "--dependent-lib=amath" + +/* Verify that the RPATH is being set when needed. */ +// RUN: %clang -### --target=aarch64-linux-gnu -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +// CHECK-RPATH-ARMPL: "-lamath" +// CHECK-RPATH-ARMPL-SAME: "-lm" +// CHECK-RPATH-ARMPL-SAME: "-rpath" diff --git a/flang/test/Driver/fveclib.f90 b/flang/test/Driver/fveclib.f90 index 14c59b0616f828..68fff31332ff8a 100644 --- a/flang/test/Driver/fveclib.f90 +++ b/flang/test/Driver/fveclib.f90 @@ -30,3 +30,17 @@ ! TODO: if we add support for -nostdlib or -nodefaultlibs we need to test that ! these prevent "-framework Accelerate" being added on Darwin + +! RUN: %flang -### --target=aarch64-pc-windows-msvc -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL-MSVC %s +! RUN: %flang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +! RUN: %flang -### --target=arm64-apple-darwin -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +! CHECK-LINKING-ARMPL: "-lamath" +! CHECK-LINKING-ARMPL-SAME: "-lm" +! CHECK-LINKING-ARMPL-MSVC: "--dependent-lib=amath" + +! RUN: %flang -### --target=aarch64-linux-gnu -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +! CHECK-RP
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
pawosm-arm wrote: > We probably have to provide a link to some documentation for libamath. May be > something like the following one. > https://developer.arm.com/documentation/102574/2410/Optimized-math-routines---libamath I'm always anxious to provide a link that contains numeric ID (like the one, `102574/2410`) as it suggest a dynamically generated reference that may be outlived by the lifespan of the change we're introducing in this commit... https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
pawosm-arm wrote: I've extended it with Windows and Darwin. https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add RPATH by default (PR #115286)
https://github.com/pawosm-arm closed https://github.com/llvm/llvm-project/pull/115286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/116432 >From 392192f186be03186cd53e2f94b56a21c5db27aa Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Fri, 15 Nov 2024 15:22:21 + Subject: [PATCH] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath Using `-fveclib=ArmPL` without `-lamath` likely effects in the link-time errors. --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 ++ clang/test/Driver/fveclib.c| 11 +++ flang/test/Driver/fveclib.f90 | 11 +++ 3 files changed, 32 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index cbba4289eb9450..7b5c40ecafa380 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -490,6 +490,16 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { + StringRef V = A->getValue(); + if ((V == "ArmPL") && TC.getTriple().isOSLinux()) { +CmdArgs.push_back(Args.MakeArgString("-lamath")); +CmdArgs.push_back(Args.MakeArgString("-lm")); +addArchSpecificRPath(TC, Args, CmdArgs); + } +} + } } void tools::addLinkerCompressDebugSectionsOption( diff --git a/clang/test/Driver/fveclib.c b/clang/test/Driver/fveclib.c index 8b233b0023398f..3c1defda2270c3 100644 --- a/clang/test/Driver/fveclib.c +++ b/clang/test/Driver/fveclib.c @@ -102,3 +102,14 @@ /* Verify no warning when math-errno is re-enabled for a different veclib (that does not imply -fno-math-errno). */ // RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL -fmath-errno -fveclib=LIBMVEC %s 2>&1 | FileCheck --check-prefix=CHECK-REPEAT-VECLIB %s // CHECK-REPEAT-VECLIB-NOT: math errno enabled + +/* Verify that vectorized routines library is being linked in. */ +// RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +// CHECK-LINKING-ARMPL: "-lamath" +// CHECK-LINKING-ARMPL-SAME: "-lm" + +/* Verify that the RPATH is being set when needed. */ +// RUN: %clang -### --target=aarch64-linux-gnu -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +// CHECK-RPATH-ARMPL: "-lamath" +// CHECK-RPATH-ARMPL-SAME: "-lm" +// CHECK-RPATH-ARMPL-SAME: "-rpath" diff --git a/flang/test/Driver/fveclib.f90 b/flang/test/Driver/fveclib.f90 index 14c59b0616f828..fc3df02c015165 100644 --- a/flang/test/Driver/fveclib.f90 +++ b/flang/test/Driver/fveclib.f90 @@ -30,3 +30,14 @@ ! TODO: if we add support for -nostdlib or -nodefaultlibs we need to test that ! these prevent "-framework Accelerate" being added on Darwin + +! RUN: %flang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +! CHECK-LINKING-ARMPL: "-lamath" +! CHECK-LINKING-ARMPL-SAME: "-lm" + +! RUN: %flang -### --target=aarch64-linux-gnu -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +! CHECK-RPATH-ARMPL: "-lamath" +! CHECK-RPATH-ARMPL-SAME: "-lm" +! We need to see "-rpath" at least twice, one for veclib, one for the Fortran runtime +! CHECK-RPATH-ARMPL-SAME: "-rpath" +! CHECK-RPATH-ARMPL-SAME: "-rpath" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][Driver] Add the ability to specify that RPATH should be added by default (PR #115163)
https://github.com/pawosm-arm closed https://github.com/llvm/llvm-project/pull/115163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
@@ -490,6 +490,16 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { + StringRef V = A->getValue(); + if (V == "ArmPL") { +CmdArgs.push_back(Args.MakeArgString("-lamath")); +CmdArgs.push_back(Args.MakeArgString("-lm")); pawosm-arm wrote: > Flang and ArmPL both support Windows. I don't see why Windows should get > second-class support. I would prefer to see this PR extended to also include > any flags required on Windows. This code is common for flang and clang. @DavidTruby as an Windows expert, what flags would you recommend for Windows here? https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
@@ -490,6 +490,16 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { + StringRef V = A->getValue(); + if (V == "ArmPL") { +CmdArgs.push_back(Args.MakeArgString("-lamath")); +CmdArgs.push_back(Args.MakeArgString("-lm")); pawosm-arm wrote: > do these flags and library names work on Windows? I've reduced it to Linux only. https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -61,3 +61,24 @@ ! CHECK-TWO-CONFIGS-NEXT: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg ! CHECK-TWO-CONFIGS: -ffp-contract=fast ! CHECK-TWO-CONFIGS: -O3 + +!--- The -l flags should be moved to the end of input list and appear only when linking. +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING pawosm-arm wrote: > Can you add a run line for Windows (`--target=x86_64-pc-windows-msvc` or > similar) with e.g. -check-prefix CHECK-WINDOWS and something along the lines > of: > > ``` > CHECK-WINDOWS: -ffast-math > CHECK-WINDOWS-SAME: "{{.*}}-{{.*}}.o" "m.lib" "happy.lib" > ``` > > And the same for the NOLINKING ones? We will want to make sure this change is > working for Windows too so I don't really want it to land before we have > Windows tests as well. I can write them locally (since I have a Windows > build) and attach a diff if that would help. Does it mean Bstatic/Bdynamic is always dropped by the frontend driver when it emits the linker's command line? https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/117573 >From d5a424405ef75b566167ce1686912a3d83949c48 Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Mon, 25 Nov 2024 14:46:55 + Subject: [PATCH] [clang][driver] Special care for -l and -Wl, flags in config files Currently, if a -l (or -Wl,) flag is added into a config file (e.g. clang.cfg), it is situated before any object file in the effective command line. If the library requested by given -l flag is static, its symbols will not be made visible to any of the object files provided by the user. Also, the presence of any of the linker flags in a config file confuses the driver whenever the user invokes clang without any parameters. This patch solves both of those problems, by moving the -l and -Wl, flags specified in a config file to the end of the input list, and by discarding them when there are no other inputs provided by the user, or the last phase is not the linking phase. --- clang/include/clang/Driver/Driver.h | 3 +++ clang/lib/Driver/Driver.cpp | 33 +++ clang/test/Driver/Inputs/config-l.cfg | 1 + clang/test/Driver/config-file.c | 32 ++ flang/test/Driver/Inputs/config-l.cfg | 1 + flang/test/Driver/config-file.f90 | 32 ++ 6 files changed, 102 insertions(+) create mode 100644 clang/test/Driver/Inputs/config-l.cfg create mode 100644 flang/test/Driver/Inputs/config-l.cfg diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 9177d56718ee77..b21606b6f54b77 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -297,6 +297,9 @@ class Driver { /// Object that stores strings read from configuration file. llvm::StringSaver Saver; + /// Linker inputs originated from configuration file. + std::unique_ptr CfgLinkerInputs; + /// Arguments originated from configuration file. std::unique_ptr CfgOptions; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index ad14b5c9b6dc80..c606cd332e47d4 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1062,6 +1062,16 @@ bool Driver::readConfigFile(StringRef FileName, for (Arg *A : *NewOptions) A->claim(); + std::unique_ptr NewLinkerIns = std::make_unique(); + for (Arg *A : NewOptions->filtered(options::OPT_Wl_COMMA, options::OPT_l)) { +const Arg *BaseArg = &A->getBaseArg(); +if (BaseArg == A) + BaseArg = nullptr; +appendOneArg(*NewLinkerIns, A, BaseArg); + } + NewOptions->eraseArg(options::OPT_Wl_COMMA); + NewOptions->eraseArg(options::OPT_l); + if (!CfgOptions) CfgOptions = std::move(NewOptions); else { @@ -1073,6 +1083,19 @@ bool Driver::readConfigFile(StringRef FileName, appendOneArg(*CfgOptions, Opt, BaseArg); } } + + if (!CfgLinkerInputs) +CfgLinkerInputs = std::move(NewLinkerIns); + else { +// If this is a subsequent config file, append options to the previous one. +for (auto *Opt : *NewLinkerIns) { + const Arg *BaseArg = &Opt->getBaseArg(); + if (BaseArg == Opt) +BaseArg = nullptr; + appendOneArg(*CfgLinkerInputs, Opt, BaseArg); +} + } + ConfigFiles.push_back(std::string(CfgFileName)); return false; } @@ -1250,6 +1273,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { if (!ContainsError) ContainsError = loadConfigFiles(); bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr); + bool HasConfigLinkerIn = !ContainsError && (CfgLinkerInputs.get() != nullptr); // All arguments, from both config file and command line. InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions) @@ -1552,6 +1576,15 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { // Construct the list of inputs. InputList Inputs; BuildInputs(C->getDefaultToolChain(), *TranslatedArgs, Inputs); + if (HasConfigLinkerIn && Inputs.size()) { +Arg *FinalPhaseArg; +if (getFinalPhase(*TranslatedArgs, &FinalPhaseArg) == phases::Link) { + DerivedArgList TranslatedLinkerIns(*CfgLinkerInputs); + for (Arg *A : *CfgLinkerInputs) +TranslatedLinkerIns.append(A); + BuildInputs(C->getDefaultToolChain(), TranslatedLinkerIns, Inputs); +} + } // Populate the tool chains for the offloading devices, if any. CreateOffloadingDeviceToolChains(*C, Inputs); diff --git a/clang/test/Driver/Inputs/config-l.cfg b/clang/test/Driver/Inputs/config-l.cfg new file mode 100644 index 00..853b179bd2da85 --- /dev/null +++ b/clang/test/Driver/Inputs/config-l.cfg @@ -0,0 +1 @@ +-Wall -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic diff --git a/clang/test/Driver/config-file.c b/clang/test/Driver/config-file.c index 9df830ca4c7538..5d6e53f28e7c76 100644 --- a/clang/test/Driver/config-file.c +++ b/clang/test/Driver/config-file.c @@ -82,3 +82,35 @@ // CHECK-TWO-CONFIGS: -isysroot /
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -1250,6 +1273,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { if (!ContainsError) ContainsError = loadConfigFiles(); bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr); + bool HasConfigLinkerIn = !ContainsError && (CfgLinkerInputs.get() != nullptr); pawosm-arm wrote: conceptually this is the right place for checking it. ContainsError is a regular variable, who knows what will happen to it before I look into its value so many lines below. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -82,3 +82,24 @@ // CHECK-TWO-CONFIGS: -isysroot // CHECK-TWO-CONFIGS-SAME: /opt/data // CHECK-TWO-CONFIGS-SAME: -Wall + +//--- The -l flags should be moved to the end of input list and appear only when linking. +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING pawosm-arm wrote: Done https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -61,3 +61,24 @@ ! CHECK-TWO-CONFIGS-NEXT: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg ! CHECK-TWO-CONFIGS: -ffp-contract=fast ! CHECK-TWO-CONFIGS: -O3 + +!--- The -l flags should be moved to the end of input list and appear only when linking. +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING pawosm-arm wrote: Done https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
pawosm-arm wrote: > I took a look and I think it may be too awkward to do, as we'd want to run > e.g. readelf afterwards. But an example is > `lld/test/ELF/as-needed-not-in-regular.s`. The idea being: if > `-Wl,--as-needed` is in the config file, do we correctly prune an unnecessary > library from a built object, or is the order wrong? We can check that with > `llvm-readelf`. There are two problems here. First, following the readelf path goes too far into the linker competences, whatever is set for the linker, the linker is obliged to do, and the order of whatever is set for the linker is already tested by other `-Wl,` occurrences in the currently available test cases. Yet your request also exposes another problem, and I can see that further comments also mention it. With my patch, there is no way to affect the user's command line linker input parameters. So if a config file is a mean for turning clang's behavior into the gcc's behavior (AFAIR, gcc always emits `-Wl,--as-needed` at the beginning of the linker invocation command line, while clang doesn't do that), this patch rejects such possibility. Something more clever is needed, the positional idea seems the right one, I could see it like this (a config file example, notice the pipe `|` character): ``` -Wall -Wl,--as-needed | -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic ``` ...things after the pipe would be appended at the end of the parameters list and only when it is known that there will be the linking. But this would require a modification in the config file reading routine, the second list of config options would have to be created much earlier than my patch proposes. Back to the drawing board then. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/117573 >From bc66202680198f0e5d1289c62f79ca5799e6f2ac Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Mon, 25 Nov 2024 14:46:55 + Subject: [PATCH] [clang][driver] Special care for -l and -Wl, flags in config files Currently, if a -l (or -Wl,) flag is added into a config file (e.g. clang.cfg), it is situated before any object file in the effective command line. If the library requested by given -l flag is static, its symbols will not be made visible to any of the object files provided by the user. Also, the presence of any of the linker flags in a config file confuses the driver whenever the user invokes clang without any parameters. This patch attempts to solve both of the problems, by allowing a split of the arguments list into two parts. The head part of the list will be used as before, but the tail part will be appended after the command line flags provided by the user and only when it is known that the linking should occur. The ^-prefixed arguments will be added to the tail part. --- clang/include/clang/Driver/Driver.h | 3 ++ clang/lib/Driver/Driver.cpp | 51 ++- clang/test/Driver/Inputs/config-l.cfg | 3 ++ clang/test/Driver/config-file.c | 26 ++ flang/test/Driver/Inputs/config-l.cfg | 3 ++ flang/test/Driver/config-file.f90 | 26 ++ 6 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 clang/test/Driver/Inputs/config-l.cfg create mode 100644 flang/test/Driver/Inputs/config-l.cfg diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 9177d56718ee77..b21606b6f54b77 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -297,6 +297,9 @@ class Driver { /// Object that stores strings read from configuration file. llvm::StringSaver Saver; + /// Linker inputs originated from configuration file. + std::unique_ptr CfgLinkerInputs; + /// Arguments originated from configuration file. std::unique_ptr CfgOptions; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 7de8341b8d2d61..3265aca659681c 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1039,34 +1039,59 @@ bool Driver::readConfigFile(StringRef FileName, } // Try reading the given file. - SmallVector NewCfgArgs; - if (llvm::Error Err = ExpCtx.readConfigFile(FileName, NewCfgArgs)) { + SmallVector NewCfgFileArgs; + if (llvm::Error Err = ExpCtx.readConfigFile(FileName, NewCfgFileArgs)) { Diag(diag::err_drv_cannot_read_config_file) << FileName << toString(std::move(Err)); return true; } + // Populate head and tail lists. The tail list is used only when linking. + SmallVector NewCfgHeadArgs, NewCfgTailArgs; + for (const char *Opt : NewCfgFileArgs) { +// An ^-prefixed option should go to the tail list. +if (Opt[0] == '^' && Opt[1]) + NewCfgTailArgs.push_back(Opt + 1); +else + NewCfgHeadArgs.push_back(Opt); + } + // Read options from config file. llvm::SmallString<128> CfgFileName(FileName); llvm::sys::path::native(CfgFileName); - bool ContainErrors; - auto NewOptions = std::make_unique( - ParseArgStrings(NewCfgArgs, /*UseDriverMode=*/true, ContainErrors)); + bool ContainErrors = false; + auto NewHeadOptions = std::make_unique( + ParseArgStrings(NewCfgHeadArgs, /*UseDriverMode=*/true, ContainErrors)); + if (ContainErrors) +return true; + auto NewTailOptions = std::make_unique( + ParseArgStrings(NewCfgTailArgs, /*UseDriverMode=*/true, ContainErrors)); if (ContainErrors) return true; // Claim all arguments that come from a configuration file so that the driver // does not warn on any that is unused. - for (Arg *A : *NewOptions) + for (Arg *A : *NewHeadOptions) +A->claim(); + for (Arg *A : *NewTailOptions) A->claim(); if (!CfgOptions) -CfgOptions = std::move(NewOptions); +CfgOptions = std::move(NewHeadOptions); else { // If this is a subsequent config file, append options to the previous one. -for (auto *Opt : *NewOptions) +for (auto *Opt : *NewHeadOptions) appendOneArg(*CfgOptions, Opt); } + + if (!CfgLinkerInputs) +CfgLinkerInputs = std::move(NewTailOptions); + else { +// If this is a subsequent config file, append options to the previous one. +for (auto *Opt : *NewTailOptions) + appendOneArg(*CfgLinkerInputs, Opt); + } + ConfigFiles.push_back(std::string(CfgFileName)); return false; } @@ -1244,6 +1269,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { if (!ContainsError) ContainsError = loadConfigFiles(); bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr); + bool HasConfigLinkerIn = !ContainsError && CfgLinkerInputs; // All arguments, from both config file and command line. InputArgList Args
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -0,0 +1 @@ +-ffast-math -Wl,--as-needed | -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic pawosm-arm wrote: Since for ` llvm::cl::ExpansionContext::readConfigFile()` a newline is just a separator, this can't be achieved by doing an enhancement in a code very distant from here. Seems now the 'pipe' idea can be buried and we're back to square one. I need to invent something different. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -0,0 +1 @@ +-ffast-math -Wl,--as-needed | -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic pawosm-arm wrote: We could prefix those linker flags with some character, e.g. '@', it would work like that: ``` -ffast-math @-lm -Wl,--as-needed @-Wl,-Bstatic -lhappy @-Wl,-Bdynamic ``` https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -0,0 +1 @@ +-ffast-math -Wl,--as-needed | -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic pawosm-arm wrote: mid-air collision. yeah, either '$' or '@' would do :) https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -0,0 +1 @@ +-ffast-math -Wl,--as-needed | -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic pawosm-arm wrote: Seems neither '$' nor '@' is the best choice, '$' looks like shell variable expansion, so I'll do `^`, e.g., '^-Wl,-Bstatic`. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -0,0 +1 @@ +-ffast-math -Wl,--as-needed | -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic pawosm-arm wrote: Done. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/116432 >From 0906f315571cfa093889077c5ec155cae3d174b6 Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Fri, 15 Nov 2024 15:22:21 + Subject: [PATCH] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath Using `-fveclib=ArmPL` without `-lamath` likely effects in the link-time errors. --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 ++ clang/test/Driver/fveclib.c| 11 +++ flang/test/Driver/fveclib.f90 | 11 +++ 3 files changed, 32 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index cbba4289eb9450..913797dec123fc 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -490,6 +490,16 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { + StringRef V = A->getValue(); + if (V == "ArmPL") { +CmdArgs.push_back(Args.MakeArgString("-lamath")); +CmdArgs.push_back(Args.MakeArgString("-lm")); +addArchSpecificRPath(TC, Args, CmdArgs); + } +} + } } void tools::addLinkerCompressDebugSectionsOption( diff --git a/clang/test/Driver/fveclib.c b/clang/test/Driver/fveclib.c index 8b233b0023398f..738c27fafb0524 100644 --- a/clang/test/Driver/fveclib.c +++ b/clang/test/Driver/fveclib.c @@ -102,3 +102,14 @@ /* Verify no warning when math-errno is re-enabled for a different veclib (that does not imply -fno-math-errno). */ // RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL -fmath-errno -fveclib=LIBMVEC %s 2>&1 | FileCheck --check-prefix=CHECK-REPEAT-VECLIB %s // CHECK-REPEAT-VECLIB-NOT: math errno enabled + +/* Verify that vectorized routines library is being linked in. */ +// RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +// CHECK-LINKING-ARMPL: "-lamath" +// CHECK-LINKING-ARMPL-SAME: "-lm" + +/* Verify that the RPATH is being set when needed. */ +// RUN: %clang -### --target=aarch64-linux-gnu -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +// CHECK-RPATH-ARMPL: "-lamath" +// CHECK-RPATH-ARMPL-SAME: "-lm" +// CHECK-RPATH-ARMPL-SAME: "-rpath" diff --git a/flang/test/Driver/fveclib.f90 b/flang/test/Driver/fveclib.f90 index 14c59b0616f828..60a711a0816d3d 100644 --- a/flang/test/Driver/fveclib.f90 +++ b/flang/test/Driver/fveclib.f90 @@ -30,3 +30,14 @@ ! TODO: if we add support for -nostdlib or -nodefaultlibs we need to test that ! these prevent "-framework Accelerate" being added on Darwin + +! RUN: %flang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +! CHECK-LINKING-ARMPL: "-lamath" +! CHECK-LINKING-ARMPL-SAME: "-lm" + +! RUN: %flang -### --target=aarch64-linux-gnu -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +! CHECK-RPATH-ARMPL: "-lamath" +! CHECK-RPATH-ARMPL-SAME: "-lm" +! We need to see "-rpath" at least twice, one for veclib, one for the Fortran runtime +! CHECK-RPATH-ARMPL-SAME: "-rpath" +! CHECK-RPATH-ARMPL-SAME: "-rpath" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
pawosm-arm wrote: > Typo in PR title `flang` -> `flag` Corrected both the title and the commit message. Thanks for spotting this! https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flang is in use, always link against libamath (PR #116432)
https://github.com/pawosm-arm created https://github.com/llvm/llvm-project/pull/116432 Using `-fveclib=ArmPL` without `-lamath` likely effects in the link-time errors. >From 991304ba11c9852964a0f0bdcde5277d47e29c7c Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Fri, 15 Nov 2024 15:22:21 + Subject: [PATCH] [clang][driver] When -fveclib=ArmPL flang is in use, always link against libamath Using `-fveclib=ArmPL` without `-lamath` likely effects in the link-time errors. --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 ++ clang/test/Driver/fveclib.c| 11 +++ flang/test/Driver/fveclib.f90 | 11 +++ 3 files changed, 32 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index cbba4289eb9450..913797dec123fc 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -490,6 +490,16 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { + StringRef V = A->getValue(); + if (V == "ArmPL") { +CmdArgs.push_back(Args.MakeArgString("-lamath")); +CmdArgs.push_back(Args.MakeArgString("-lm")); +addArchSpecificRPath(TC, Args, CmdArgs); + } +} + } } void tools::addLinkerCompressDebugSectionsOption( diff --git a/clang/test/Driver/fveclib.c b/clang/test/Driver/fveclib.c index 8b233b0023398f..738c27fafb0524 100644 --- a/clang/test/Driver/fveclib.c +++ b/clang/test/Driver/fveclib.c @@ -102,3 +102,14 @@ /* Verify no warning when math-errno is re-enabled for a different veclib (that does not imply -fno-math-errno). */ // RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL -fmath-errno -fveclib=LIBMVEC %s 2>&1 | FileCheck --check-prefix=CHECK-REPEAT-VECLIB %s // CHECK-REPEAT-VECLIB-NOT: math errno enabled + +/* Verify that vectorized routines library is being linked in. */ +// RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +// CHECK-LINKING-ARMPL: "-lamath" +// CHECK-LINKING-ARMPL-SAME: "-lm" + +/* Verify that the RPATH is being set when needed. */ +// RUN: %clang -### --target=aarch64-linux-gnu -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +// CHECK-RPATH-ARMPL: "-lamath" +// CHECK-RPATH-ARMPL-SAME: "-lm" +// CHECK-RPATH-ARMPL-SAME: "-rpath" diff --git a/flang/test/Driver/fveclib.f90 b/flang/test/Driver/fveclib.f90 index 14c59b0616f828..60a711a0816d3d 100644 --- a/flang/test/Driver/fveclib.f90 +++ b/flang/test/Driver/fveclib.f90 @@ -30,3 +30,14 @@ ! TODO: if we add support for -nostdlib or -nodefaultlibs we need to test that ! these prevent "-framework Accelerate" being added on Darwin + +! RUN: %flang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +! CHECK-LINKING-ARMPL: "-lamath" +! CHECK-LINKING-ARMPL-SAME: "-lm" + +! RUN: %flang -### --target=aarch64-linux-gnu -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +! CHECK-RPATH-ARMPL: "-lamath" +! CHECK-RPATH-ARMPL-SAME: "-lm" +! We need to see "-rpath" at least twice, one for veclib, one for the Fortran runtime +! CHECK-RPATH-ARMPL-SAME: "-rpath" +! CHECK-RPATH-ARMPL-SAME: "-rpath" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/116432 >From 865831ba2fb4778e8a1c25402b1cff70abd44d6b Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Fri, 15 Nov 2024 15:22:21 + Subject: [PATCH] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath Using `-fveclib=ArmPL` without `-lamath` likely effects in the link-time errors. --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 ++ clang/test/Driver/fveclib.c| 11 +++ flang/test/Driver/fveclib.f90 | 11 +++ 3 files changed, 32 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index cbba4289eb9450..913797dec123fc 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -490,6 +490,16 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { + StringRef V = A->getValue(); + if (V == "ArmPL") { +CmdArgs.push_back(Args.MakeArgString("-lamath")); +CmdArgs.push_back(Args.MakeArgString("-lm")); +addArchSpecificRPath(TC, Args, CmdArgs); + } +} + } } void tools::addLinkerCompressDebugSectionsOption( diff --git a/clang/test/Driver/fveclib.c b/clang/test/Driver/fveclib.c index 8b233b0023398f..3c1defda2270c3 100644 --- a/clang/test/Driver/fveclib.c +++ b/clang/test/Driver/fveclib.c @@ -102,3 +102,14 @@ /* Verify no warning when math-errno is re-enabled for a different veclib (that does not imply -fno-math-errno). */ // RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL -fmath-errno -fveclib=LIBMVEC %s 2>&1 | FileCheck --check-prefix=CHECK-REPEAT-VECLIB %s // CHECK-REPEAT-VECLIB-NOT: math errno enabled + +/* Verify that vectorized routines library is being linked in. */ +// RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +// CHECK-LINKING-ARMPL: "-lamath" +// CHECK-LINKING-ARMPL-SAME: "-lm" + +/* Verify that the RPATH is being set when needed. */ +// RUN: %clang -### --target=aarch64-linux-gnu -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +// CHECK-RPATH-ARMPL: "-lamath" +// CHECK-RPATH-ARMPL-SAME: "-lm" +// CHECK-RPATH-ARMPL-SAME: "-rpath" diff --git a/flang/test/Driver/fveclib.f90 b/flang/test/Driver/fveclib.f90 index 14c59b0616f828..fc3df02c015165 100644 --- a/flang/test/Driver/fveclib.f90 +++ b/flang/test/Driver/fveclib.f90 @@ -30,3 +30,14 @@ ! TODO: if we add support for -nostdlib or -nodefaultlibs we need to test that ! these prevent "-framework Accelerate" being added on Darwin + +! RUN: %flang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +! CHECK-LINKING-ARMPL: "-lamath" +! CHECK-LINKING-ARMPL-SAME: "-lm" + +! RUN: %flang -### --target=aarch64-linux-gnu -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +! CHECK-RPATH-ARMPL: "-lamath" +! CHECK-RPATH-ARMPL-SAME: "-lm" +! We need to see "-rpath" at least twice, one for veclib, one for the Fortran runtime +! CHECK-RPATH-ARMPL-SAME: "-rpath" +! CHECK-RPATH-ARMPL-SAME: "-rpath" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/116432 >From dde726aa72cd23a5a824846e6b65583d1274d18c Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Fri, 15 Nov 2024 15:22:21 + Subject: [PATCH] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath Using `-fveclib=ArmPL` without `-lamath` likely effects in the link-time errors. --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 ++ clang/test/Driver/fveclib.c| 11 +++ flang/test/Driver/fveclib.f90 | 11 +++ 3 files changed, 32 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index cbba4289eb9450..913797dec123fc 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -490,6 +490,16 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { + StringRef V = A->getValue(); + if (V == "ArmPL") { +CmdArgs.push_back(Args.MakeArgString("-lamath")); +CmdArgs.push_back(Args.MakeArgString("-lm")); +addArchSpecificRPath(TC, Args, CmdArgs); + } +} + } } void tools::addLinkerCompressDebugSectionsOption( diff --git a/clang/test/Driver/fveclib.c b/clang/test/Driver/fveclib.c index 8b233b0023398f..f148e83e11d918 100644 --- a/clang/test/Driver/fveclib.c +++ b/clang/test/Driver/fveclib.c @@ -102,3 +102,14 @@ /* Verify no warning when math-errno is re-enabled for a different veclib (that does not imply -fno-math-errno). */ // RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL -fmath-errno -fveclib=LIBMVEC %s 2>&1 | FileCheck --check-prefix=CHECK-REPEAT-VECLIB %s // CHECK-REPEAT-VECLIB-NOT: math errno enabled + +/* Verify that vectorized routines library is being linked in. */ +// RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +// CHECK-LINKING-ARMPL: "-lamath" +// CHECK-LINKING-ARMPL-SAME: "-lm" + +/* Verify that the RPATH is being set when needed. */ +// RUN: %clang -### --target=aarch64-linux-gnu -resource-dir=%S -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +// CHECK-RPATH-ARMPL: "-lamath" +// CHECK-RPATH-ARMPL-SAME: "-lm" +// CHECK-RPATH-ARMPL-SAME: "-rpath" diff --git a/flang/test/Driver/fveclib.f90 b/flang/test/Driver/fveclib.f90 index 14c59b0616f828..130fb266bf4ac9 100644 --- a/flang/test/Driver/fveclib.f90 +++ b/flang/test/Driver/fveclib.f90 @@ -30,3 +30,14 @@ ! TODO: if we add support for -nostdlib or -nodefaultlibs we need to test that ! these prevent "-framework Accelerate" being added on Darwin + +! RUN: %flang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL %s +! CHECK-LINKING-ARMPL: "-lamath" +! CHECK-LINKING-ARMPL-SAME: "-lm" + +! RUN: %flang -### --target=aarch64-linux-gnu -resource-dir=%S -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +! CHECK-RPATH-ARMPL: "-lamath" +! CHECK-RPATH-ARMPL-SAME: "-lm" +! We need to see "-rpath" at least twice, one for veclib, one for the Fortran runtime +! CHECK-RPATH-ARMPL-SAME: "-rpath" +! CHECK-RPATH-ARMPL-SAME: "-rpath" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm created https://github.com/llvm/llvm-project/pull/117573 Currently, if a -l flag is added into a config file (e.g. clang.cfg), it is situated before any object file in the effective command line. If the library requested by given -l flag is static, its symbols will not be made visible to any of the object files provided by the user. Also, the presence of any of the -l flags in a config file confuses the driver whenever the user invokes clang without any parameters. This patch solves both of those problems, by moving the -lm flags specified in a config file to the end of the input list, and by discarding them when there are no other inputs provided by the user, or the last phase is not the linking phase. >From 1c4cab58007a854a66b70dc47376328a8dc7ecf9 Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Mon, 25 Nov 2024 14:46:55 + Subject: [PATCH] [clang][driver] Special care for -l flags in config files Currently, if a -l flag is added into a config file (e.g. clang.cfg), it is situated before any object file in the effective command line. If the library requested by given -l flag is static, its symbols will not be made visible to any of the object files provided by the user. Also, the presence of any of the -l flags in a config file confuses the driver whenever the user invokes clang without any parameters. This patch solves both of those problems, by moving the -lm flags specified in a config file to the end of the input list, and by discarding them when there are no other inputs provided by the user, or the last phase is not the linking phase. --- clang/include/clang/Driver/Driver.h | 3 ++ clang/lib/Driver/Driver.cpp | 32 clang/test/Driver/Inputs/config-l-openmp.cfg | 1 + clang/test/Driver/Inputs/config-l.cfg| 1 + clang/test/Driver/config-file.c | 19 flang/test/Driver/Inputs/config-l-openmp.cfg | 1 + flang/test/Driver/Inputs/config-l.cfg| 1 + flang/test/Driver/config-file.f90| 19 8 files changed, 77 insertions(+) create mode 100644 clang/test/Driver/Inputs/config-l-openmp.cfg create mode 100644 clang/test/Driver/Inputs/config-l.cfg create mode 100644 flang/test/Driver/Inputs/config-l-openmp.cfg create mode 100644 flang/test/Driver/Inputs/config-l.cfg diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 9177d56718ee77..b21606b6f54b77 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -297,6 +297,9 @@ class Driver { /// Object that stores strings read from configuration file. llvm::StringSaver Saver; + /// Linker inputs originated from configuration file. + std::unique_ptr CfgLinkerInputs; + /// Arguments originated from configuration file. std::unique_ptr CfgOptions; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index ad14b5c9b6dc80..bfd440461ea601 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1062,6 +1062,15 @@ bool Driver::readConfigFile(StringRef FileName, for (Arg *A : *NewOptions) A->claim(); + std::unique_ptr NewLinkerIns = std::make_unique(); + for (Arg *A : NewOptions->filtered(options::OPT_l)) { +const Arg *BaseArg = &A->getBaseArg(); +if (BaseArg == A) + BaseArg = nullptr; +appendOneArg(*NewLinkerIns, A, BaseArg); + } + NewOptions->eraseArg(options::OPT_l); + if (!CfgOptions) CfgOptions = std::move(NewOptions); else { @@ -1073,6 +1082,19 @@ bool Driver::readConfigFile(StringRef FileName, appendOneArg(*CfgOptions, Opt, BaseArg); } } + + if (!CfgLinkerInputs) +CfgLinkerInputs = std::move(NewLinkerIns); + else { +// If this is a subsequent config file, append options to the previous one. +for (auto *Opt : *NewLinkerIns) { + const Arg *BaseArg = &Opt->getBaseArg(); + if (BaseArg == Opt) +BaseArg = nullptr; + appendOneArg(*CfgLinkerInputs, Opt, BaseArg); +} + } + ConfigFiles.push_back(std::string(CfgFileName)); return false; } @@ -1250,6 +1272,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { if (!ContainsError) ContainsError = loadConfigFiles(); bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr); + bool HasConfigLinkerIn = !ContainsError && (CfgLinkerInputs.get() != nullptr); // All arguments, from both config file and command line. InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions) @@ -1552,6 +1575,15 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { // Construct the list of inputs. InputList Inputs; BuildInputs(C->getDefaultToolChain(), *TranslatedArgs, Inputs); + if (HasConfigLinkerIn && Inputs.size()) { +Arg *FinalPhaseArg; +if (getFinalPhase(*TranslatedArgs, &FinalPhaseArg) == phases::Link) { + DerivedArgList TranslatedLinkerIns(*CfgLinkerInputs); +
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -61,3 +61,22 @@ ! CHECK-TWO-CONFIGS-NEXT: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg ! CHECK-TWO-CONFIGS: -ffp-contract=fast ! CHECK-TWO-CONFIGS: -O3 + +!--- The -l flags should be moved to the end of input list and appear only when linking. +! RUN: %flang --config %S/Inputs/config-l.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING +! RUN: %flang --config %S/Inputs/config-l-openmp.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST +! RUN: %flang --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! RUN: %flang --config %S/Inputs/config-l-openmp.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +! CHECK-LINKING: "-ffast-math" +! CHECK-LINKING: "/tmp/{{.*}}-{{.*}}.o" "-lm" "-lhappy" pawosm-arm wrote: yeah, full path isn't really needed there, a good regexp can solve this. Also I've added `--target=aarch64-unknown-linux-gnu` to all, to simplify the test cases. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
pawosm-arm wrote: > I think it makes sense to handle linker options differently so I'm in favour > of this change in principle. > > Am I right in thinking that if the config file puts things last, the `-l` > options provided by users will come before the config file ones, and unlike > other options that will lead to those libraries being chosen first? If so, I > think that's the correct way to do things anyway, so I prefer that to the > current approach of putting the ones from the config file first. It might be > considered a breaking change though. > > @mgorny's question has got me thinking and given me concerns; I think if the > user passes `-Wl,-Bstatic -lmystaticlib` and the config file is adding > `-lmydynamiclib` after that, things will fail, because the `-Bstatic` will > also apply to the lib in the config file. So we need to do something to > prevent situations like that. > > I've also added a few comments so that we can hopefully get this working for > Windows as well. That was actually easy to fix. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -0,0 +1 @@ +-ffast-math -lm -lhappy pawosm-arm wrote: yes https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/117573 >From 2acf432b48c275492d9c5d012e85da36750c9299 Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Mon, 25 Nov 2024 14:46:55 + Subject: [PATCH] [clang][driver] Special care for -l and -Wl, flags in config files Currently, if a -l (or -Wl,) flag is added into a config file (e.g. clang.cfg), it is situated before any object file in the effective command line. If the library requested by given -l flag is static, its symbols will not be made visible to any of the object files provided by the user. Also, the presence of any of the linker flags in a config file confuses the driver whenever the user invokes clang without any parameters. This patch solves both of those problems, by moving the -l and -Wl, flags specified in a config file to the end of the input list, and by discarding them when there are no other inputs provided by the user, or the last phase is not the linking phase. --- clang/include/clang/Driver/Driver.h | 3 +++ clang/lib/Driver/Driver.cpp | 33 +++ clang/test/Driver/Inputs/config-l.cfg | 1 + clang/test/Driver/config-file.c | 21 + flang/test/Driver/Inputs/config-l.cfg | 1 + flang/test/Driver/config-file.f90 | 21 + 6 files changed, 80 insertions(+) create mode 100644 clang/test/Driver/Inputs/config-l.cfg create mode 100644 flang/test/Driver/Inputs/config-l.cfg diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 9177d56718ee77..b21606b6f54b77 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -297,6 +297,9 @@ class Driver { /// Object that stores strings read from configuration file. llvm::StringSaver Saver; + /// Linker inputs originated from configuration file. + std::unique_ptr CfgLinkerInputs; + /// Arguments originated from configuration file. std::unique_ptr CfgOptions; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index ad14b5c9b6dc80..c606cd332e47d4 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1062,6 +1062,16 @@ bool Driver::readConfigFile(StringRef FileName, for (Arg *A : *NewOptions) A->claim(); + std::unique_ptr NewLinkerIns = std::make_unique(); + for (Arg *A : NewOptions->filtered(options::OPT_Wl_COMMA, options::OPT_l)) { +const Arg *BaseArg = &A->getBaseArg(); +if (BaseArg == A) + BaseArg = nullptr; +appendOneArg(*NewLinkerIns, A, BaseArg); + } + NewOptions->eraseArg(options::OPT_Wl_COMMA); + NewOptions->eraseArg(options::OPT_l); + if (!CfgOptions) CfgOptions = std::move(NewOptions); else { @@ -1073,6 +1083,19 @@ bool Driver::readConfigFile(StringRef FileName, appendOneArg(*CfgOptions, Opt, BaseArg); } } + + if (!CfgLinkerInputs) +CfgLinkerInputs = std::move(NewLinkerIns); + else { +// If this is a subsequent config file, append options to the previous one. +for (auto *Opt : *NewLinkerIns) { + const Arg *BaseArg = &Opt->getBaseArg(); + if (BaseArg == Opt) +BaseArg = nullptr; + appendOneArg(*CfgLinkerInputs, Opt, BaseArg); +} + } + ConfigFiles.push_back(std::string(CfgFileName)); return false; } @@ -1250,6 +1273,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { if (!ContainsError) ContainsError = loadConfigFiles(); bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr); + bool HasConfigLinkerIn = !ContainsError && (CfgLinkerInputs.get() != nullptr); // All arguments, from both config file and command line. InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions) @@ -1552,6 +1576,15 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { // Construct the list of inputs. InputList Inputs; BuildInputs(C->getDefaultToolChain(), *TranslatedArgs, Inputs); + if (HasConfigLinkerIn && Inputs.size()) { +Arg *FinalPhaseArg; +if (getFinalPhase(*TranslatedArgs, &FinalPhaseArg) == phases::Link) { + DerivedArgList TranslatedLinkerIns(*CfgLinkerInputs); + for (Arg *A : *CfgLinkerInputs) +TranslatedLinkerIns.append(A); + BuildInputs(C->getDefaultToolChain(), TranslatedLinkerIns, Inputs); +} + } // Populate the tool chains for the offloading devices, if any. CreateOffloadingDeviceToolChains(*C, Inputs); diff --git a/clang/test/Driver/Inputs/config-l.cfg b/clang/test/Driver/Inputs/config-l.cfg new file mode 100644 index 00..853b179bd2da85 --- /dev/null +++ b/clang/test/Driver/Inputs/config-l.cfg @@ -0,0 +1 @@ +-Wall -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic diff --git a/clang/test/Driver/config-file.c b/clang/test/Driver/config-file.c index 9df830ca4c7538..21d1c23ee34369 100644 --- a/clang/test/Driver/config-file.c +++ b/clang/test/Driver/config-file.c @@ -82,3 +82,24 @@ // CHECK-TWO-CONFIGS: -isysroot // CHECK-TWO-CONFIGS
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/117573 >From e5769df6180f6b89ad2c494e74d3e4dc9d88dec8 Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Mon, 25 Nov 2024 14:46:55 + Subject: [PATCH] [clang][driver] Special care for -l and -Wl, flags in config files Currently, if a -l (or -Wl,) flag is added into a config file (e.g. clang.cfg), it is situated before any object file in the effective command line. If the library requested by given -l flag is static, its symbols will not be made visible to any of the object files provided by the user. Also, the presence of any of the linker flags in a config file confuses the driver whenever the user invokes clang without any parameters. This patch solves both of those problems, by moving the -l and -Wl, flags specified in a config file to the end of the input list, and by discarding them when there are no other inputs provided by the user, or the last phase is not the linking phase. --- clang/include/clang/Driver/Driver.h | 3 +++ clang/lib/Driver/Driver.cpp | 36 +++ clang/test/Driver/Inputs/config-l.cfg | 1 + clang/test/Driver/config-file.c | 26 +++ flang/test/Driver/Inputs/config-l.cfg | 1 + flang/test/Driver/config-file.f90 | 26 +++ 6 files changed, 93 insertions(+) create mode 100644 clang/test/Driver/Inputs/config-l.cfg create mode 100644 flang/test/Driver/Inputs/config-l.cfg diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 9177d56718ee77..b21606b6f54b77 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -297,6 +297,9 @@ class Driver { /// Object that stores strings read from configuration file. llvm::StringSaver Saver; + /// Linker inputs originated from configuration file. + std::unique_ptr CfgLinkerInputs; + /// Arguments originated from configuration file. std::unique_ptr CfgOptions; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index ad14b5c9b6dc80..007b25f31d5aee 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1062,6 +1062,19 @@ bool Driver::readConfigFile(StringRef FileName, for (Arg *A : *NewOptions) A->claim(); + // Filter out all -l and -Wl, options, put them into a separate list and erase + // from the original list of configuration file options. These will be used + // only when linking and appended after all of the command line options. + auto NewLinkerIns = std::make_unique(); + for (Arg *A : NewOptions->filtered(options::OPT_Wl_COMMA, options::OPT_l)) { +const Arg *BaseArg = &A->getBaseArg(); +if (BaseArg == A) + BaseArg = nullptr; +appendOneArg(*NewLinkerIns, A, BaseArg); + } + NewOptions->eraseArg(options::OPT_Wl_COMMA); + NewOptions->eraseArg(options::OPT_l); + if (!CfgOptions) CfgOptions = std::move(NewOptions); else { @@ -1073,6 +1086,19 @@ bool Driver::readConfigFile(StringRef FileName, appendOneArg(*CfgOptions, Opt, BaseArg); } } + + if (!CfgLinkerInputs) +CfgLinkerInputs = std::move(NewLinkerIns); + else { +// If this is a subsequent config file, append options to the previous one. +for (auto *Opt : *NewLinkerIns) { + const Arg *BaseArg = &Opt->getBaseArg(); + if (BaseArg == Opt) +BaseArg = nullptr; + appendOneArg(*CfgLinkerInputs, Opt, BaseArg); +} + } + ConfigFiles.push_back(std::string(CfgFileName)); return false; } @@ -1250,6 +1276,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { if (!ContainsError) ContainsError = loadConfigFiles(); bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr); + bool HasConfigLinkerIn = !ContainsError && CfgLinkerInputs; // All arguments, from both config file and command line. InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions) @@ -1552,6 +1579,15 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { // Construct the list of inputs. InputList Inputs; BuildInputs(C->getDefaultToolChain(), *TranslatedArgs, Inputs); + if (HasConfigLinkerIn && Inputs.size()) { +Arg *FinalPhaseArg; +if (getFinalPhase(*TranslatedArgs, &FinalPhaseArg) == phases::Link) { + DerivedArgList TranslatedLinkerIns(*CfgLinkerInputs); + for (Arg *A : *CfgLinkerInputs) +TranslatedLinkerIns.append(A); + BuildInputs(C->getDefaultToolChain(), TranslatedLinkerIns, Inputs); +} + } // Populate the tool chains for the offloading devices, if any. CreateOffloadingDeviceToolChains(*C, Inputs); diff --git a/clang/test/Driver/Inputs/config-l.cfg b/clang/test/Driver/Inputs/config-l.cfg new file mode 100644 index 00..853b179bd2da85 --- /dev/null +++ b/clang/test/Driver/Inputs/config-l.cfg @@ -0,0 +1 @@ +-Wall -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic diff --git a/clang/test/Driver/config-file.c b/clang/test/Driv
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -82,3 +82,29 @@ // CHECK-TWO-CONFIGS: -isysroot // CHECK-TWO-CONFIGS-SAME: /opt/data // CHECK-TWO-CONFIGS-SAME: -Wall + +//--- The linker input flags should be moved to the end of input list and appear only when linking. +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +// RUN: %clang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.lib -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-MSVC +// RUN: %clang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-MSVC +// CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +// CHECK-LINKING: "-Wall" +// CHECK-LINKING: "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" +// CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +// CHECK-LINKING-LIBOMP-GOES-LAST: "-Wall" pawosm-arm wrote: ??? https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -61,3 +61,29 @@ ! CHECK-TWO-CONFIGS-NEXT: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg ! CHECK-TWO-CONFIGS: -ffp-contract=fast ! CHECK-TWO-CONFIGS: -O3 + +!--- The linker input flags should be moved to the end of input list and appear only when linking. +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! RUN: %flang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.lib -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-MSVC +! RUN: %flang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-MSVC +! CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +! CHECK-LINKING: "-ffast-math" +! CHECK-LINKING: "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" +! CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +! CHECK-LINKING-LIBOMP-GOES-LAST: "-ffast-math" pawosm-arm wrote: I need some help here. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
pawosm-arm wrote: > This seems OK to me, although I'd feel more comfortable if we had a test > which relied on `--Wl,--as-needed` too. Could you provide or describe some example? https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -1062,6 +1062,16 @@ bool Driver::readConfigFile(StringRef FileName, for (Arg *A : *NewOptions) A->claim(); + std::unique_ptr NewLinkerIns = std::make_unique(); pawosm-arm wrote: changed and added a comment https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -61,3 +61,29 @@ ! CHECK-TWO-CONFIGS-NEXT: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg ! CHECK-TWO-CONFIGS: -ffp-contract=fast ! CHECK-TWO-CONFIGS: -O3 + +!--- The linker input flags should be moved to the end of input list and appear only when linking. +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! RUN: %flang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.lib -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-MSVC +! RUN: %flang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-MSVC +! CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +! CHECK-LINKING: "-ffast-math" +! CHECK-LINKING: "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" +! CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +! CHECK-LINKING-LIBOMP-GOES-LAST: "-ffast-math" pawosm-arm wrote: Seems I had wrong idea of how "SAME:" works. I'll make adjustments you're suggesting here. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm edited https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -61,3 +61,29 @@ ! CHECK-TWO-CONFIGS-NEXT: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg ! CHECK-TWO-CONFIGS: -ffp-contract=fast ! CHECK-TWO-CONFIGS: -O3 + +!--- The linker input flags should be moved to the end of input list and appear only when linking. +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! RUN: %flang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.lib -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-MSVC +! RUN: %flang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-MSVC +! CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +! CHECK-LINKING: "-ffast-math" +! CHECK-LINKING: "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" +! CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +! CHECK-LINKING-LIBOMP-GOES-LAST: "-ffast-math" pawosm-arm wrote: Adding "-fopenmp" may result in: "-latomic" "-lm" "-lomp" https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -1073,6 +1086,19 @@ bool Driver::readConfigFile(StringRef FileName, appendOneArg(*CfgOptions, Opt, BaseArg); } } + + if (!CfgLinkerInputs) +CfgLinkerInputs = std::move(NewLinkerIns); + else { +// If this is a subsequent config file, append options to the previous one. +for (auto *Opt : *NewLinkerIns) { + const Arg *BaseArg = &Opt->getBaseArg(); pawosm-arm wrote: removed https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -1062,6 +1062,19 @@ bool Driver::readConfigFile(StringRef FileName, for (Arg *A : *NewOptions) A->claim(); + // Filter out all -l and -Wl, options, put them into a separate list and erase + // from the original list of configuration file options. These will be used + // only when linking and appended after all of the command line options. + auto NewLinkerIns = std::make_unique(); + for (Arg *A : NewOptions->filtered(options::OPT_Wl_COMMA, options::OPT_l)) { +const Arg *BaseArg = &A->getBaseArg(); +if (BaseArg == A) + BaseArg = nullptr; pawosm-arm wrote: rebased https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -82,3 +82,29 @@ // CHECK-TWO-CONFIGS: -isysroot // CHECK-TWO-CONFIGS-SAME: /opt/data // CHECK-TWO-CONFIGS-SAME: -Wall + +//--- The linker input flags should be moved to the end of input list and appear only when linking. +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +// RUN: %clang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.lib -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-MSVC +// RUN: %clang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-MSVC +// CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +// CHECK-LINKING: "-Wall" +// CHECK-LINKING: "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" +// CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +// CHECK-LINKING-LIBOMP-GOES-LAST: "-Wall" pawosm-arm wrote: the changes have been made https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/117573 >From 65917ba219e26bd8547e8d1d9ff2c4ced1cdcad1 Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Mon, 25 Nov 2024 14:46:55 + Subject: [PATCH] [clang][driver] Special care for -l and -Wl, flags in config files Currently, if a -l (or -Wl,) flag is added into a config file (e.g. clang.cfg), it is situated before any object file in the effective command line. If the library requested by given -l flag is static, its symbols will not be made visible to any of the object files provided by the user. Also, the presence of any of the linker flags in a config file confuses the driver whenever the user invokes clang without any parameters. This patch solves both of those problems, by moving the -l and -Wl, flags specified in a config file to the end of the input list, and by discarding them when there are no other inputs provided by the user, or the last phase is not the linking phase. --- clang/include/clang/Driver/Driver.h | 3 +++ clang/lib/Driver/Driver.cpp | 28 +++ clang/test/Driver/Inputs/config-l.cfg | 1 + clang/test/Driver/config-file.c | 26 + flang/test/Driver/Inputs/config-l.cfg | 1 + flang/test/Driver/config-file.f90 | 26 + 6 files changed, 85 insertions(+) create mode 100644 clang/test/Driver/Inputs/config-l.cfg create mode 100644 flang/test/Driver/Inputs/config-l.cfg diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 9177d56718ee77..b21606b6f54b77 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -297,6 +297,9 @@ class Driver { /// Object that stores strings read from configuration file. llvm::StringSaver Saver; + /// Linker inputs originated from configuration file. + std::unique_ptr CfgLinkerInputs; + /// Arguments originated from configuration file. std::unique_ptr CfgOptions; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 7de8341b8d2d61..edc3d34c0fa63a 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1060,6 +1060,15 @@ bool Driver::readConfigFile(StringRef FileName, for (Arg *A : *NewOptions) A->claim(); + // Filter out all -l and -Wl, options, put them into a separate list and erase + // from the original list of configuration file options. These will be used + // only when linking and appended after all of the command line options. + auto NewLinkerIns = std::make_unique(); + for (Arg *A : NewOptions->filtered(options::OPT_Wl_COMMA, options::OPT_l)) +appendOneArg(*NewLinkerIns, A); + NewOptions->eraseArg(options::OPT_Wl_COMMA); + NewOptions->eraseArg(options::OPT_l); + if (!CfgOptions) CfgOptions = std::move(NewOptions); else { @@ -1067,6 +1076,15 @@ bool Driver::readConfigFile(StringRef FileName, for (auto *Opt : *NewOptions) appendOneArg(*CfgOptions, Opt); } + + if (!CfgLinkerInputs) +CfgLinkerInputs = std::move(NewLinkerIns); + else { +// If this is a subsequent config file, append options to the previous one. +for (auto *Opt : *NewLinkerIns) + appendOneArg(*CfgLinkerInputs, Opt); + } + ConfigFiles.push_back(std::string(CfgFileName)); return false; } @@ -1244,6 +1262,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { if (!ContainsError) ContainsError = loadConfigFiles(); bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr); + bool HasConfigLinkerIn = !ContainsError && CfgLinkerInputs; // All arguments, from both config file and command line. InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions) @@ -1540,6 +1559,15 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { // Construct the list of inputs. InputList Inputs; BuildInputs(C->getDefaultToolChain(), *TranslatedArgs, Inputs); + if (HasConfigLinkerIn && Inputs.size()) { +Arg *FinalPhaseArg; +if (getFinalPhase(*TranslatedArgs, &FinalPhaseArg) == phases::Link) { + DerivedArgList TranslatedLinkerIns(*CfgLinkerInputs); + for (Arg *A : *CfgLinkerInputs) +TranslatedLinkerIns.append(A); + BuildInputs(C->getDefaultToolChain(), TranslatedLinkerIns, Inputs); +} + } // Populate the tool chains for the offloading devices, if any. CreateOffloadingDeviceToolChains(*C, Inputs); diff --git a/clang/test/Driver/Inputs/config-l.cfg b/clang/test/Driver/Inputs/config-l.cfg new file mode 100644 index 00..853b179bd2da85 --- /dev/null +++ b/clang/test/Driver/Inputs/config-l.cfg @@ -0,0 +1 @@ +-Wall -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic diff --git a/clang/test/Driver/config-file.c b/clang/test/Driver/config-file.c index 9df830ca4c7538..0f6fc885eef1f8 100644 --- a/clang/test/Driver/config-file.c +++ b/clang/test/Driver/config-file.c @@ -82,3 +82,29 @@ // CHECK-TWO-CONFIGS: -isysroot // CHECK-
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -61,3 +61,29 @@ ! CHECK-TWO-CONFIGS-NEXT: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg ! CHECK-TWO-CONFIGS: -ffp-contract=fast ! CHECK-TWO-CONFIGS: -O3 + +!--- The linker input flags should be moved to the end of input list and appear only when linking. +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +! RUN: %flang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.lib -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-MSVC +! RUN: %flang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-MSVC +! CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +! CHECK-LINKING: "-ffast-math" +! CHECK-LINKING: "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" +! CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +! CHECK-LINKING-LIBOMP-GOES-LAST: "-ffast-math" pawosm-arm wrote: removed all SAME: https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -82,3 +82,35 @@ // CHECK-TWO-CONFIGS: -isysroot // CHECK-TWO-CONFIGS-SAME: /opt/data // CHECK-TWO-CONFIGS-SAME: -Wall + +//--- The linker input flags should be moved to the end of input list and appear only when linking. +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING +// RUN: %clang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-MSVC +// RUN: %clang --target=x86_64-pc-windows-msvc--config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-MSVC +// CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +// CHECK-LINKING: "-Wall" +// CHECK-LINKING: "{{.*}}-{{.*}}.o" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" +// CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +// CHECK-LINKING-LIBOMP-GOES-LAST: "-Wall" +// CHECK-LINKING-LIBOMP-GOES-LAST-SAME: "-fopenmp" +// CHECK-LINKING-LIBOMP-GOES-LAST: "{{.*}}-{{.*}}.o" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" +// CHECK-LINKING-LIBOMP-GOES-LAST-SAME: "-lomp" +// CHECK-NOLINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg +// CHECK-NOLINKING: "-Wall" +// CHECK-NOLINKING-NO: "-lm" pawosm-arm wrote: ok https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
@@ -82,3 +82,35 @@ // CHECK-TWO-CONFIGS: -isysroot // CHECK-TWO-CONFIGS-SAME: /opt/data // CHECK-TWO-CONFIGS-SAME: -Wall + +//--- The linker input flags should be moved to the end of input list and appear only when linking. +// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING pawosm-arm wrote: Thanks for all the comments. Added both things. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
pawosm-arm wrote: > > I took a look and I think it may be too awkward to do, as we'd want to run > > e.g. readelf afterwards. But an example is > > `lld/test/ELF/as-needed-not-in-regular.s`. The idea being: if > > `-Wl,--as-needed` is in the config file, do we correctly prune an > > unnecessary library from a built object, or is the order wrong? We can > > check that with `llvm-readelf`. > > There are two problems here. First, following the readelf path goes too far > into the linker competences, whatever is set for the linker, the linker is > obliged to do, and the order of whatever is set for the linker is already > tested by other `-Wl,` occurrences in the currently available test cases. > > Yet your request also exposes another problem, and I can see that further > comments also mention it. With my patch, there is no way to affect the user's > command line linker input parameters. So if a config file is a mean for > turning clang's behavior into the gcc's behavior (AFAIR, gcc always emits > `-Wl,--as-needed` at the beginning of the linker invocation command line, > while clang doesn't do that), this patch rejects such possibility. Something > more clever is needed, the positional idea seems the right one, I could see > it like this (a config file example, notice the pipe `|` character): > > ``` > -Wall -Wl,--as-needed | -lm -Wl,-Bstatic -lhappy -Wl,-Bdynamic > ``` > > ...things after the pipe would be appended at the end of the parameters list > and only when it is known that there will be the linking. But this would > require a modification in the config file reading routine, the second list of > config options would have to be created much earlier than my patch proposes. > > Back to the drawing board then. Actually, it was easy to implement, I've presented it here. https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/117573 >From b9dd5f9356d457ef1ad050083d650a1da21c4377 Mon Sep 17 00:00:00 2001 From: Pawel Osmialowski Date: Mon, 25 Nov 2024 14:46:55 + Subject: [PATCH] [clang][driver] Special care for -l and -Wl, flags in config files Currently, if a -l (or -Wl,) flag is added into a config file (e.g. clang.cfg), it is situated before any object file in the effective command line. If the library requested by given -l flag is static, its symbols will not be made visible to any of the object files provided by the user. Also, the presence of any of the linker flags in a config file confuses the driver whenever the user invokes clang without any parameters. This patch attempts to solve both of the problems, by allowing a split of the arguments list around the pipe character. The head part of the list will be used as before, but the tail part will be appended after the command line flags provided by the user and only when it is known that the linking should occur. --- clang/include/clang/Driver/Driver.h | 3 ++ clang/lib/Driver/Driver.cpp | 55 +++ clang/test/Driver/Inputs/config-l.cfg | 1 + clang/test/Driver/config-file.c | 26 + flang/test/Driver/Inputs/config-l.cfg | 1 + flang/test/Driver/config-file.f90 | 26 + 6 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 clang/test/Driver/Inputs/config-l.cfg create mode 100644 flang/test/Driver/Inputs/config-l.cfg diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 9177d56718ee77..b21606b6f54b77 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -297,6 +297,9 @@ class Driver { /// Object that stores strings read from configuration file. llvm::StringSaver Saver; + /// Linker inputs originated from configuration file. + std::unique_ptr CfgLinkerInputs; + /// Arguments originated from configuration file. std::unique_ptr CfgOptions; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 7de8341b8d2d61..9ea22419d392b0 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1039,34 +1039,63 @@ bool Driver::readConfigFile(StringRef FileName, } // Try reading the given file. - SmallVector NewCfgArgs; - if (llvm::Error Err = ExpCtx.readConfigFile(FileName, NewCfgArgs)) { + SmallVector NewCfgFileArgs; + if (llvm::Error Err = ExpCtx.readConfigFile(FileName, NewCfgFileArgs)) { Diag(diag::err_drv_cannot_read_config_file) << FileName << toString(std::move(Err)); return true; } + // Populate head and tail lists. The tail list is used only when linking. + SmallVector NewCfgHeadArgs, NewCfgTailArgs; + bool SeenPipe = false; + for (const char *Opt : NewCfgFileArgs) { +if (!strcmp(Opt, "|")) { + SeenPipe = true; +} else { + if (SeenPipe) +NewCfgTailArgs.push_back(Opt); + else +NewCfgHeadArgs.push_back(Opt); +} + } + // Read options from config file. llvm::SmallString<128> CfgFileName(FileName); llvm::sys::path::native(CfgFileName); - bool ContainErrors; - auto NewOptions = std::make_unique( - ParseArgStrings(NewCfgArgs, /*UseDriverMode=*/true, ContainErrors)); + bool ContainErrors = false; + auto NewHeadOptions = std::make_unique( + ParseArgStrings(NewCfgHeadArgs, /*UseDriverMode=*/true, ContainErrors)); + if (ContainErrors) +return true; + auto NewTailOptions = std::make_unique( + ParseArgStrings(NewCfgTailArgs, /*UseDriverMode=*/true, ContainErrors)); if (ContainErrors) return true; // Claim all arguments that come from a configuration file so that the driver // does not warn on any that is unused. - for (Arg *A : *NewOptions) + for (Arg *A : *NewHeadOptions) +A->claim(); + for (Arg *A : *NewTailOptions) A->claim(); if (!CfgOptions) -CfgOptions = std::move(NewOptions); +CfgOptions = std::move(NewHeadOptions); else { // If this is a subsequent config file, append options to the previous one. -for (auto *Opt : *NewOptions) +for (auto *Opt : *NewHeadOptions) appendOneArg(*CfgOptions, Opt); } + + if (!CfgLinkerInputs) +CfgLinkerInputs = std::move(NewTailOptions); + else { +// If this is a subsequent config file, append options to the previous one. +for (auto *Opt : *NewTailOptions) + appendOneArg(*CfgLinkerInputs, Opt); + } + ConfigFiles.push_back(std::string(CfgFileName)); return false; } @@ -1244,6 +1273,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { if (!ContainsError) ContainsError = loadConfigFiles(); bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr); + bool HasConfigLinkerIn = !ContainsError && CfgLinkerInputs; // All arguments, from both config file and command line. InputArgList Args = std::move(HasCo
[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)
pawosm-arm wrote: > @pawosm-arm See also > https://wiki.gentoo.org/wiki/Project:Quality_Assurance/As-needed#Importance_of_linking_order. > It is common for distributions to enable `-Wl,--as-needed` via patches to > the compiler or configuration files. Indeed, the distributions couldn't avoid this annoying behavior of clang (spitting `undefined reference to `main'`) when started without any command line params: ``` $ clang /usr/bin/x86_64-pc-linux-gnu-ld.bfd: /usr/lib/gcc/x86_64-pc-linux-gnu/14/../../../../lib64/Scrt1.o: in function `_start': (.text+0x17): undefined reference to `main' clang: error: linker command failed with exit code 1 (use -v to see invocation) $ clang --no-default-config clang: error: no input files $ clang -### clang version 18.1.8 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/lib/llvm/18/bin Configuration file: /etc/clang/x86_64-pc-linux-gnu-clang.cfg "/usr/bin/x86_64-pc-linux-gnu-ld.bfd" "--hash-style=gnu" "--eh-frame-hdr" "-m" "elf_x86_64" "-pie" "-dynamic-linker" "/lib64/ld-linux-x86-64.so.2" "-o" "a.out" "/usr/lib/gcc/x86_64-pc-linux-gnu/14/../../../../lib64/Scrt1.o" "/usr/lib/gcc/x86_64-pc-linux-gnu/14/../../../../lib64/crti.o" "/usr/lib/gcc/x86_64-pc-linux-gnu/14/crtbeginS.o" "-L/usr/lib/gcc/x86_64-pc-linux-gnu/14" "-L/usr/lib/gcc/x86_64-pc-linux-gnu/14/../../../../lib64" "-L/lib/../lib64" "-L/usr/lib/../lib64" "-L/usr/lib/gcc/x86_64-pc-linux-gnu/14/../../../../x86_64-pc-linux-gnu/lib" "-L/lib" "-L/usr/lib" "-z" "relro" "-z" "now" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "/usr/lib/gcc/x86_64-pc-linux-gnu/14/crtendS.o" "/usr/lib/gcc/x86_64-pc-linux-gnu/14/../../../../lib64/crtn.o" $ cat /etc/clang/x86_64-pc-linux-gnu-clang.cfg # This configuration file is used by x86_64-pc-linux-gnu-clang driver. @gentoo-common.cfg @gentoo-common-ld.cfg @gentoo-cet.cfg` ``` https://github.com/llvm/llvm-project/pull/117573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/116432 >From 12654b674725715422ba114e98bf76e40d557d16 Mon Sep 17 00:00:00 2001 From: Paul Osmialowski Date: Tue, 3 Dec 2024 12:41:15 + Subject: [PATCH] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath Using `-fveclib=ArmPL` without libamath likely effects in the link-time errors. --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 29 ++ clang/lib/Driver/ToolChains/MSVC.cpp | 6 + clang/test/Driver/fveclib.c| 13 ++ flang/test/Driver/fveclib.f90 | 13 ++ 4 files changed, 61 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 8d977149e62485..9e6d43fe3f2f5d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -490,6 +490,35 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +const llvm::Triple &Triple = TC.getTriple(); +StringRef V = A->getValue(); +if (V == "ArmPL" && (Triple.isOSLinux() || Triple.isOSDarwin())) { + // To support -fveclib=ArmPL we need to link against libamath. + // Some of the libamath functions depend on libm, at the same time, + // libamath exports its own implementation of some of the libm + // functions. Since here we are interested only in the subset of + // libamath functions that is covered by the veclib mappings, + // we need to do the following: + // + // 1. On Linux, link only when actually needed. + // + // 2. Prefer libm functions over libamath. + // + // 3. Link against libm to resolve libamath dependencies. + // + if (Triple.isOSLinux()) { +CmdArgs.push_back(Args.MakeArgString("--push-state")); +CmdArgs.push_back(Args.MakeArgString("--as-needed")); + } + CmdArgs.push_back(Args.MakeArgString("-lm")); + CmdArgs.push_back(Args.MakeArgString("-lamath")); + CmdArgs.push_back(Args.MakeArgString("-lm")); + if (Triple.isOSLinux()) +CmdArgs.push_back(Args.MakeArgString("--pop-state")); + addArchSpecificRPath(TC, Args, CmdArgs); +} + } } void tools::addLinkerCompressDebugSectionsOption( diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp index 80799d1e715f07..752c2e2751ab67 100644 --- a/clang/lib/Driver/ToolChains/MSVC.cpp +++ b/clang/lib/Driver/ToolChains/MSVC.cpp @@ -84,6 +84,12 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA, else if (TC.getTriple().isWindowsArm64EC()) CmdArgs.push_back("-machine:arm64ec"); + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +StringRef V = A->getValue(); +if (V == "ArmPL") + CmdArgs.push_back(Args.MakeArgString("--dependent-lib=amath")); + } + if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles) && !C.getDriver().IsCLMode() && !C.getDriver().IsFlangMode()) { CmdArgs.push_back("-defaultlib:libcmt"); diff --git a/clang/test/Driver/fveclib.c b/clang/test/Driver/fveclib.c index 09a12c2327137c..52e5c30eafcd8d 100644 --- a/clang/test/Driver/fveclib.c +++ b/clang/test/Driver/fveclib.c @@ -112,3 +112,16 @@ /* Verify no warning when math-errno is re-enabled for a different veclib (that does not imply -fno-math-errno). */ // RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL -fmath-errno -fveclib=LIBMVEC %s 2>&1 | FileCheck --check-prefix=CHECK-REPEAT-VECLIB %s // CHECK-REPEAT-VECLIB-NOT: math errno enabled + +/// Verify that vectorized routines library is being linked in. +// RUN: %clang -### --target=aarch64-pc-windows-msvc -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL-MSVC %s +// RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL-LINUX %s +// RUN: %clang -### --target=arm64-apple-darwin -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-LINKING-ARMPL-DARWIN %s +// CHECK-LINKING-ARMPL-LINUX: "--push-state" "--as-needed" "-lm" "-lamath" "-lm" "--pop-state" +// CHECK-LINKING-ARMPL-DARWIN: "-lm" "-lamath" "-lm" +// CHECK-LINKING-ARMPL-MSVC: "--dependent-lib=amath" + +/// Verify that the RPATH is being set when needed. +// RUN: %clang -### --target=aarch64-linux-gnu -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir -frtlib-add-rpath -fveclib=ArmPL %s 2>&1 | FileCheck --check-prefix=CHECK-RPATH-ARMPL %s +// CHECK-RPATH-ARMPL: "--push-state" "--as-needed" "-lm" "-lamath" "-lm" "--pop-state" +// CHECK-RPATH-ARMPL-SAME: "-rpath" diff --git a/flang/test/Driver/fveclib.f90 b/flang/test/Driver/fveclib.f90 index 14c59b0616f828..e863e8d6d63c5a 100644 --- a/flang/test/Driver/fveclib.f90 +++ b/fl
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
@@ -102,3 +102,17 @@ /* Verify no warning when math-errno is re-enabled for a different veclib (that does not imply -fno-math-errno). */ // RUN: %clang -### --target=aarch64-linux-gnu -fveclib=ArmPL -fmath-errno -fveclib=LIBMVEC %s 2>&1 | FileCheck --check-prefix=CHECK-REPEAT-VECLIB %s // CHECK-REPEAT-VECLIB-NOT: math errno enabled + +/* Verify that vectorized routines library is being linked in. */ pawosm-arm wrote: ok https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
@@ -490,6 +490,17 @@ void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs, else A.renderAsInput(Args, CmdArgs); } + if (const Arg *A = Args.getLastArg(options::OPT_fveclib)) { +if (A->getNumValues() == 1) { pawosm-arm wrote: ok https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [clang][driver] When -fveclib=ArmPL flag is in use, always link against libamath (PR #116432)
pawosm-arm wrote: I've added these suggested changes. Also, I was advised by the libamath team that the complicated relationship between libamath and libm needs more elaborate handling, hence additional `-lm` before `-lamath` (see comment that I've also added). Also I was advised to use `--as-needed`, so I wrapped it around push/pop-state. https://github.com/llvm/llvm-project/pull/116432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits