Skip to content

Commit

Permalink
Revert "Inject IJW Copy Constructor calls in the JIT instead of in th…
Browse files Browse the repository at this point in the history
…e IL stubs (dotnet#106000)"

This reverts commit ec067c8.

Just being cautions -- the PR was merged before all tests had run.
  • Loading branch information
AndyAyersMS committed Aug 14, 2024
1 parent 7d2c465 commit f9b663b
Show file tree
Hide file tree
Showing 42 changed files with 755 additions and 1,197 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -344,12 +344,6 @@ 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: 69 additions & 0 deletions src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,75 @@ 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: 0 additions & 3 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,6 @@ 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 @@ -3326,8 +3325,6 @@ 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
3 changes: 0 additions & 3 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -742,9 +742,6 @@ 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 = { /* 62865a69-7c84-4ba5-8636-a7dec55c05a7 */
0x62865a69,
0x7c84,
0x4ba5,
{0x86, 0x36, 0xa7, 0xde, 0xc5, 0x5c, 0x05, 0xa7}
constexpr GUID JITEEVersionIdentifier = { /* f43f9022-8795-4791-ba55-c450d76cfeb9 */
0xf43f9022,
0x8795,
0x4791,
{0xba, 0x55, 0xc4, 0x50, 0xd7, 0x6c, 0xfe, 0xb9}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/ICorJitInfo_names_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,5 @@ 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: 0 additions & 9 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1742,15 +1742,6 @@ 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: 0 additions & 4 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1989,10 +1989,6 @@ 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: 0 additions & 30 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5703,36 +5703,6 @@ 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: 0 additions & 1 deletion src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16599,7 +16599,6 @@ 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
58 changes: 6 additions & 52 deletions src/coreclr/jit/gschecks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,59 +516,13 @@ void Compiler::gsParamsToShadows()
continue;
}

#if defined(TARGET_X86) && defined(FEATURE_IJW)
if (lclNum < info.compArgsCount && argRequiresSpecialCopy(lclNum) && (varDsc->TypeGet() == TYP_STRUCT))
{
JITDUMP("arg%02u requires special copy, using special copy helper to copy to shadow var V%02u\n", lclNum,
shadowVarNum);
CORINFO_METHOD_HANDLE copyHelper =
info.compCompHnd->getSpecialCopyHelper(varDsc->GetLayout()->GetClassHandle());
GenTreeCall* call = gtNewCallNode(CT_USER_FUNC, copyHelper, TYP_VOID);

GenTree* src = gtNewLclVarAddrNode(lclNum);
GenTree* dst = gtNewLclVarAddrNode(shadowVarNum);

call->gtArgs.PushBack(this, NewCallArg::Primitive(dst));
call->gtArgs.PushBack(this, NewCallArg::Primitive(src));

fgEnsureFirstBBisScratch();
compCurBB = fgFirstBB; // Needed by some morphing
if (opts.IsReversePInvoke())
{
JITDUMP(
"Inserting special copy helper call at the end of the first block after Reverse P/Invoke transition\n");

#ifdef DEBUG
// assert that we don't have any uses of the local variable in the first block
// before we insert the shadow copy statement.
for (Statement* const stmt : fgFirstBB->Statements())
{
assert(!gtHasRef(stmt->GetRootNode(), lclNum));
}
#endif
// If we are in a reverse P/Invoke, then we need to insert
// the call at the end of the first block as we need to do the GC transition
// before we can call the helper.
(void)fgNewStmtAtEnd(fgFirstBB, fgMorphTree(call));
}
else
{
JITDUMP("Inserting special copy helper call at the beginning of the first block\n");
(void)fgNewStmtAtBeg(fgFirstBB, fgMorphTree(call));
}
}
else
#endif // TARGET_X86 && FEATURE_IJW
{
GenTree* src = gtNewLclvNode(lclNum, varDsc->TypeGet());
src->gtFlags |= GTF_DONT_CSE;
GenTree* src = gtNewLclvNode(lclNum, varDsc->TypeGet());
src->gtFlags |= GTF_DONT_CSE;
GenTree* store = gtNewStoreLclVarNode(shadowVarNum, src);

GenTree* store = gtNewStoreLclVarNode(shadowVarNum, src);

fgEnsureFirstBBisScratch();
compCurBB = fgFirstBB; // Needed by some morphing
(void)fgNewStmtAtBeg(fgFirstBB, fgMorphTree(store));
}
fgEnsureFirstBBisScratch();
compCurBB = fgFirstBB; // Needed by some morphing
(void)fgNewStmtAtBeg(fgFirstBB, fgMorphTree(store));
}
compCurBB = nullptr;

Expand Down
13 changes: 0 additions & 13 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -653,19 +653,6 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo, unsigned skipArgs, un
CorInfoTypeWithMod corInfoType = info.compCompHnd->getArgType(&info.compMethodInfo->args, argLst, &typeHnd);
varDsc->lvIsParam = 1;

#if defined(TARGET_X86) && defined(FEATURE_IJW)
if ((corInfoType & CORINFO_TYPE_MOD_COPY_WITH_HELPER) != 0)
{
CorInfoType typeWithoutMod = strip(corInfoType);
if (typeWithoutMod == CORINFO_TYPE_VALUECLASS || typeWithoutMod == CORINFO_TYPE_PTR ||
typeWithoutMod == CORINFO_TYPE_BYREF)
{
JITDUMP("Marking user arg%02u as requiring special copy semantics\n", i);
recordArgRequiresSpecialCopy(i);
}
}
#endif // TARGET_X86 && FEATURE_IJW

lvaInitVarDsc(varDsc, varDscInfo->varNum, strip(corInfoType), typeHnd, argLst, &info.compMethodInfo->args);

if (strip(corInfoType) == CORINFO_TYPE_CLASS)
Expand Down
Loading

0 comments on commit f9b663b

Please sign in to comment.