Skip to content

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 4 commits into from
Dec 2, 2024

Conversation

hjyamauchi
Copy link
Contributor

@hjyamauchi hjyamauchi commented Oct 28, 2024

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
@hjyamauchi
Copy link
Contributor Author

Please test with following PRs:
swiftlang/llvm-project#9476

@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
@hjyamauchi
Copy link
Contributor Author

Please test with following PRs:
swiftlang/llvm-project#9476

@swift-ci please test

@hjyamauchi hjyamauchi force-pushed the release/6.0 branch 2 times, most recently from 8ce4710 to 36f450c Compare October 29, 2024 19:20
@hjyamauchi hjyamauchi marked this pull request as ready for review October 30, 2024 17:00
@hjyamauchi hjyamauchi requested a review from a team as a code owner October 30, 2024 17:00
@hjyamauchi
Copy link
Contributor Author

@rjmccall @eeckstein @hyp @compnerd This is a cherrypick of the fixes for the ARM64 compiler to release/6.0. Can you review?

@hjyamauchi
Copy link
Contributor Author

@DougGregor would you review this cherrypick of the fixes for the ARM64 compiler to release/6.0? The context is #74866.

@DougGregor DougGregor merged commit 6d3eed5 into swiftlang:release/6.0 Dec 2, 2024
5 checks passed
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