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

(release 20.x non-upstream patch) [clang] Lower non-builtin sincos[f|l] calls to llvm.sincos.* when -fno-math-errno is set #83

Merged

Conversation

MacDue
Copy link
Contributor

@MacDue MacDue commented Feb 10, 2025

This will allow vectorizing these calls (after a few more patches). This should not change the codegen for targets that enable the use of AA during the codegen (in TargetSubtargetInfo::useAA()). This includes targets such as AArch64. This notably does not include x86 but can be worked around by passing -mllvm -combiner-global-alias-analysis=true to clang.


Downstream issue: #87

…o-math-errno is set

This will allow vectorizing these calls (after a few more patches). This
should not change the codegen for targets that enable the use of AA
during the codegen (in `TargetSubtargetInfo::useAA()`). This includes
targets such as AArch64. This notably does not include x86 but can be
worked around by passing `-mllvm -combiner-global-alias-analysis=true`
to clang.
@MacDue
Copy link
Contributor Author

MacDue commented Feb 10, 2025

This PR is a copy of the LLVM PR llvm/llvm-project#121763, which was not possible to upstream for the release.

pawosm-arm
pawosm-arm previously approved these changes Feb 10, 2025
Copy link
Contributor

@dcandler dcandler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a downstream change, it should follow the downstream patch policy: https://github.com/arm/arm-toolchain/blob/arm-software/CONTRIBUTING.md#downstream-patch-policy

If it's a critical change that didn't make the upstream release, then please make sure it's tracked appropriately.

@MacDue MacDue marked this pull request as draft February 20, 2025 14:14
@MacDue MacDue marked this pull request as ready for review February 24, 2025 14:33
Copy link
Contributor

@dcandler dcandler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - do you need this merged too?

@MacDue
Copy link
Contributor Author

MacDue commented Feb 24, 2025

LGTM - do you need this merged too?

Yes please :)

@dcandler dcandler merged commit e7d5a88 into arm:release/arm-software/20.x Feb 24, 2025
@MacDue MacDue deleted the release_20_sincos_builtin branch February 24, 2025 14:58
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.

3 participants