Skip to content

Commit

Permalink
[RISC-V] Transfer arguments between calling conventions in shuffling …
Browse files Browse the repository at this point in the history
…thunks (#107282)

* Log ShuffleEntries from GenerateShuffleArray

* Add failing tests for passing FP structs through shuffling thunks

* Report proper ShuffleEntries for lowered FP structs

* Implement shuffling thunk generation in tighter, more focused loops instead of an omnibus loop handling ShuffleEntries

* Generate ShuffleEntries for delowered arguments

* Better ShuffleEntry mask names, one more bit for field offset

* Fix FpStruct for dst arg loc

* Fold ShuffleEntry generation code for lowering and delowering FpStructs

* ShuffleEntry mask doc update

* Implement forward shuffling of floating registers and delowering of FpStructs. EmptyStructs test passes except for ShufflingThunk_FloatEmptyShort_DoubleFloatNestedEmpty_RiscV

* Fix shuffling of integer registers for member functions

* The delowered argument can also be put in the first stack slot

* Stask shuffling after delowered argument doesn't start with 0. Fixes Regressions/coreclr/GitHub_16833/Test16833

* Code cleanup, fewer indents

* Support second lowering

* Remove unused CondCode

* Handle stack slot shuffling to the right

* Add some stack slots to shuffle in the growing stack test case

* Fix Equals signature on test structs

* Remodel the shuffling with calling convention transfer to recognize the key points first, which simplifies code and solves some corner cases e.g. where we can't assume struct stack size by checking the size + offset of the last field

* Use helper functions in EmitComputedInstantiatingMethodStub

* Implement stack growing in shuffling thunk

* Use signed immediate in EmitSubImm to be consistent with EmitAddImm

* Use ABI register names in logs

* Remove LoadRegPair because it's not used

* Add logging for slli and lui

* Remove stack growing from hand-emitted shuffle thunks

* Minor FloatFloatEmpty test cleanup

* Implement IL shuffling thunks for cases where the stack is growing

* Test stack walking in frames laid by the IL shuffle thunk

* Add assert and comment in CreateILDelegateShuffleThunk

* Fix release build

* Fixes for static delegates from member methods

* Fix log and comment

* Remove EmitJumpAndLinkRegister because it's no longer used

* Use TransferredField.reg in delowering (cosmetic fix to restart CI)

* New stub type for delegate shuffle thunk so it doesn't go in multidelegate code paths

* Make Test_ShufflingThunk_MemberGrowsStack_RiscV harder by returning via buffer

* Explain lowering

* Fold 12-bit sign extension branch in EmitMovConstant

* Harden Test_ShufflingThunk_PackedEmptyUintEmptyFloat_PackedEmptyDouble to cover interleaving FP and int arguments

* Handle shuffles between calling conventions in IL stubs

* Update comments

* Don't use NewStub for IL stubs

* Fold some more duplicated code into SetupShuffleThunk

* Clean up unnecessary diffs

* IL shuffle thunk takes target function address from delegate object. Cache the generated thunk on DelegateEEClass

* Build target signature based on delegate signature instead of just using the signature from target method to retain type context

* Test calling instance and static methods via the same delegate type

* Simplify shuffle thunk caching on DelegateEEClass

* Clean up CreateILDelegateShuffleThunk

* Delete Windows X86 stack size check

* Remove #ifdefs around ILSTUB_DELEGATE_SHUFFLE_THUNK, fix typo in GetStubMethodName

* Fix DecRef'ing path when the IL thunk is already cached on DelegateEEClass

* Fix shuffle thunk destruction in EEClass::Destruct: properly handle IL shuffle thunks and call RemoveStubRange if m_pInstRetBuffCallStub was deleted

* Don't use RemoveStubRange in the destructor, make code for dereferencing shuffle thunk when caching fails the same as destructor

* Remove unused RemoveStubRange

* Cover IL shuffle thunks in ILStubManager::TraceManager

* Remove unused start, end arguments from RangeList::RemoveRanges[Worker]

* Update src/coreclr/vm/comdelegate.cpp

---------

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
  • Loading branch information
tomeksowi and jkotas authored Oct 9, 2024
1 parent 9d28160 commit 4f60693
Show file tree
Hide file tree
Showing 16 changed files with 964 additions and 297 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/inc/sigparser.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class SigParser
void
GetSignature(
PCCOR_SIGNATURE * pSig,
uint32_t * pcbSigSize)
uint32_t * pcbSigSize) const
{
*pSig = m_ptr;
*pcbSigSize = m_dwLen;
Expand Down
12 changes: 5 additions & 7 deletions src/coreclr/inc/utilcode.h
Original file line number Diff line number Diff line change
Expand Up @@ -3098,9 +3098,9 @@ class RangeList
return this->AddRangeWorker(start, end, id);
}

void RemoveRanges(void *id, const BYTE *start = NULL, const BYTE *end = NULL)
void RemoveRanges(void *id)
{
return this->RemoveRangesWorker(id, start, end);
return this->RemoveRangesWorker(id);
}

BOOL IsInRange(TADDR address, TADDR *pID = NULL)
Expand All @@ -3114,16 +3114,14 @@ class RangeList

// You can overload these two for synchronization (as LockedRangeList does)
virtual BOOL AddRangeWorker(const BYTE *start, const BYTE *end, void *id);
// If both "start" and "end" are NULL, then this method deletes all ranges with
// the given id (i.e. the original behaviour). Otherwise, it ignores the given
// id and deletes all ranges falling in the region [start, end).
virtual void RemoveRangesWorker(void *id, const BYTE *start = NULL, const BYTE *end = NULL);
// Deletes all ranges with the given id
virtual void RemoveRangesWorker(void *id);
#else
virtual BOOL AddRangeWorker(const BYTE *start, const BYTE *end, void *id)
{
return TRUE;
}
virtual void RemoveRangesWorker(void *id, const BYTE *start = NULL, const BYTE *end = NULL) { }
virtual void RemoveRangesWorker(void *id) { }
#endif // !DACCESS_COMPILE

virtual BOOL IsInRangeWorker(TADDR address, TADDR *pID = NULL);
Expand Down
21 changes: 3 additions & 18 deletions src/coreclr/utilcode/loaderheap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ BOOL RangeList::AddRangeWorker(const BYTE *start, const BYTE *end, void *id)
}
}

void RangeList::RemoveRangesWorker(void *id, const BYTE* start, const BYTE* end)
void RangeList::RemoveRangesWorker(void *id)
{
CONTRACTL
{
Expand All @@ -177,24 +177,9 @@ void RangeList::RemoveRangesWorker(void *id, const BYTE* start, const BYTE* end)

while (r < rEnd)
{
if (r->id != (TADDR)NULL)
if (r->id == (TADDR)id)
{
if (start != NULL)
{
_ASSERTE(end != NULL);

if (r->start >= (TADDR)start && r->start < (TADDR)end)
{
CONSISTENCY_CHECK_MSGF(r->end >= (TADDR)start &&
r->end <= (TADDR)end,
("r: %p start: %p end: %p", r, start, end));
r->id = (TADDR)NULL;
}
}
else if (r->id == (TADDR)id)
{
r->id = (TADDR)NULL;
}
r->id = (TADDR)NULL;
}

r++;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/callingconvention.h
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ class ArgIteratorTemplate : public ARGITERATOR_BASE
//------------------------------------------------------------
int GetNextOffset();

CorElementType GetArgType(TypeHandle *pTypeHandle = NULL)
CorElementType GetArgType(TypeHandle *pTypeHandle = NULL) const
{
LIMITED_METHOD_CONTRACT;
if (pTypeHandle != NULL)
Expand All @@ -652,7 +652,7 @@ class ArgIteratorTemplate : public ARGITERATOR_BASE
return m_argType;
}

int GetArgSize()
int GetArgSize() const
{
LIMITED_METHOD_CONTRACT;
return m_argSize;
Expand Down
29 changes: 13 additions & 16 deletions src/coreclr/vm/class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,25 +124,22 @@ void EEClass::Destruct(MethodTable * pOwningMT)
if (IsDelegate())
{
DelegateEEClass* pDelegateEEClass = (DelegateEEClass*)this;

if (pDelegateEEClass->m_pStaticCallStub)
for (Stub* pThunk : {pDelegateEEClass->m_pStaticCallStub, pDelegateEEClass->m_pInstRetBuffCallStub})
{
// Collect data to remove stub entry from StubManager if
// stub is deleted.
BYTE* entry = (BYTE*)pDelegateEEClass->m_pStaticCallStub->GetEntryPoint();
UINT length = pDelegateEEClass->m_pStaticCallStub->GetNumCodeBytes();

ExecutableWriterHolder<Stub> stubWriterHolder(pDelegateEEClass->m_pStaticCallStub, sizeof(Stub));
BOOL fStubDeleted = stubWriterHolder.GetRW()->DecRef();
if (fStubDeleted)
if (pThunk == nullptr)
continue;

_ASSERTE(pThunk->IsShuffleThunk());

if (pThunk->HasExternalEntryPoint()) // IL thunk
{
StubLinkStubManager::g_pManager->RemoveStubRange(entry, length);
pThunk->DecRef();
}
else
{
ExecutableWriterHolder<Stub> stubWriterHolder(pThunk, sizeof(Stub));
stubWriterHolder.GetRW()->DecRef();
}
}
if (pDelegateEEClass->m_pInstRetBuffCallStub)
{
ExecutableWriterHolder<Stub> stubWriterHolder(pDelegateEEClass->m_pInstRetBuffCallStub, sizeof(Stub));
stubWriterHolder.GetRW()->DecRef();
}
}

Expand Down
Loading

0 comments on commit 4f60693

Please sign in to comment.