ro added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:150
+ const SanitizerArgs &SA = getToolChain().getSanitizerArgs();
+ if (LINKER_SUPPORTS_Z_RELAX_TRANSTLS &&
+ getToolChain().getTriple().getArch() == llvm::Triple::x86_64 &&
----------------
MaskRay wrote:
> MaskRay wrote:
> > Can the configure-time variable `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` be
> > replaced by a version check on the triple suffix?
> >
> > You can see some tests where `x86_64-unknown-freebsd11` or
> > `x86_64-unknown-freebsd12` is used. The idea is that the driver
> > continuously bumps the default version.
> .. and the code base should not be littered with -D macros from
> configure-time variables here and there.
> Can the configure-time variable `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` be
> replaced by a version check on the triple suffix?
>
> You can see some tests where `x86_64-unknown-freebsd11` or
> `x86_64-unknown-freebsd12` is used. The idea is that the driver continuously
> bumps the default version.
This cannot work: both Solaris 11 and Illumos share the same configure triple,
`x86_64-pc-solaris2.11`.
Please keep in mind that Solaris 11 was released almost exactly 9 years ago, is
in its 5th micro version since, each of which getting monthly updates (at least
for a while). Even if it were possible to extract the micro version and update
number from the triple, hardcoding which introduced which feature is
effectively impossible. Some `ld` and `libc` features are also backported to
an update of the previous micro version; hardcoding all this knowledge is
simply a nightmare.
TBH, I feel put back to the bad old times 20+ years ago when software tried to
hardcode all knowledge about the environment, and usually failed even at the
time of writing. This only got better once feature-based approaches got
traction.
================
Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:150
+ const SanitizerArgs &SA = getToolChain().getSanitizerArgs();
+ if (LINKER_SUPPORTS_Z_RELAX_TRANSTLS &&
+ getToolChain().getTriple().getArch() == llvm::Triple::x86_64 &&
----------------
ro wrote:
> MaskRay wrote:
> > MaskRay wrote:
> > > Can the configure-time variable `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` be
> > > replaced by a version check on the triple suffix?
> > >
> > > You can see some tests where `x86_64-unknown-freebsd11` or
> > > `x86_64-unknown-freebsd12` is used. The idea is that the driver
> > > continuously bumps the default version.
> > .. and the code base should not be littered with -D macros from
> > configure-time variables here and there.
> > Can the configure-time variable `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` be
> > replaced by a version check on the triple suffix?
> >
> > You can see some tests where `x86_64-unknown-freebsd11` or
> > `x86_64-unknown-freebsd12` is used. The idea is that the driver
> > continuously bumps the default version.
>
> This cannot work: both Solaris 11 and Illumos share the same configure
> triple, `x86_64-pc-solaris2.11`.
>
> Please keep in mind that Solaris 11 was released almost exactly 9 years ago,
> is in its 5th micro version since, each of which getting monthly updates (at
> least for a while). Even if it were possible to extract the micro version
> and update number from the triple, hardcoding which introduced which feature
> is effectively impossible. Some `ld` and `libc` features are also backported
> to an update of the previous micro version; hardcoding all this knowledge is
> simply a nightmare.
>
> TBH, I feel put back to the bad old times 20+ years ago when software tried
> to hardcode all knowledge about the environment, and usually failed even at
> the time of writing. This only got better once feature-based approaches got
> traction.
> .. and the code base should not be littered with -D macros from
> configure-time variables here and there.
What's so bad about that? Why is `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` detected
at build time better than a runtime check for Solaris/x86 11.3 or higher, but
not 11.3 update < x and not Illumos?
With the configure-based approach, I know at least which features are tested by
the code, rather than finding months or even years later that some check is
hardcoded in some obscure part of the code that I missed? And for new targets,
many things just fall into place automatically. Consider `compiler-rt`'s maze
in `sanitizer_platform_interceptors.h`. With features introduced in micro
versions and updates, one cannot even express that some feature got introduced
in one of those.
================
Comment at: clang/tools/driver/CMakeLists.txt:123
+
+check_linker_flag("-Wl,-z,relax=transtls" LINKER_SUPPORTS_Z_RELAX_TRANSTLS)
----------------
MaskRay wrote:
> GNU ld reports a warning instead of an error when an unknown `-z` is seen.
> The warning remains a warning even with `--fatal-warnings`.
> GNU ld reports a warning instead of an error when an unknown `-z` is seen.
> The warning remains a warning even with `--fatal-warnings`.
Thanks for reminding me about that misfeature of GNU `ld`. I guess
`check_linker_flags` needs to be updated to handle that.
In the case at hand, it won't matter either way: the flag is only passed to
`ld`, which on Solaris is guaranteed to be the native linker. Once (if at all)
I get around to completing D85309, I can deal with that. For now, other
targets won't see linker warnings about this flag, other than when the flag is
used at build time.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91605/new/
https://reviews.llvm.org/D91605
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
