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

Inject IJW Copy Constructor calls in the JIT instead of in the IL stubs #106424

Merged
merged 37 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
e3126aa
Revert "Disable GS checks in Reverse P/Invoke stubs to avoid missing …
jkoritzinsky Aug 1, 2024
0f5c835
Revert "Since the fix is in the CoreCLR VM side, we need to disable t…
jkoritzinsky Aug 1, 2024
6ae45a8
Propagate the modreq to the stub and into the GS cookie logic to avoi…
jkoritzinsky Aug 1, 2024
dc15fb2
Add RuntimeHelpers copy-constructor helper
jkoritzinsky Aug 2, 2024
bf92d9e
Implement Reverse P/Invoke GS shadow copy for copy constructors
jkoritzinsky Aug 2, 2024
110b29b
Use a jitinterface function instead of a jit helper so we don't need …
jkoritzinsky Aug 2, 2024
b2bb8fc
Remove extra P/Invoke copy constructor call
jkoritzinsky Aug 2, 2024
037ad6a
Propagate the "special-copy" modifier into the JIT for byref locals (…
jkoritzinsky Aug 2, 2024
c8fa45d
Move copy constructor handling into the JIT for arguments
jkoritzinsky Aug 5, 2024
52d4da9
Remove now-dead code
jkoritzinsky Aug 5, 2024
1e55410
Fix formatting
jkoritzinsky Aug 5, 2024
59c4aaf
Fix build on Windows x64
jkoritzinsky Aug 6, 2024
390185c
Call copy helper in lowering and heavily reduce changes in higher layers
jkoritzinsky Aug 6, 2024
6d5ce12
Revert "Call copy helper in lowering and heavily reduce changes in hi…
jkoritzinsky Aug 6, 2024
f76a582
Implement copy-constructor handling for P/Invokes via a map back to t…
jkoritzinsky Aug 6, 2024
761e648
Various PR feedback and propagate special copy directly from the IL a…
jkoritzinsky Aug 8, 2024
78d770e
Use a simple bool array instead of a vector as a function that has on…
jkoritzinsky Aug 8, 2024
0f0f244
Format
jkoritzinsky Aug 8, 2024
fa74b2a
Update hashing
jkoritzinsky Aug 8, 2024
dce5b11
Remove assert
jkoritzinsky Aug 8, 2024
416da37
Update Misc test
jkoritzinsky Aug 8, 2024
1287ae0
Cast through size_t to make it explicit that we're dropping the top bits
jkoritzinsky Aug 8, 2024
22a456b
Use compArgsCount and remove assert
jkoritzinsky Aug 8, 2024
bb7217b
Various fixes
jkoritzinsky Aug 9, 2024
8e977cc
Fix GCC build
jkoritzinsky Aug 12, 2024
850e78d
Fix signature type check
jkoritzinsky Aug 12, 2024
6ddfe1e
Merge branch 'main' of https://github.com/dotnet/runtime into copy-ct…
jkoritzinsky Aug 12, 2024
670c40d
Feedback
jkoritzinsky Aug 13, 2024
dc3754f
Add headers
jkoritzinsky Aug 14, 2024
b0ec55c
Fix superpmi build failures. Correctly handle multiple-arg copy const…
jkoritzinsky Aug 14, 2024
048b0c3
Add check and assert
jkoritzinsky Aug 14, 2024
8e68287
Fix superpmi build failures. Correctly handle multiple-arg copy const…
jkoritzinsky Aug 15, 2024
4371de2
When resolving the tokens for the stub, re-translate signatures that …
jkoritzinsky Aug 15, 2024
bb9fe1a
Instead of carrying around a token map, introduce a new kind of "inte…
jkoritzinsky Aug 16, 2024
1836497
Merge branch 'main' of https://github.com/dotnet/runtime into copy-ct…
jkoritzinsky Aug 16, 2024
a349c3c
Add support in cDAC for CModInternal
jkoritzinsky Aug 16, 2024
b15299e
PR feedback
jkoritzinsky Aug 21, 2024
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
1 change: 1 addition & 0 deletions docs/design/datacontracts/RuntimeTypeSystem.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ internal enum CorElementType
// Values defined in ECMA-335 - II.23.1.16 Element types used in signatures
// +
Internal = 0x21, // Indicates that the next pointer sized number of bytes is the address of a TypeHandle. Signatures that contain the Internal CorElementType cannot exist in metadata that is saved into a serialized format.
CModInternal = 0x22, // Indicates that the next byte specifies if the modifier is required and the next pointer sized number of bytes after that is the address of a TypeHandle. Signatures that contain the CModInternal CorElementType cannot exist in metadata that is saved into a seralized format.
}
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,11 @@ internal static int EnumCompareTo<T>(T x, T y) where T : struct, Enum
return x.CompareTo(y);
}

// The body of this function will be created by the EE for the specific type.
// See getILIntrinsicImplementation for how this happens.
[Intrinsic]
internal static extern unsafe void CopyConstruct<T>(T* dest, T* src) where T : unmanaged;

internal static ref byte GetRawData(this object obj) =>
ref Unsafe.As<RawData>(obj).Data;

Expand Down
69 changes: 0 additions & 69 deletions src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1315,75 +1315,6 @@ public IntPtr AddRef()
}
} // class CleanupWorkListElement

internal unsafe struct CopyConstructorCookie
{
private void* m_source;

private nuint m_destinationOffset;

public delegate*<void*, void*, void> m_copyConstructor;

public delegate*<void*, void> m_destructor;

public CopyConstructorCookie* m_next;

[StackTraceHidden]
public void ExecuteCopy(void* destinationBase)
{
if (m_copyConstructor != null)
{
m_copyConstructor((byte*)destinationBase + m_destinationOffset, m_source);
}

if (m_destructor != null)
{
m_destructor(m_source);
}
}
}

internal unsafe struct CopyConstructorChain
{
public void* m_realTarget;
public CopyConstructorCookie* m_head;

public void Add(CopyConstructorCookie* cookie)
{
cookie->m_next = m_head;
m_head = cookie;
}

[ThreadStatic]
private static CopyConstructorChain s_copyConstructorChain;

public void Install(void* realTarget)
{
m_realTarget = realTarget;
s_copyConstructorChain = this;
}

[StackTraceHidden]
private void ExecuteCopies(void* destinationBase)
{
for (CopyConstructorCookie* current = m_head; current != null; current = current->m_next)
{
current->ExecuteCopy(destinationBase);
}
}

[UnmanagedCallersOnly]
[StackTraceHidden]
public static void* ExecuteCurrentCopiesAndGetTarget(void* destinationBase)
{
void* target = s_copyConstructorChain.m_realTarget;
s_copyConstructorChain.ExecuteCopies(destinationBase);
// Reset this instance to ensure we don't accidentally execute the copies again.
// All of the pointers point to the stack, so we don't need to free any memory.
s_copyConstructorChain = default;
return target;
}
}

internal static partial class StubHelpers
{
[MethodImpl(MethodImplOptions.InternalCall)]
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/ildasm/dasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2088,6 +2088,9 @@ BYTE* PrettyPrintCABlobValue(PCCOR_SIGNATURE &typePtr,

#ifdef LOGGING
case ELEMENT_TYPE_INTERNAL :
case ELEMENT_TYPE_CMOD_INTERNAL :
typePtr += 1;
Reiterate = TRUE;
#endif // LOGGING
return NULL;

Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/inc/corhdr.h
Original file line number Diff line number Diff line change
Expand Up @@ -911,9 +911,10 @@ typedef enum CorElementType

// This is for signatures generated internally (which will not be persisted in any way).
ELEMENT_TYPE_INTERNAL = 0x21, // INTERNAL <typehandle>
ELEMENT_TYPE_CMOD_INTERNAL = 0x22, // CMOD_INTERNAL <required (1 byte: non-zero if required, 0 if optional)> <typehandle>

// Note that this is the max of base type excluding modifiers
ELEMENT_TYPE_MAX = 0x22, // first invalid element type
ELEMENT_TYPE_MAX = 0x23, // first invalid element type


ELEMENT_TYPE_MODIFIER = 0x40,
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,7 @@ enum CorInfoTypeWithMod
{
CORINFO_TYPE_MASK = 0x3F, // lower 6 bits are type mask
CORINFO_TYPE_MOD_PINNED = 0x40, // can be applied to CLASS, or BYREF to indicate pinned
CORINFO_TYPE_MOD_COPY_WITH_HELPER = 0x80 // can be applied to VALUECLASS to indicate 'needs helper to copy'
};

inline CorInfoType strip(CorInfoTypeWithMod val) {
Expand Down Expand Up @@ -3325,6 +3326,8 @@ class ICorDynamicInfo : public ICorStaticInfo
// but for tailcalls, the contract is that JIT leaves the indirection cell in
// a register during tailcall.
virtual void updateEntryPointForTailCall(CORINFO_CONST_LOOKUP* entryPoint) = 0;

virtual CORINFO_METHOD_HANDLE getSpecialCopyHelper(CORINFO_CLASS_HANDLE type) = 0;
};

/**********************************************************************************/
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/inc/cortypeinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,4 @@ TYPEINFO(ELEMENT_TYPE_MVAR, NULL, NULL, TARGET_POINTER_SI
TYPEINFO(ELEMENT_TYPE_CMOD_REQD, NULL, NULL, 0, TYPE_GC_NONE, false, false, false, false, false) // 0x1f
TYPEINFO(ELEMENT_TYPE_CMOD_OPT, NULL, NULL, 0, TYPE_GC_NONE, false, false, false, false, false) // 0x20
TYPEINFO(ELEMENT_TYPE_INTERNAL, NULL, NULL, 0, TYPE_GC_OTHER, false, false, false, false, false) // 0x21
TYPEINFO(ELEMENT_TYPE_CMOD_INTERNAL,NULL, NULL, 0, TYPE_GC_NONE, false, false, false, false, false) // 0x22
38 changes: 38 additions & 0 deletions src/coreclr/inc/formattype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,44 @@ PCCOR_SIGNATURE PrettyPrintType(
appendStr(out, sz);
break;
}
case ELEMENT_TYPE_CMOD_INTERNAL :
{
// ELEMENT_TYPE_CMOD_INTERNAL <required> <TypeHandle>
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
bool required = *typePtr++ != 0;
_ASSERTE(sizeof(TypeHandle) == sizeof(void *));
TypeHandle typeHandle;
typePtr += CorSigUncompressPointer(typePtr, (void **)&typeHandle);

MethodTable *pMT = NULL;
if (typeHandle.IsTypeDesc())
{
pMT = typeHandle.AsTypeDesc()->GetMethodTable();
if (pMT)
{
PrettyPrintClass(out, pMT->GetCl(), pMT->GetMDImport());

// It could be a "native version" of the managed type used in interop
if (typeHandle.AsTypeDesc()->IsNativeValueType())
appendStr(out, "_NativeValueType");
}
else
appendStr(out, "(null)");
}
else
{
pMT = typeHandle.AsMethodTable();
if (pMT)
PrettyPrintClass(out, pMT->GetCl(), pMT->GetMDImport());
else
appendStr(out, "(null)");
}

const char fmt[] = " mod%s(/* MT: %p */)";
char sz[Max64BitHexString + ARRAY_SIZE(fmt) + ARRAY_SIZE("req")];
sprintf_s(sz, ARRAY_SIZE(sz), fmt, required ? "req" : "opt", pMT);
appendStr(out, sz);
break;
}
#endif


Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,9 @@ uint32_t getJitFlags(
CORJIT_FLAGS* flags,
uint32_t sizeInBytes) override;

CORINFO_METHOD_HANDLE getSpecialCopyHelper(
CORINFO_CLASS_HANDLE type) override;

/**********************************************************************************/
// clang-format on
/**********************************************************************************/
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* f43f9022-8795-4791-ba55-c450d76cfeb9 */
0xf43f9022,
0x8795,
0x4791,
{0xba, 0x55, 0xc4, 0x50, 0xd7, 0x6c, 0xfe, 0xb9}
constexpr GUID JITEEVersionIdentifier = { /* 62865a69-7c84-4ba5-8636-a7dec55c05a7 */
0x62865a69,
0x7c84,
0x4ba5,
{0x86, 0x36, 0xa7, 0xde, 0xc5, 0x5c, 0x05, 0xa7}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
57 changes: 41 additions & 16 deletions src/coreclr/inc/sigparser.h
Original file line number Diff line number Diff line change
Expand Up @@ -582,19 +582,32 @@ class SigParser
return hr;

while ((ELEMENT_TYPE_CMOD_REQD == bElementType) ||
(ELEMENT_TYPE_CMOD_OPT == bElementType))
(ELEMENT_TYPE_CMOD_OPT == bElementType) ||
(ELEMENT_TYPE_CMOD_INTERNAL == bElementType))
{
sigTemp.SkipBytes(1);
if (ELEMENT_TYPE_CMOD_INTERNAL == bElementType)
{
void * pMT;
// If this custom modifier is required or optional
uint8_t required;
if (FAILED(hr = sigTemp.GetByte(&required)))
return hr;

if (FAILED(hr = sigTemp.GetPointer(&pMT)))
return hr;
}
else
{
mdToken token;

mdToken token;

hr = sigTemp.GetToken(&token);
hr = sigTemp.GetToken(&token);

if (FAILED(hr))
return hr;
if (FAILED(hr))
return hr;
}

hr = sigTemp.PeekByte(&bElementType);
if (FAILED(hr))
if (FAILED(hr = sigTemp.PeekByte(&bElementType)))
return hr;
}

Expand Down Expand Up @@ -643,19 +656,31 @@ class SigParser
while (ELEMENT_TYPE_CMOD_REQD == bElementType ||
ELEMENT_TYPE_CMOD_OPT == bElementType ||
ELEMENT_TYPE_MODIFIER == bElementType ||
ELEMENT_TYPE_PINNED == bElementType)
ELEMENT_TYPE_PINNED == bElementType ||
ELEMENT_TYPE_CMOD_INTERNAL == bElementType)
{
sigTemp.SkipBytes(1);
if (ELEMENT_TYPE_CMOD_INTERNAL == bElementType)
{
void * pMT;
uint8_t required;
if (FAILED(hr = sigTemp.GetByte(&required)))
return hr;

if (FAILED(hr = sigTemp.GetPointer(&pMT)))
return hr;
}
else
{
mdToken token;

mdToken token;

hr = sigTemp.GetToken(&token);
hr = sigTemp.GetToken(&token);

if (FAILED(hr))
return hr;
if (FAILED(hr))
return hr;
}

hr = sigTemp.PeekByte(&bElementType);
if (FAILED(hr))
if (FAILED(hr = sigTemp.PeekByte(&bElementType)))
return hr;
}

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/ICorJitInfo_names_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,5 +180,6 @@ DEF_CLR_API(recordRelocation)
DEF_CLR_API(getRelocTypeHint)
DEF_CLR_API(getExpectedTargetArchitecture)
DEF_CLR_API(getJitFlags)
DEF_CLR_API(getSpecialCopyHelper)

#undef DEF_CLR_API
9 changes: 9 additions & 0 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1742,6 +1742,15 @@ uint32_t WrapICorJitInfo::getJitFlags(
return temp;
}

CORINFO_METHOD_HANDLE WrapICorJitInfo::getSpecialCopyHelper(
CORINFO_CLASS_HANDLE type)
{
API_ENTER(getSpecialCopyHelper);
CORINFO_METHOD_HANDLE temp = wrapHnd->getSpecialCopyHelper(type);
API_LEAVE(getSpecialCopyHelper);
return temp;
}

/**********************************************************************************/
// clang-format on
/**********************************************************************************/
4 changes: 4 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1989,6 +1989,10 @@ void Compiler::compInit(ArenaAllocator* pAlloc,
m_fpStructLoweringCache = nullptr;
#endif

#if defined(TARGET_X86) && defined(FEATURE_IJW)
m_specialCopyArgs = nullptr;
#endif

// check that HelperCallProperties are initialized

assert(s_helperCallProperties.IsPure(CORINFO_HELP_GET_GCSTATIC_BASE));
Expand Down
30 changes: 30 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5703,6 +5703,36 @@ class Compiler
const CORINFO_SWIFT_LOWERING* GetSwiftLowering(CORINFO_CLASS_HANDLE clsHnd);
#endif

#if defined(TARGET_X86) && defined(FEATURE_IJW)
bool* m_specialCopyArgs;
bool recordArgRequiresSpecialCopy(unsigned argNum)
{
if (argNum >= info.compArgsCount)
{
return false;
}

if (m_specialCopyArgs == nullptr)
{
m_specialCopyArgs = new (getAllocator()) bool[info.compArgsCount];
memset(m_specialCopyArgs, 0, info.compArgsCount * sizeof(bool));
}

m_specialCopyArgs[argNum] = true;
return true;
}

bool argRequiresSpecialCopy(unsigned argNum)
{
return argNum < info.compArgsCount && m_specialCopyArgs != nullptr && m_specialCopyArgs[argNum];
}

bool compHasSpecialCopyArgs()
{
return m_specialCopyArgs != nullptr;
}
#endif

void optRecordLoopMemoryDependence(GenTree* tree, BasicBlock* block, ValueNum memoryVN);
void optCopyLoopMemoryDependence(GenTree* fromTree, GenTree* toTree);

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16599,6 +16599,7 @@ GenTree* Compiler::gtNewTempStore(
valTyp = lvaGetRealType(val->AsLclVar()->GetLclNum());
val->gtType = valTyp;
}

var_types dstTyp = varDsc->TypeGet();

/* If the variable's lvType is not yet set then set it here */
Expand Down
Loading
Loading