Skip to content

[LoongArch64] Enable Runtime Async. (issue#124935)#125114

Draft
LuckyXu-HF wants to merge 1 commit intodotnet:mainfrom
LuckyXu-HF:main-LA64-async
Draft

[LoongArch64] Enable Runtime Async. (issue#124935)#125114
LuckyXu-HF wants to merge 1 commit intodotnet:mainfrom
LuckyXu-HF:main-LA64-async

Conversation

@LuckyXu-HF
Copy link
Contributor

  • Contributes to [LoongArch] Enable Runtime Async #124935
  • Fix up hijacking on loongarch64 (preserve async continuation register).
  • Revert lvaAsyncExecutionContextVar, lvaAsyncSynchronizationContextVar related offset according to LA64's frame layout.

* Enable Runtime Async of LoongArch64  dotnet#124935
* Fix up hijacking on loongarch64 (preserve async continuation register).
* Revert `lvaAsyncExecutionContextVar, lvaAsyncSynchronizationContextVar` related offset according to LA64's frame layout.
Comment on lines -4351 to -4364
if ((lvaAsyncExecutionContextVar != BAD_VAR_NUM) && !opts.IsOSR())
{
int offset = lvaTable[lvaAsyncExecutionContextVar].GetStackOffset() + delta;
lvaTable[lvaAsyncExecutionContextVar].SetStackOffset(offset);
delta += lvaLclStackHomeSize(lvaAsyncExecutionContextVar);
}

if ((lvaAsyncSynchronizationContextVar != BAD_VAR_NUM) && !opts.IsOSR())
{
int offset = lvaTable[lvaAsyncSynchronizationContextVar].GetStackOffset() + delta;
lvaTable[lvaAsyncSynchronizationContextVar].SetStackOffset(offset);
delta += lvaLclStackHomeSize(lvaAsyncSynchronizationContextVar);
}

Copy link
Member

Choose a reason for hiding this comment

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

For EnC to work it is necessary that this is allocated in the top of the stack frame and included as part of the EnC frame header (the code you deleted under the m_compiler->opts.compDbgEnC in the PR). This is similar to how lvaMonAcquired is handled.
I am ok with it, but just know that EnC will not work correctly without this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much for point this out!

I have some questions about this, could you please give me some suggestions?

  1. Is the EnC request to enble FEATURE_REMAP_FUNCTION? If so I think LA64 will not hit the EnC scenes temporarily. (the code under the m_compiler->opts.compDbgEnC in CodeGen::genCreateAndStoreGCInfo looks improper for LA64 now. I will delete and add an assert at it.)

  2. Are there any testing related to EnC? We can add EnC support for LA64 if necessary later. ( I used to run a hot reload demo at .NET6 and it's ok on LA64 at that time: https://github.com/jsuarezruiz/AvaloniaSkiaSharpFiddle ).

For EnC to work it is necessary that this is allocated in the top of the stack frame and included as part of the EnC frame header (the code you deleted under the m_compiler->opts.compDbgEnC in the PR). This is similar to how lvaMonAcquired is handled.

Thank you! If I understand correctly lvaAsyncExecutionContextVar and lvaAsyncExecutionContextVar are temp locals, on loongarch64 (and maybe on risc-v, as similar frame layout) if place this after lvaMonAcquired, the temp locals aera will be split by Callee saved registers and fp/ra, I think we should add a handle about this case when zeroinit the frame in CodeGen::genZeroInitFrameUsingBlockInit() to avoid clean the Callee saved registers/fp/ra on stack.

Here I think the int offset = lvaTable[lvaAsyncExecutionContextVar].GetStackOffset() + delta; , deltashould be (compCalleeRegsPushed << 3), as well as lvaMonAcquired and lvaAsyncSynchronizationContextVar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-loongarch64 area-VM-coreclr community-contribution Indicates that the PR has been added by a community member runtime-async

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants