Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable GNUABI build on more targets #525 #562

Merged
merged 3 commits into from
Jul 17, 2024
Merged

Conversation

joeramsay
Copy link
Contributor

Previously SLEEF_BUILD_GNUABI_LIBS was silently ignored except for a small number of targets, or if the compiler does not support weak aliases.

It can now be built regardless of OS and compiler on AArch64 and x86_64 - trying to enable it on any other target is now an error.

The GLIBC *_finite symbols are handled using the same workaround as is used for the DALIAS macro when compiling on a target which does not support aliases. Though in practice these symbols are unlikely to be required on systems where aliases are unsupported, they are part of the API so need adding, if just to make the tests build.

Also removed two CMake variables:

  • ENABLE_ALIAS, which was unused
  • ENABLE_GNUABI, which was now redundant but could still be forced on, leading to cryptic build failures Both names are still used by the preprocessor in sleefsimd* sources for managing names and aliases.

Previously SLEEF_BUILD_GNUABI_LIBS was silently ignored except for a
small number of targets, or if the compiler does not support weak
aliases.

It can now be built regardless of OS and compiler on AArch64 and
x86_64 - trying to enable it on any other target is now an error.

The GLIBC *_finite symbols are handled using the same workaround as is
used for the DALIAS macro when compiling on a target which does not
support aliases. Though in practice these symbols are unlikely to be
required on systems where aliases are unsupported, they are part of
the API so need adding, if just to make the tests build.

Also removed two CMake variables:
- ENABLE_ALIAS, which was unused
- ENABLE_GNUABI, which was now redundant but could still be forced on,
  leading to cryptic build failures
Both names are still used by the preprocessor in sleefsimd* sources
for managing names and aliases.
The previous patch only added support for the unmasked build - masked
symbols also need to sidestep aliases when they are not supported.
Default is on, but this setting was previously just ignored when
unsupported. Disable it in the failing pipelines.
@blapie blapie merged commit 0e47fcc into shibatch:master Jul 17, 2024
30 checks passed
blapie pushed a commit to blapie/sleef that referenced this pull request Aug 2, 2024
* Enable GNUABI build on more targets shibatch#525

Previously SLEEF_BUILD_GNUABI_LIBS was silently ignored except for a
small number of targets, or if the compiler does not support weak
aliases.

It can now be built regardless of OS and compiler on AArch64 and
x86_64 - trying to enable it on any other target is now an error.

The GLIBC *_finite symbols are handled using the same workaround as is
used for the DALIAS macro when compiling on a target which does not
support aliases. Though in practice these symbols are unlikely to be
required on systems where aliases are unsupported, they are part of
the API so need adding, if just to make the tests build.

Also removed two CMake variables:
- ENABLE_ALIAS, which was unused
- ENABLE_GNUABI, which was now redundant but could still be forced on,
  leading to cryptic build failures
Both names are still used by the preprocessor in sleefsimd* sources
for managing names and aliases.

* Fix masked GNUABI build for NOALIAS

Masked symbols also need to sidestep aliases when they are not
supported.

* Disable GNUABI for unsupported targets in precommit

Default is on, but this setting was previously just ignored when
unsupported. Disable it in the failing pipelines.
blapie pushed a commit to blapie/sleef that referenced this pull request Aug 2, 2024
* Enable GNUABI build on more targets shibatch#525

Previously SLEEF_BUILD_GNUABI_LIBS was silently ignored except for a
small number of targets, or if the compiler does not support weak
aliases.

It can now be built regardless of OS and compiler on AArch64 and
x86_64 - trying to enable it on any other target is now an error.

The GLIBC *_finite symbols are handled using the same workaround as is
used for the DALIAS macro when compiling on a target which does not
support aliases. Though in practice these symbols are unlikely to be
required on systems where aliases are unsupported, they are part of
the API so need adding, if just to make the tests build.

Also removed two CMake variables:
- ENABLE_ALIAS, which was unused
- ENABLE_GNUABI, which was now redundant but could still be forced on,
  leading to cryptic build failures
Both names are still used by the preprocessor in sleefsimd* sources
for managing names and aliases.

* Fix masked GNUABI build for NOALIAS

Masked symbols also need to sidestep aliases when they are not
supported.

* Disable GNUABI for unsupported targets in precommit

Default is on, but this setting was previously just ignored when
unsupported. Disable it in the failing pipelines.
joeramsay added a commit to joeramsay/sleef that referenced this pull request Sep 16, 2024
Only Linux supports the AArch64 vector ABI, so it doesn't make sense
to expose these symbols anywhere else for AArch64. Without any
compelling reason to provide them on x86_64, it is better to just
revert this patch and deal with this more carefully after the release.

This reverts commit 0e47fcc.
blapie pushed a commit that referenced this pull request Sep 17, 2024
Only Linux supports the AArch64 vector ABI, so it doesn't make sense
to expose these symbols anywhere else for AArch64. Without any
compelling reason to provide them on x86_64, it is better to just
revert this patch and deal with this more carefully after the release.

This reverts commit 0e47fcc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants