Skip to content

Re-order some blocks in runtimelookup - #83453

Closed
EgorBo wants to merge 8 commits into
dotnet:mainfrom
EgorBo:runtimelookup-reorder-blocks
Closed

Re-order some blocks in runtimelookup#83453
EgorBo wants to merge 8 commits into
dotnet:mainfrom
EgorBo:runtimelookup-reorder-blocks

Conversation

@EgorBo

@EgorBoEgorBo commented Mar 15, 2023

Copy link
Copy Markdown
Member

Motivation: #83430 (comment) let's see the diffs. #83430 changes JIT-EE guid so can't see diffs there

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 15, 2023
@ghostghost assigned EgorBoMar 15, 2023
@ghost

Copy link
Copy Markdown

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

Issue Details

Motivation: #83430 (comment)

Author:EgorBo
Assignees:-
Labels:

area-CodeGen-coreclr

Milestone:-

@EgorBo

Copy link
Copy Markdown
MemberAuthor

Comment on lines -415 to -458
if (result == PhaseStatus::MODIFIED_EVERYTHING)
{
if (opts.OptimizationEnabled())
{
fgReorderBlocks(/* useProfileData */ false);
fgUpdateChangedFlowGraph(FlowGraphUpdates::COMPUTE_BASICS);
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

W.r.t. the confusing diffs, maybe try with this change reverted?

Comment threadsrc/coreclr/jit/runtimelookup.cpp Outdated
nullcheckBb->bbJumpDest = fallbackBb;
fastPathBb->bbJumpDest = block;
nullcheckBb->bbJumpDest = fastPathBb;
fallbackBb->bbJumpDest = block;

@kunalspathakkunalspathakMar 28, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if you mark fallBackBb->bbSetRunRarely()? I had to do similar thing to re-arrange in #80297.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

G_M10050_IG01:subrsp,40 ;; size=4 bbWeight=1 PerfScore 0.25G_M10050_IG02:movrcx, qword ptr GS:[0x0058]movrcx, qword ptr [rcx+30H]movedx, dword ptr [rcx+48H]cmpedx,2jl SHORT G_M10050_IG04 ;; size=21 bbWeight=1 PerfScore 6.25G_M10050_IG03:movrcx, qword ptr [rcx+50H]cmp qword ptr [rcx+10H],0jne SHORT G_M10050_IG05 ;; size=11 bbWeight=0.90 PerfScore 5.42G_M10050_IG04:movrcx,0xD1FFAB1Emovedx,29movr8d,2call CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_NOCTOR_OPTIMIZEDmov qword ptr [rsp+20H],rax ;; size=31 bbWeight=0.09 PerfScore 0.25G_M10050_IG05:movrax, qword ptr [rsp+20H]moveax, dword ptr [rax+8CH] ;; size=11 bbWeight=1 PerfScore 3.00G_M10050_IG06:addrsp,40ret ;; size=5 bbWeight=1 PerfScore 1.25

vs.

G_M10050_IG01:subrsp,40 ;; size=4 bbWeight=1 PerfScore 0.25G_M10050_IG02:movrax, qword ptr GS:[0x0058]movrax, qword ptr [rax+30H]movecx, dword ptr [rax+48H]cmpecx,2jl SHORT G_M10050_IG06 ;; size=21 bbWeight=1 PerfScore 6.25G_M10050_IG03:movrax, qword ptr [rax+50H]cmp qword ptr [rax+10H],0je SHORT G_M10050_IG06 ;; size=11 bbWeight=0.90 PerfScore 5.42G_M10050_IG04:movr9, qword ptr [rsp+20H]moveax, dword ptr [r9+8CH] ;; size=12 bbWeight=1 PerfScore 3.00G_M10050_IG05:addrsp,40ret ;; size=5 bbWeight=1 PerfScore 1.25G_M10050_IG06:movrcx,0xD1FFAB1Emovedx,29movr8d,2call CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_NOCTOR_OPTIMIZEDmovr9,raxmov qword ptr [rsp+20H],r9jmp SHORT G_M10050_IG04 ;; size=36 bbWeight=0 PerfScore 0.00

@EgorBoEgorBo closed this Apr 14, 2023
@ghostghost locked as resolved and limited conversation to collaborators May 14, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@EgorBo@jakobbotsch@kunalspathak