Skip to content

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.

Sign up for GitHub

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
davidwrighton merged 12 commits into from
Aug 19, 2022
Merged

Disable Int128 use in by value ABI scenarios, and fix field layout behavior #74123

Show file tree
Hide file tree
Changes from 1 commit
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 Aug 17, 2022
ca40e03
Add ABI tests for Int128 covering interesting scenarios
davidwrighton Aug 9, 2022
183a36e
Fix bugs so that at least Windows Arm64 works
davidwrighton Aug 17, 2022
823090b
Add more types to the ABI tester, so that we cover the Int128 scenarios
davidwrighton Aug 18, 2022
22adce8
Revert changes which attempted to enable by value passing for Int128
davidwrighton Aug 18, 2022
e36292f
Make Int128 have layout match the expected unmanaged field layout
davidwrighton Aug 18, 2022
867ab2e
Mark Int128 types as not having a stable abi
davidwrighton Aug 18, 2022
8585002
Address all known issues
davidwrighton Aug 19, 2022
fdba562
Merge branch 'main' of github.com:dotnet/runtime into fix_72206_2
davidwrighton Aug 19, 2022
4fab4d3
Try to fix PR job
davidwrighton Aug 19, 2022
1170afe
Should fix the test issues
davidwrighton Aug 19, 2022
81c4439
Apply suggestions from code review
davidwrighton Aug 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix bugs so that at least Windows Arm64 works
  • Loading branch information
@davidwrighton
davidwrighton committed Aug 18, 2022
commit 183a36e5d3fc725e01415ae073d09ffd9c7d0f37
2 changes: 1 addition & 1 deletion src/coreclr/jit/lclvars.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6337,7 +6337,7 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum,
unsigned argAlignment = eeGetArgSizeAlignment(varDsc->lvType, isFloatHfa);

#if TARGET_ARM64
if (!info.compIsVarArgs && varDsc->lvSize() == 16 && varDsc->lvType == TYP_STRUCT && !isFloatHfa && info.compCompHnd->getClassAlignmentRequirement(varDsc->GetStructHnd()) == 16)
if (!info.compIsVarArgs && varDsc->lvType == TYP_STRUCT && varDsc->lvSize() == 16 && !isFloatHfa && info.compCompHnd->getClassAlignmentRequirement(varDsc->GetStructHnd()) == 16)
{
// TODO-Cleanup: use "eeGetArgSizeAlignment" here. See also: https://github.com/dotnet/runtime/issues/46026.
argAlignment = 16;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp

ComputedInstanceFieldLayout layoutFromMetadata = _fallbackAlgorithm.ComputeInstanceLayout(defType, layoutKind);

if (defType.Context.Target.IsWindows || (defType.Context.Target.PointerSize == 4))
if (defType.Context.Target.Architecture != TargetArchitecture.ARM64 && defType.Context.Target.IsWindows || (defType.Context.Target.PointerSize == 4))
{
return layoutFromMetadata;
}
Expand Down Expand Up @@ -72,7 +72,7 @@ public override ValueTypeShapeCharacteristics ComputeValueTypeShapeCharacteristi
public static bool IsIntegerType(DefType type)
{
return type.IsIntrinsic
&& type.Namespace == "System."
&& type.Namespace == "System"
&& ((type.Name == "Int128") || (type.Name == "UInt128"));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class TestTypeSystemContext : MetadataTypeSystemContext
public CanonicalizationMode CanonMode { get; set; } = CanonicalizationMode.RuntimeDetermined;

public TestTypeSystemContext(TargetArchitecture arch, TargetOS targetOS = TargetOS.Unknown)
: base(new TargetDetails(arch, TargetOS.Unknown, TargetAbi.Unknown))
: base(new TargetDetails(arch, targetOS, TargetAbi.Unknown))
{
_vectorFieldLayoutAlgorithm = new VectorFieldLayoutAlgorithm(_metadataFieldLayout, true);
_int128FieldLayoutAlgorithm = new Int128FieldLayoutAlgorithm(_metadataFieldLayout);
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/vm/callingconvention.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1575,10 +1575,9 @@ int ArgIteratorTemplate<ARGITERATOR_BASE>::GetNextOffset()
// If the argument has an alignment of 16 then the NGRN is rounded up to the next even number.
//
// This rule is not used for Apple platforms. See https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms
if (!this->IsVarArg() && ((_arm64IdxGenReg & 1) == 1) && regSlots == 2 && thValueType.GetMethodTable()->GetFieldAlignmentRequirement() == 16)
if (!this->IsVarArg() && ((m_idxGenReg & 1) == 1) && regSlots == 2 && thValueType.GetMethodTable()->GetFieldAlignmentRequirement() == 16)
{
m_idxGenReg++;
numRegistersUsed++;
}
#endif

Expand Down
8 changes: 6 additions & 2 deletions src/tests/Interop/PInvoke/Int128/Int128Native.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,14 @@
#elif defined(__SIZEOF_INT128__)
typedef __int128 Int128;
#else
typedef struct {
struct
#ifdef _M_ARM64
alignas(16)
#endif
Int128 {
uint64_t lower;
uint64_t upper;
} Int128;
};
#endif

static Int128 Int128Value = { };
Expand Down