Skip to content

Commit

Permalink
don't use gc heap for ret buffer
Browse files Browse the repository at this point in the history
  • Loading branch information
EgorBo committed Feb 1, 2025
1 parent ad7b829 commit e00043b
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ enum CorInfoHelpFunc
CORINFO_HELP_ASSIGN_REF, // universal helpers with F_CALL_CONV calling convention
CORINFO_HELP_CHECKED_ASSIGN_REF,
CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP, // Do the store, and ensure that the target was not in the heap.
CORINFO_HELP_ENSURE_NONHEAP, // Ensure that the target was not in the heap.

CORINFO_HELP_ASSIGN_BYREF,
CORINFO_HELP_BULK_WRITEBARRIER,
Expand Down
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 = { /* cc0e7adf-e397-40b6-9d14-a7149815c991 */
0xcc0e7adf,
0xe397,
0x40b6,
{0x9d, 0x14, 0xa7, 0x14, 0x98, 0x15, 0xc9, 0x91}
constexpr GUID JITEEVersionIdentifier = { /* bcc768c7-c29d-467d-89f6-f5adca5f2e59 */
0xbcc768c7,
0xc29d,
0x467d,
{0x89, 0xf6, 0xf5, 0xad, 0xca, 0x5f, 0x2e, 0x59}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/inc/jithelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@
DYNAMICJITHELPER(CORINFO_HELP_ASSIGN_REF, JIT_WriteBarrier, METHOD__NIL)
DYNAMICJITHELPER(CORINFO_HELP_CHECKED_ASSIGN_REF, JIT_CheckedWriteBarrier,METHOD__NIL)
JITHELPER(CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP, JIT_WriteBarrierEnsureNonHeapTarget,METHOD__NIL)
JITHELPER(CORINFO_HELP_ENSURE_NONHEAP, JIT_EnsureNonHeapTarget,METHOD__NIL)

DYNAMICJITHELPER(CORINFO_HELP_ASSIGN_BYREF, JIT_ByRefWriteBarrier,METHOD__NIL)
DYNAMICJITHELPER(CORINFO_HELP_BULK_WRITEBARRIER, NULL, METHOD__BUFFER__MEMCOPYGC)
Expand Down
46 changes: 44 additions & 2 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1024,8 +1024,20 @@ GenTree* Compiler::impStoreStructPtr(GenTree* destAddr, GenTree* value, unsigned
{
var_types type = value->TypeGet();
ClassLayout* layout = (type == TYP_STRUCT) ? value->GetLayout(this) : nullptr;
GenTree* store = gtNewStoreValueNode(type, layout, destAddr, value);
store = impStoreStruct(store, curLevel);

bool isRetBuffer = false;
if (!compIsForInlining() && destAddr->OperIs(GT_LCL_VAR))
{
isRetBuffer = destAddr->AsLclVar()->GetLclNum() == info.compRetBuffArg;
}

GenTree* store = gtNewStoreValueNode(type, layout, destAddr, value);
if (isRetBuffer)
{
store->gtFlags |= GTF_IND_TGT_NOT_HEAP;
}

store = impStoreStruct(store, curLevel);

return store;
}
Expand Down Expand Up @@ -3333,6 +3345,16 @@ void Compiler::impImportAndPushBox(CORINFO_RESOLVED_TOKEN* pResolvedToken)
StackEntry se = impPopStack();
GenTree* exprToBox = se.val;

if (varTypeIsStruct(exprToBox) && (exprToBox->IsCall() || exprToBox->OperIs(GT_RET_EXPR)))
{
unsigned callTmp = lvaGrabTemp(true DEBUGARG("spill struct call to tmp"));
CORINFO_CLASS_HANDLE cls = exprToBox->IsCall() ? exprToBox->AsCall()->gtRetClsHnd
: exprToBox->AsRetExpr()->gtInlineCandidate->gtRetClsHnd;
lvaSetStruct(callTmp, cls, false);
impStoreToTemp(callTmp, exprToBox, CHECK_SPILL_ALL);
exprToBox = gtNewLclVarNode(callTmp);
}

// Look at what helper we should use.
CorInfoHelpFunc boxHelper = info.compCompHnd->getBoxHelper(pResolvedToken->hClass);

Expand Down Expand Up @@ -6429,6 +6451,17 @@ void Compiler::impImportBlockCode(BasicBlock* block)
JITDUMP("\n [%2u] %3u (0x%03x) ", stackState.esStackDepth, impCurOpcOffs, impCurOpcOffs);
#endif

// CI testing. To be enabled only for jitstress
if (!opts.IsReadyToRun() && !compIsForInlining() && (info.compRetBuffArg != BAD_VAR_NUM) &&
(block == fgFirstBB))
{
// Write something into the buffer to make sure it's not on the gc heap.
// We write the value already in the buffer, so that we don't have to worry about
GenTree* retBuffAddr = gtNewLclvNode(info.compRetBuffArg, TYP_BYREF);
GenTreeCall* call = gtNewHelperCallNode(CORINFO_HELP_ENSURE_NONHEAP, TYP_VOID, retBuffAddr);
impAppendTree(call, CHECK_SPILL_NONE, impCurStmtDI);
}

DECODE_OPCODE:

// Return if any previous code has caused inline to fail.
Expand Down Expand Up @@ -9416,6 +9449,15 @@ void Compiler::impImportBlockCode(BasicBlock* block)
// Pull the value from the stack.
op2 = impPopStack().val;

// For STFLD(CALL) we need to spill the call, as the JIT
if (varTypeIsStruct(op2) && (op2->IsCall() || op2->OperIs(GT_RET_EXPR)))
{
unsigned callTmp = lvaGrabTemp(true DEBUGARG("spill struct call to tmp"));
lvaSetStruct(callTmp, fieldInfo.structType, false);
impStoreToTemp(callTmp, op2, CHECK_SPILL_ALL);
op2 = gtNewLclVarNode(callTmp);
}

if (opcode == CEE_STFLD)
{
obj = impPopStack().val;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1770,6 +1770,7 @@ void HelperCallProperties::init()
isNoGC = true;
FALLTHROUGH;
case CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP:
case CORINFO_HELP_ENSURE_NONHEAP:
case CORINFO_HELP_BULK_WRITEBARRIER:
mutatesHeap = true;
break;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ which is the right helper to use to allocate an object of a given type. */
CORINFO_HELP_ASSIGN_REF, // universal helpers with F_CALL_CONV calling convention
CORINFO_HELP_CHECKED_ASSIGN_REF,
CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP, // Do the store, and ensure that the target was not in the heap.
CORINFO_HELP_ENSURE_NONHEAP, // Ensure that the target was not in the heap.

CORINFO_HELP_ASSIGN_BYREF,
CORINFO_HELP_BULK_WRITEBARRIER,
Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/vm/gchelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1466,6 +1466,16 @@ extern "C" HCIMPL2_RAW(VOID, JIT_WriteBarrierEnsureNonHeapTarget, Object **dst,
}
HCIMPLEND_RAW

extern "C" HCIMPL1_RAW(VOID, JIT_EnsureNonHeapTarget, void *dst)
{
STATIC_CONTRACT_MODE_COOPERATIVE;
STATIC_CONTRACT_THROWS;
STATIC_CONTRACT_GC_NOTRIGGER;

assert(!GCHeapUtilities::GetGCHeap()->IsHeapPointer((void*)dst));
}
HCIMPLEND_RAW

// This function sets the card table with the granularity of 1 byte, to avoid ghost updates
// that could occur if multiple threads were trying to set different bits in the same card.

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/jitinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ extern "C" FCDECL2(VOID, JIT_CheckedWriteBarrier, Object **dst, Object *ref);

extern "C" FCDECL2(VOID, JIT_WriteBarrier, Object **dst, Object *ref);
extern "C" FCDECL2(VOID, JIT_WriteBarrierEnsureNonHeapTarget, Object **dst, Object *ref);
extern "C" FCDECL1(VOID, JIT_EnsureNonHeapTarget, void *dst);

// ARM64 JIT_WriteBarrier uses special ABI and thus is not callable directly
// Copied write barriers must be called at a different location
Expand Down
16 changes: 14 additions & 2 deletions src/coreclr/vm/reflectioninvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,10 @@ extern "C" void QCALLTYPE RuntimeMethodHandle_InvokeMethod(
// If an exception occurs a gc may happen but we are going to dump the stack anyway and we do
// not need to protect anything.

// Allocate a local buffer for the return buffer if necessary
DWORD objSize = retTH.GetMethodTable()->GetBaseSize();
PVOID pLocalRetBuf = _alloca(objSize);

{
BEGINFORBIDGC();
#ifdef _DEBUG
Expand All @@ -501,8 +505,10 @@ extern "C" void QCALLTYPE RuntimeMethodHandle_InvokeMethod(
// Take care of any return arguments
if (fHasRetBuffArg)
{
PVOID pRetBuff = gc.retVal->GetData();
*((LPVOID*) (pTransitionBlock + argit.GetRetBuffArgOffset())) = pRetBuff;
_ASSERT(hasValueTypeReturn);

memset(pLocalRetBuf, 0, objSize);
*((LPVOID*) (pTransitionBlock + argit.GetRetBuffArgOffset())) = pLocalRetBuf;
}

// copy args
Expand Down Expand Up @@ -572,6 +578,12 @@ extern "C" void QCALLTYPE RuntimeMethodHandle_InvokeMethod(
// Call the method
CallDescrWorkerWithHandler(&callDescrData);

if (fHasRetBuffArg)
{
// Copy the return value from the return buffer to the object
memmoveGCRefs(gc.retVal->GetData(), pLocalRetBuf, objSize);
}

// It is still illegal to do a GC here. The return type might have/contain GC pointers.
if (fConstructor)
{
Expand Down

0 comments on commit e00043b

Please sign in to comment.