-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Disable Int128 use in by value ABI scenarios, and fix field layout behavior #74123
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
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
29ea888
First stab at support for proper 128bit integer layout and abi
davidwrighton ca40e03
Add ABI tests for Int128 covering interesting scenarios
davidwrighton 183a36e
Fix bugs so that at least Windows Arm64 works
davidwrighton 823090b
Add more types to the ABI tester, so that we cover the Int128 scenarios
davidwrighton 22adce8
Revert changes which attempted to enable by value passing for Int128
davidwrighton e36292f
Make Int128 have layout match the expected unmanaged field layout
davidwrighton 867ab2e
Mark Int128 types as not having a stable abi
davidwrighton 8585002
Address all known issues
davidwrighton fdba562
Merge branch 'main' of github.com:dotnet/runtime into fix_72206_2
davidwrighton 4fab4d3
Try to fix PR job
davidwrighton 1170afe
Should fix the test issues
davidwrighton 81c4439
Apply suggestions from code review
davidwrighton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9907,21 +9907,6 @@ void MethodTableBuilder::CheckForSystemTypes() | |
|
|
||
| return; | ||
| } | ||
| #if defined(UNIX_AMD64_ABI) || defined(TARGET_ARM64) | ||
| else if (strcmp(nameSpace, g_SystemNS) == 0) | ||
| { | ||
| EEClassLayoutInfo* pLayout = pClass->GetLayoutInfo(); | ||
|
|
||
| // These types correspond to fundamental data types in the underlying ABIs: | ||
| // * Int128: __int128 | ||
| // * UInt128: unsigned __int128 | ||
|
|
||
| if ((strcmp(name, g_Int128Name) == 0) || (strcmp(name, g_UInt128Name) == 0)) | ||
| { | ||
| pLayout->m_ManagedLargestAlignmentRequirementOfAllMembers = 16; // sizeof(__int128) | ||
| } | ||
| } | ||
| #endif // UNIX_AMD64_ABI || TARGET_ARM64 | ||
| } | ||
|
|
||
| if (g_pNullableClass != NULL) | ||
|
|
@@ -10005,6 +9990,29 @@ void MethodTableBuilder::CheckForSystemTypes() | |
| { | ||
| pMT->SetInternalCorElementType (ELEMENT_TYPE_I); | ||
| } | ||
| else if ((strcmp(name, g_Int128Name) == 0) || (strcmp(name, g_UInt128Name) == 0)) | ||
| { | ||
| EEClassLayoutInfo* pLayout = pClass->GetLayoutInfo(); | ||
| #ifdef TARGET_ARM | ||
| // No such type exists for the Procedure Call Standard for ARM. We will default | ||
| // to the same alignment as __m128, which is supported by the ABI. | ||
|
|
||
| pLayout->m_ManagedLargestAlignmentRequirementOfAllMembers = 8; | ||
| #elif defined(TARGET_64BIT) || defined(TARGET_X86) | ||
|
|
||
| // These types correspond to fundamental data types in the underlying ABIs: | ||
| // * Int128: __int128 | ||
| // * UInt128: unsigned __int128 | ||
| // | ||
| // This behavior matches the ABI standard on various Unix platforms | ||
| // On Windows, no standard for Int128 has been established yet, | ||
| // although applying 16 byte alignment is consistent with treatment of 128 bit SSE types | ||
| // even on X86 | ||
|
Comment on lines
+10009
to
+10011
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth noting that the MSVC STL does have a private Nothing official/documented of course, but it does help justify using |
||
| pLayout->m_ManagedLargestAlignmentRequirementOfAllMembers = 16; // sizeof(__int128) | ||
| #else | ||
| #error Unknown architecture | ||
| #endif // TARGET_64BIT | ||
| } | ||
| } | ||
| else | ||
| { | ||
|
|
||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.