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

Jump to bottom

[LoongArch64] Enable the "TC_QuickJitForLoops" on LoongArch64. #86022

Closed
wants to merge 1 commit into from
Closed

[LoongArch64] Enable the "TC_QuickJitForLoops" on LoongArch64. #86022

wants to merge 1 commit into from

Conversation

LuckyXu-HF
Copy link
Contributor

This PR is part of the issue #69705 to amend the LA's port.

Enable the "TC_QuickJitForLoops" on LoongArch64.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 10, 2023
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label May 10, 2023
@LuckyXu-HF
Copy link
Contributor Author

LuckyXu-HF commented May 10, 2023 •
edited
Loading

Hi, @shushanhf @jakobbotsch
We have tested this PR, and it's ok on LoongArch64.
Could you please review this PR?
Thanks

@build-analysis build-analysis bot mentioned this pull request May 10, 2023
Closed
@jakobbotsch
Copy link
Member

I would advise against enabling quick-jit for loops without corresponding support for on-stack replacement. x64 and arm64 support on-stack replacement which is why they also enable quick-jit for loops.

Enabling QJFL without OSR risks methods with loops in them getting stuck in slow unoptimized code since tiering only kicks in at method call boundaries.

@shushanhf
Copy link
Contributor

Enabling QJFL without OSR risks methods with loops in them getting stuck in slow unoptimized code since tiering only kicks in at method call boundaries.

Thanks very much.

@LuckyXu-HF
Copy link
Contributor Author

I would advise against enabling quick-jit for loops without corresponding support for on-stack replacement. x64 and arm64 support on-stack replacement which is why they also enable quick-jit for loops.

Enabling QJFL without OSR risks methods with loops in them getting stuck in slow unoptimized code since tiering only kicks in at method call boundaries.

Thank you very much，we will open the feature OSR and test again.

@teo-tsirpanis teo-tsirpanis added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 13, 2023
@ghost
Copy link

ghost commented May 13, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR is part of the issue #69705 to amend the LA's port.

Enable the "TC_QuickJitForLoops" on LoongArch64.

Author: HFDude
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution
Milestone: -

@shushanhf
Copy link
Contributor

The OSR for LA64 is just for optimization and we will do it later.

@shushanhf shushanhf mentioned this pull request May 16, 2023
Closed
@jakobbotsch
Copy link
Member

The OSR for LA64 is just for optimization and we will do it later.

Then we should keep TC_QuickJitForLoops with its current default value.

@shushanhf
Copy link
Contributor

The OSR for LA64 is just for optimization and we will do it later.

Then we should keep TC_QuickJitForLoops with its current default value.

Yes, we will still develop it but not so urgent.
We will fix some errors fisrt to make sure the .NET8 released on LA64, the performance is the next step.

@JulieLeeMSFT JulieLeeMSFT requested a review from jakobbotsch May 22, 2023 16:50
@jakobbotsch
Copy link
Member

Sounds good @shushanhf. In that case can we close this PR until OSR is working?

@shushanhf
Copy link
Contributor

Sounds good @shushanhf. In that case can we close this PR until OSR is working?

OK, We will close this PR.
Thanks

@LuckyXu-HF LuckyXu-HF closed this May 23, 2023
@LuckyXu-HF LuckyXu-HF deleted the main-LA64 branch May 24, 2023 03:40
@ghost ghost locked as resolved and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jakobbotsch jakobbotsch Awaiting requested review from jakobbotsch

Assignees
No one assigned
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LuckyXu-HF @jakobbotsch @shushanhf @teo-tsirpanis