-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Cherrypick fixes for issue 74866 to release/6.0 #77261
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On Windows/AArch64, a different register is used between when an arugment is both inreg and sret (X0 or X1) and when it is just sret (X8) as the following comment indicates: https://github.com/llvm/llvm-project/blob/46fe36a4295f05d5d3731762e31fc4e6e99863e9/llvm/lib/Target/AArch64/AArch64CallingConvention.td#L42 ``` // In AAPCS, an SRet is passed in X8, not X0 like a normal pointer parameter. // However, on windows, in some circumstances, the SRet is passed in X0 or X1 // instead. The presence of the inreg attribute indicates that SRet is // passed in the alternative register (X0 or X1), not X8: // - X0 for non-instance methods. // - X1 for instance methods. // The "sret" attribute identifies indirect returns. // The "inreg" attribute identifies non-aggregate types. // The position of the "sret" attribute identifies instance/non-instance // methods. // "sret" on argument 0 means non-instance methods. // "sret" on argument 1 means instance methods. CCIfInReg<CCIfType<[i64], CCIfSRet<CCIfType<[i64], CCAssignToReg<[X0, X1]>>>>>, CCIfSRet<CCIfType<[i64], CCAssignToReg<[X8]>>>, ``` So missing/dropping inreg can cause a codegen bug. This is a partial fix for swiftlang#74866 Cherrypick swiftlang#76159
In GenCall, fix the IR gen for C++ method calls as under MSVC as the calling conventions for free functions and C++ methods can be different. This also fixes the missing inreg (on sret arguments) issues on Windows ARM64. Also refactor to use CGFunctionInfo returnInfo isSretAfterThis to detect when to reorder the sret and the this arguments under MSVC. In ClagImporter, don't drop the return type for the compound assignment operators such as operator+= when the return value is a reference so that the CGFunctionInfo will be correctly indicate an indirect return for the compound assignment operators. Cherrypick swiftlang#76324
Please test with following PRs: @swift-ci please test |
On Windows ARM64, how a struct value type is returned is sensitive to conditions including whether a user-defined constructor exists, etc. See https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-170#return-values That caused a calling convention mismatch between the non-USED_IN_CPP_SOURCE (Swift) side and the USE_IN_CPP_SOURCE (C++) side and a crash. Add this constructor so that the calling convention matches. This is a fix for the OnoneSimplification crash in and is a partial fix for Cherrypick swiftlang#76433
On Windows ARM64, how a struct value type is returned is sensitive to conditions including whether a user-defined constructor exists, etc. See https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-170#return-values That caused a calling convention mismatch between the non-USED_IN_CPP_SOURCE (Swift) side and the USE_IN_CPP_SOURCE (C++) side and a crash. Following swiftlang#76433 add constructors to several bridged C++ struct/class types so that the calling convention matches. This is a partial fix for swiftlang#74866 Cherrypick swiftlang#76589
c71fae1
to
36f450c
Compare
Please test with following PRs: @swift-ci please test |
8ce4710
to
36f450c
Compare
@rjmccall @eeckstein @hyp @compnerd This is a cherrypick of the fixes for the ARM64 compiler to |
This was referenced Oct 30, 2024
@DougGregor would you review this cherrypick of the fixes for the ARM64 compiler to release/6.0? The context is #74866. |
airspeedswift
approved these changes
Nov 14, 2024
DougGregor
approved these changes
Dec 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
arrangeCXXMethodCall
for C++ methods instead ofarrangeFreeFunctionCall
in the IRGen.