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.

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

JIT: enable removal of try/catch if the try can't throw. #110273

Merged
merged 3 commits into from
Dec 2, 2024

Conversation

AndyAyersMS
Copy link
Member

If no tree in the try region of a try/catch can throw, then we can remove the try and delete the catch.

If no tree in the try region of a try/finally can throw, we can remove the try and inline the finally. This slightly generalizes the empty-try/finally opt we have been doing for a long time. (We should do something similar for try/fault, but don't, yet).

Since these optimization passes are cheap, and opportunities for them arise after other optimizations and unblock subsequent optimizations, run them early, middle, and late.

Resolves #107191.

I expect we'll see more of these cases in the future, say if we unblock cloning of loops with EH.

If no tree in the try region of a try/catch can throw, then
we can remove the try and delete the catch.

If no tree in the try region of a try/finally can throw, we can
remove the try and inline the finally. This slightly generalizes
the empty-try/finally opt we have been doing for a long time.
(We should do something similar for try/fault, but don't, yet).

Since these optimization passes are cheap, and opportunities
for them arise after other optimizations and unblock subsequent
optimizations, run them early, middle, and late.

Resolves dotnet#107191.

I expect we'll see more of these cases in the future, say if
we unblock cloning of loops with EH.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Nov 29, 2024
@AndyAyersMS
Copy link
Member Author

@EgorBo PTAL
cc @dotnet/jit-contrib

Note we are trusting GTF_EXCEPT fairly early... I think this is ok (it is glob ref that is iffy). Should be small number of diffs, but potential for more down the road with some work I have in flight.

Comment on lines 1045 to 1053
if (hadDfs)
{
fgInvalidateDfsTree();
m_dfsTree = fgComputeDfs();
if (hadLoops)
{
m_loops = FlowGraphNaturalLoops::Find(m_dfsTree);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

It seems unnecessary to recompute this in each of the phases. Maybe combine it into one phase, or make the actual phase that needs these annotations compute them if they aren't computed when we get there?

Copy link
Member Author

Choose a reason for hiding this comment

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

moved recomputation later (to doms for dfs, vn for loops).

@VSadov
Copy link
Member

VSadov commented Nov 29, 2024

If no tree in the try region of a try/finally can throw, we can remove the try and inline the finally.

Just wondering - We used to have some special meaning for code in finally with respect to ThreadAbort. Is that no longer a thing or still somehow maintained when finally is inlined?

@MichalPetryka
Copy link
Contributor

If no tree in the try region of a try/finally can throw, we can remove the try and inline the finally.

Just wondering - We used to have some special meaning for code in finally with respect to ThreadAbort. Is that no longer a thing or still somehow maintained when finally is inlined?

AFAIR all the handling for that was removed.

@AndyAyersMS
Copy link
Member Author

If no tree in the try region of a try/finally can throw, we can remove the try and inline the finally.

Just wondering - We used to have some special meaning for code in finally with respect to ThreadAbort. Is that no longer a thing or still somehow maintained when finally is inlined?

AFAIR all the handling for that was removed.

That is only an issue for .NET Framework. Removal of empty-try / finally has been in .NET Core since 2.x, this just extends it a bit further to some trys that are not empty.

@VSadov
Copy link
Member

VSadov commented Nov 30, 2024

I remember long time ago we could do similar optimizations in Roslyn, in theory, but in practice we could not.
And I guess it is still not feasible on Roslyn side since targeting .net FX is still a real scenario.

@AndyAyersMS
Copy link
Member Author

On x86 we have callfinallys within the try. In this case inside the try/finally there is a try/catch that returns from the catch, so must invoke the outer finally, thus there is a callfinally pair in the catch.

BB05 [0004]  1  0  3 BB12                  0    [008..00B)-> BB07(1)                 (callf ) T0 H3   try {       i rare keep
BB13 [0012]  1  0  3 BB07                  0    [???..???)-> BB14(1)                 (callfr) T0 H3   }           i rare internal
BB06 [0005]  1  1  0                       0    [00B..00F)-> BB07(1)                 (callf ) T1 H0   catch {     i rare keep
BB15 [0014]  1  1  0 BB07                  0    [???..???)-> BB16(1)                 (callfr) T1 H0   } }         i rare internal
BB07 [0006]  3  2  1 BB05,BB06             0    [00F..012)-> BB13(0.5),BB15(0.5)     (finret) T2 H1 finally { } } i rare keep

Here T0 is empty and we're trying to remove it and H0.

But there is a pred edge into the middle of H0/BB15 from an outside region (BB07), which is not something my simple-minded handler removal code supports yet.

@AndyAyersMS
Copy link
Member Author

/azp run runtime-coreclr jitstress

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AndyAyersMS AndyAyersMS merged commit 7ccf491 into dotnet:main Dec 2, 2024
123 of 126 checks passed
eduardo-vp pushed a commit to eduardo-vp/runtime that referenced this pull request Dec 5, 2024
If no tree in the try region of a try/catch can throw, then
we can remove the try and delete the catch.

If no tree in the try region of a try/finally can throw, we can
remove the try and inline the finally. This slightly generalizes
the empty-try/finally opt we have been doing for a long time.
(We should do something similar for try/fault, but don't, yet).

Since these optimization passes are cheap, and opportunities
for them arise after other optimizations and unblock subsequent
optimizations, run them early, middle, and late.

Resolves dotnet#107191.

I expect we'll see more of these cases in the future, say if
we unblock cloning of loops with EH.
@hez2010
Copy link
Contributor

hez2010 commented Dec 5, 2024

With this both Test1 and Test2 can produce optimal codegen now

void Test1()
{
    var x = 42;
    var z = 52;
    try
    {
        var y = 14;
        z = x + y;
    }
    finally
    {
        Console.WriteLine(z);
    }
}

void Test2()
{
    var x = 42;
    var z = 52;
    try
    {
        var y = 14;
        z = x + y;
    }
    catch { }
    Console.WriteLine(z);
}

codegen:

G_M27646_IG01:  ;; offset=0x0000
						;; size=0 bbWeight=1 PerfScore 0.00
G_M27646_IG02:  ;; offset=0x0000
       mov      edi, 56
						;; size=5 bbWeight=1 PerfScore 0.25
G_M27646_IG03:  ;; offset=0x0005
       tail.jmp [System.Console:WriteLine(int)]
						;; size=6 bbWeight=1 PerfScore 2.00

But the codegen of Test3 which has both catch and finally is still not optimal:

void Test3()
{
    var x = 42;
    var z = 52;
    try
    {
        var y = 14;
        z = x + y;
    }
    catch { }
    finally
    {
        Console.WriteLine(z);
    }
}

codegen:

G_M27646_IG01:  ;; offset=0x0000
       push     rbp
       sub      rsp, 16
       lea      rbp, [rsp+0x10]
       mov      qword ptr [rbp-0x10], rsp
						;; size=14 bbWeight=1 PerfScore 2.75
G_M27646_IG02:  ;; offset=0x000E
       mov      dword ptr [rbp-0x04], 52
						;; size=7 bbWeight=1 PerfScore 1.00
G_M27646_IG03:  ;; offset=0x0015
       mov      dword ptr [rbp-0x04], 56
						;; size=7 bbWeight=1 PerfScore 1.00
G_M27646_IG04:  ;; offset=0x001C
       mov      edi, 56
       call     [System.Console:WriteLine(int)]
       nop      
						;; size=12 bbWeight=1 PerfScore 3.50
G_M27646_IG05:  ;; offset=0x0028
       add      rsp, 16
       pop      rbp
       ret      
						;; size=6 bbWeight=1 PerfScore 1.75
G_M27646_IG06:  ;; offset=0x002E
       push     rbp
       sub      rsp, 16
       mov      rbp, qword ptr [rdi]
       mov      qword ptr [rsp], rbp
       lea      rbp, [rbp+0x10]
						;; size=16 bbWeight=0 PerfScore 0.00
G_M27646_IG07:  ;; offset=0x003E
       mov      edi, dword ptr [rbp-0x04]
       call     [System.Console:WriteLine(int)]
       nop      
						;; size=10 bbWeight=0 PerfScore 0.00
G_M27646_IG08:  ;; offset=0x0048
       add      rsp, 16
       pop      rbp
       ret      
						;; size=6 bbWeight=0 PerfScore 0.00

/cc: @AndyAyersMS

@AndyAyersMS
Copy link
Member Author

But the codegen of Test3 which has both catch and finally is still not optimal:

This turns into a try-catch inside a try-finally. We remove the try-catch and convert the try-finally to a try-fault. But we haven't yet implemented (empty)try-fault removal.

@AndyAyersMS
Copy link
Member Author

Seems like this is actually simple to handle; I'll put up a PR shortly.

mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this pull request Dec 10, 2024
If no tree in the try region of a try/catch can throw, then
we can remove the try and delete the catch.

If no tree in the try region of a try/finally can throw, we can
remove the try and inline the finally. This slightly generalizes
the empty-try/finally opt we have been doing for a long time.
(We should do something similar for try/fault, but don't, yet).

Since these optimization passes are cheap, and opportunities
for them arise after other optimizations and unblock subsequent
optimizations, run them early, middle, and late.

Resolves dotnet#107191.

I expect we'll see more of these cases in the future, say if
we unblock cloning of loops with EH.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR 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.

JIT does not elide empty try/catch blocks
5 participants