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

[AArch64][CodeGen] Fix wrong operand order when creating vcmla intrin… #9012

Open
wants to merge 1 commit into
base: swift/release/6.0
Choose a base branch
from

Conversation

ellishg
Copy link

@ellishg ellishg commented Jul 25, 2024

…sic (llvm#65278)

@ellishg
Copy link
Author

ellishg commented Jul 25, 2024

We've discovered a floating point miscompilation in XCode 16 when using -ffp-contract=fast. I found this patch that fixes the issue.

@fhahn can we get this merged into the release branch?

@ellishg
Copy link
Author

ellishg commented Jul 25, 2024

Oh, and the patch did not cherry-pick cleanly so I had to fix the tests to make them pass on this branch.

@drodriguez
Copy link

@swift-ci please test llvm

@drodriguez
Copy link

@swift-ci please test

@drodriguez
Copy link

(I think the LLVM tests will fail in some cases, but hopefully not in the specific tests that the commit changes)

@drodriguez
Copy link

drodriguez commented Jul 26, 2024

About the LLVM Test failures:

I left a comment about ExtractAPI/enum.c last week in #8987 (comment)

For the Driver/ ones is probably because #8668 is in stable/20230725, but not in swift/release/6.0 (has been like that a couple of months)

@ellishg
Copy link
Author

ellishg commented Aug 21, 2024

Friendly ping @fhahn @ravikandhadai

@ravikandhadai
Copy link

cc @Gerolf-Apple

@fhahn
Copy link

fhahn commented Aug 28, 2024

Unfortunately I think it is too late at this point in the release to take this change.

@drodriguez
Copy link

Should it be applied to stable/20230725 at least so it is used by a future Swift 6.0.x release? What's the process to get it in the next release before stable/20240723 is used?

@ellishg
Copy link
Author

ellishg commented Aug 29, 2024

This is a critical codegen fix. If it isn't picked, some floating point operations will simply give the wrong result. Also, the non-test changes are pretty simple and safe in my opinion.

Copy link

@Gerolf-Apple Gerolf-Apple left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@drodriguez
Copy link

@Gerolf-Apple are you able to merge into this branch for us? Is this the right branch to merge at this point?

@Gerolf-Apple
Copy link

We will need to get this on a newer branch.

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.

6 participants