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.

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

Replace HELPER_METHOD_FRAME with DynamicHelperFrame in patchpoints #112025

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Replace HELPER_METHOD_FRAME with DynamicHelperFrame in patchpoints #112025

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
01c2a18
Replace HELPER_METHOD_FRAME with DynamicHelperFrame in JIT_Patchpoint…
davidwrighton Jan 31, 2025
ac49b21
Attempt 2 at the asm helpers, hopefully fewer silly mistakes
davidwrighton Jan 31, 2025
c1b79ab
Fix Windows X86 build
davidwrighton Jan 31, 2025
483aedb
Fixes for issues found in CI as well as some corrections to assembly …
davidwrighton Jan 31, 2025
b10459b
Refactor the logic for the patchpoints to extract the two policies in…
davidwrighton Feb 3, 2025
f87e4b9
Fix issue where we didn't update the m_osrMethodCode field in the ppInfo
davidwrighton Feb 3, 2025
f5208bf
Hopefully final round of code review feedback
davidwrighton Feb 5, 2025
4f7e68c
Apply suggestions from code review
jkotas Feb 5, 2025
36a8db0
Update src/coreclr/vm/jithelpers.cpp
jkotas Feb 5, 2025
034df27
Update src/coreclr/vm/jithelpers.cpp
jkotas Feb 5, 2025
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
19 changes: 19 additions & 0 deletions src/coreclr/vm/amd64/AsmHelpers.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,25 @@ NESTED_ENTRY OnCallCountThresholdReachedStub, _TEXT
TAILJMP_RAX
NESTED_END OnCallCountThresholdReachedStub, _TEXT

extern JIT_PatchpointWorkerWorkerWithPolicy:proc

NESTED_ENTRY JIT_Patchpoint, _TEXT
PROLOG_WITH_TRANSITION_BLOCK

lea rcx, [rsp + __PWTB_TransitionBlock] ; TransitionBlock *
call JIT_PatchpointWorkerWorkerWithPolicy

EPILOG_WITH_TRANSITION_BLOCK_RETURN
TAILJMP_RAX
NESTED_END JIT_Patchpoint, _TEXT

; first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL
LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we can remove the need of separateJIT_PartialCompilationPatchpoint if we update its only usage:

runtime/src/coreclr/jit/patchpoint.cpp

Lines 241 to 242 in d8a6117

GenTreeCall* helperCall =
compiler->gtNewHelperCallNode(CORINFO_HELP_PARTIAL_COMPILATION_PATCHPOINT, TYP_VOID, ilOffsetNode);

to be:

GenTree* nullCounter = compiler->gtNewIconNode(0, TYP_I_IMPL);
GenTreeCall* helperCall =
    compiler->gtNewHelperCallNode(CORINFO_HELP_PATCHPOINT, TYP_VOID, nullCounter, ilOffsetNode);

mov rdx, rcx
xor rcx, rcx
jmp JIT_Patchpoint
LEAF_END JIT_PartialCompilationPatchpoint, _TEXT

endif ; FEATURE_TIERED_COMPILATION

LEAF_ENTRY JIT_PollGC, _TEXT
Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/vm/amd64/unixasmhelpers.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,20 @@ NESTED_ENTRY OnCallCountThresholdReachedStub, _TEXT, NoHandler
TAILJMP_RAX
NESTED_END OnCallCountThresholdReachedStub, _TEXT

NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler
PROLOG_WITH_TRANSITION_BLOCK

lea rdi, [rsp + __PWTB_TransitionBlock] // TransitionBlock *
call C_FUNC(JIT_PatchpointWorkerWorkerWithPolicy)

EPILOG_WITH_TRANSITION_BLOCK_RETURN
NESTED_END JIT_Patchpoint, _TEXT

// first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL
LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT
mov rsi, rdi
xor rdi, rdi
jmp C_FUNC(JIT_Patchpoint)
LEAF_END JIT_PartialCompilationPatchpoint, _TEXT

#endif // FEATURE_TIERED_COMPILATION
16 changes: 16 additions & 0 deletions src/coreclr/vm/arm64/asmhelpers.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,22 @@ NESTED_ENTRY OnCallCountThresholdReachedStub, _TEXT, NoHandler
EPILOG_BRANCH_REG x9
NESTED_END OnCallCountThresholdReachedStub, _TEXT

NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler
PROLOG_WITH_TRANSITION_BLOCK

add x0, sp, #__PWTB_TransitionBlock // TransitionBlock *
bl C_FUNC(JIT_PatchpointWorkerWorkerWithPolicy)

EPILOG_WITH_TRANSITION_BLOCK_RETURN
NESTED_END JIT_Patchpoint, _TEXT

// first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL
LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT
mov x1, x0
mov x0, #0
b C_FUNC(JIT_Patchpoint)
LEAF_END JIT_PartialCompilationPatchpoint, _TEXT

#endif // FEATURE_TIERED_COMPILATION

LEAF_ENTRY JIT_ValidateIndirectCall, _TEXT
Expand Down
18 changes: 18 additions & 0 deletions src/coreclr/vm/arm64/asmhelpers.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,24 @@ __HelperNakedFuncName SETS "$helper":CC:"Naked"
EPILOG_BRANCH_REG x9
NESTED_END

IMPORT JIT_PatchpointWorkerWorkerWithPolicy

NESTED_ENTRY JIT_Patchpoint
PROLOG_WITH_TRANSITION_BLOCK

add x0, sp, #__PWTB_TransitionBlock ; TransitionBlock *
bl JIT_PatchpointWorkerWorkerWithPolicy

EPILOG_WITH_TRANSITION_BLOCK_RETURN
NESTED_END

// first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL
LEAF_ENTRY JIT_PartialCompilationPatchpoint
mov x1, x0
mov x0, #0
b JIT_Patchpoint
LEAF_END

#endif ; FEATURE_TIERED_COMPILATION

LEAF_ENTRY JIT_ValidateIndirectCall
Expand Down
24 changes: 24 additions & 0 deletions src/coreclr/vm/callingconvention.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2209,4 +2209,28 @@ inline BOOL IsRetBuffPassedAsFirstArg()
#endif
}

inline TADDR GetFirstArgumentRegisterValuePtr(TransitionBlock * pTransitionBlock)
{
TADDR pArgument = (TADDR)pTransitionBlock + TransitionBlock::GetOffsetOfArgumentRegisters();
#ifdef TARGET_X86
// x86 is special as always
pArgument += offsetof(ArgumentRegisters, ECX);
#endif

return pArgument;
}

inline TADDR GetSecondArgumentRegisterValuePtr(TransitionBlock * pTransitionBlock)
{
TADDR pArgument = (TADDR)pTransitionBlock + TransitionBlock::GetOffsetOfArgumentRegisters();
#ifdef TARGET_X86
// x86 is special as always
pArgument += offsetof(ArgumentRegisters, EDX);
#else
pArgument += sizeof(TADDR);
#endif

return pArgument;
}

#endif // __CALLING_CONVENTION_INCLUDED
8 changes: 4 additions & 4 deletions src/coreclr/vm/codeman.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2477,7 +2477,7 @@ class EECodeInfo

TADDR GetSavedMethodCode();

TADDR GetStartAddress();
TADDR GetStartAddress() const;

BOOL IsValid()
{
Expand Down Expand Up @@ -2505,15 +2505,15 @@ class EECodeInfo
}

// This returns a pointer to the start of an instruction; conceptually, a PINSTR.
TADDR GetCodeAddress()
TADDR GetCodeAddress() const
{
LIMITED_METHOD_DAC_CONTRACT;
return PCODEToPINSTR(m_codeAddress);
}

NativeCodeVersion GetNativeCodeVersion();
NativeCodeVersion GetNativeCodeVersion() const;

MethodDesc * GetMethodDesc()
MethodDesc * GetMethodDesc() const
{
LIMITED_METHOD_DAC_CONTRACT;
return m_pMD;
Expand Down
Loading
Loading