MaskRay wrote:
Note that if the code uses LLVM_ENABLE_PER_TARGET_RUNTIME_DIR to decide whether
to probe the new or old hierarchy first, these clang/test/Driver
`libclang_rt.asan.a`/`libclang_rt.asan-x86_64.a` tests will need a `REQUIRES:
a_feature_signaling_the_config`, otherwise the tests wou
pogo59 wrote:
Poking around more, it looks like the canonical way to convert a CMake variable
to a C++ define is to add an entry to
llvm/include/llvm/Config/llvm-config.h.cmake. I'll have a go at doing it that
way.
https://github.com/llvm/llvm-project/pull/89775
__
tru wrote:
I think I suggested that we should make the CMake variable change the driver
behaviour to reflect the setting in order to remove unexpected fallbacks and
that the vendor could select the layout. I still think that's probably the
better solution.
https://github.com/llvm/llvm-projec
tru wrote:
> > LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on
>
>
>
> I'm unable to find what code this affects. I don't see it mentioned anywhere
> in clang/lib or clang/include.
>
>
>
> It does seem like it should control the behavior of
> `ToolChain::getCompilerRT`; where I had added the Windo
nico wrote:
> Again, clangDriver does file probing in various places.
This is the only place where we probe link-time inputs for compile-time
decisions as far as I know.
https://github.com/llvm/llvm-project/pull/89775
___
cfe-commits mailing list
cfe
pogo59 wrote:
> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on
I'm unable to find what code this affects. I don't see it mentioned anywhere in
clang/lib or clang/include.
It does seem like it should control the behavior of `ToolChain::getCompilerRT`;
where I had added the Windows/PS check, seems like
MaskRay wrote:
I understand that there is some distributed build system pain and I feel
sympathy.
However, I have pointed out that it is *unsupported* to not provide files for
driver probing, therefore I am not sure it is fair to revert the patches
improving
`LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
tru wrote:
And I think it's better to revert it all instead of implementing this
half-revert in this PR in that case and then we should discuss how to move
forward.
https://github.com/llvm/llvm-project/pull/89775
___
cfe-commits mailing list
cfe-comm
tru wrote:
Honestly - I think going back to *one* style of runtime path is to be preferred
now. But I don't think it's fair to say that it doesn't solve any problems,
because we switched to it so that we could ship more platforms in one toolchain
package without having to add cfg files and sim
nico wrote:
Anyway, for this specific issue it sounds like the change here was
unintentional. As suggested by mstorsjo at
https://github.com/llvm/llvm-project/pull/87866#issuecomment-2073231116, I
think it'd be good to revert these changes, and then make a mindful decision
about this when rel
nico wrote:
> My recollection of the past discussions is that we want users to switch to
> the new hierarchy.
FWIW this doesn't match my understanding. phosek added the new hierarchy to
Fuchsia, maskray put in some work to enable it elsewhere, but it's a huge
headache, and we've forcibly turn
tru wrote:
I agree that if downstream want to change stuff, they need to engage. We can't
guess what microsoft wants to do (or sony) unless we have a discussion about
it. This is also documented in the developer policy. If there are missed
release notes, they need to be added of course.
That
pogo59 wrote:
> There have been a few cases before and we did not write specific release
> notes for the driver behavior:
Possibly users of those targets do not depend on distributed builds the way our
users do, and didn't run into the problem? They don't look like Windows
targets, anyway. Di
MaskRay wrote:
> What is -frtlib-defaultlib? I don't see it in include/clang/Driver/Options.td.
This is from @nico in a recent change #89642. It seems that Chromium is moving
away from driver passing `--dependent-lib=` to ld?
https://github.com/llvm/llvm-project/pull/89775
MaskRay wrote:
We can add a release note.
There is a lot of path probing code in clangDriver.
clangDriver trusts the result and does its best to construct assembler/linker
command lines.
This distributed build system use case is an unfortunate case where compilation
depends on linking action b
pogo59 wrote:
> I think the distributed build system users need to prepare a file hierarchy
> to get the intended --dependent-lib= (like how to prepare --sysroot in
> clang/test/Driver tests), or turn off it with the recent -frtlib-defaultlib.
Is there a Release Note for this new requirement o
MaskRay wrote:
> > My recollection of the past discussions is that we want users to switch to
> > the new hierarchy.
>
> Is Microsoft a "user" who agreed to this change? Was clang-vendors tagged on
> this discussion about a new file hierarchy? I don't recall it. The library
> name change is n
pogo59 wrote:
> My recollection of the past discussions is that we want users to switch to
> the new hierarchy.
Is Microsoft a "user" who agreed to this change? Was clang-vendors tagged on
this discussion about a new file hierarchy? I don't recall it. The library name
change is news to me, a
mstorsjo wrote:
> > The recent changes, #81037 and #87866, were (as far as I know) only
> > intended to change what is printed as error messages, when neither is
> > found, to help users correct their setup for the new style layout. But
> > those changes also seem to have unexpected effects in
MaskRay wrote:
My recollection of the past discussions is that we want users to switch to the
new hierarchy.
*BSD, Linux, and z/OS have migrated but I am not familiar with the Windows
ecosystems.
`ToolChain::getCompilerRT` detects both the old and new compiler-rt directory.
Does it not work fo
pogo59 wrote:
> It's been like that for maybe 2-3 years now and no one has complained about it
As a downstream vendor I can tell you we work around things all the time
without bothering to try to fix things upstream (I try to encourage more
engagement, but I can't control what people actually
pogo59 wrote:
> what gets emitted as embedded directive, when the compiler doesn't see either
> of them at compile time (with e.g. distributed build systems), while they
> might be available at link time.
This is exactly the scenario we had. I will double-check that it is solved by
my patch,
mstorsjo wrote:
> Commit
> [b876596](https://github.com/llvm/llvm-project/commit/b876596a76cdc183439b36455d26883b67f8ee51)
> corrected default compiler-rt library names for many targets
Are you sure it's this change? There are reports of similar changes showing up
in https://github.com/llvm/l
tru wrote:
It's been like that for maybe 2-3 years now and no one has complained about it,
so I think that change is solid. I can suggest a CMake change, but last time it
was discussed I think @maskray was against a new variable, but since we might
need to have some different behaviour it migh
pogo59 wrote:
> if you build with runtimes on windows (which is the suggested way) it will
> end up with the new style without arch suffix
Was someone from Microsoft party to that decision? If not, perhaps it needs to
be revisited. We should not be making decisions about how they want to do
pogo59 wrote:
Microsoft obviously builds and distributes with the arch suffix, supporting
both 32-bit and 64-bit in the same distro. So I think they will continue to
need it. (I can't actually speak for MS, but it's what I see in the Visual
Studio installation.)
I don't know enough about how
tru wrote:
We have a custom toolchain that uses the new style on windows and if you build
with runtimes on windows (which is the suggested way) it will end up with the
new style without arch suffix. I don't think we can assume this is correct for
windows in all setups. I am fine with this chan
https://github.com/pogo59 edited https://github.com/llvm/llvm-project/pull/89775
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
28 matches
Mail list logo
