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: support dead block elimination for EH handlers #82335

Open
BruceForstall opened this issue Feb 18, 2023 · 4 comments
Open

JIT: support dead block elimination for EH handlers #82335

BruceForstall opened this issue Feb 18, 2023 · 4 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@BruceForstall
Copy link
Member

fgRemoveDeadBlocks() removes unreachable code by starting from a set of initial blocks and walking control-flow paths until all reachable blocks are marked, then removing any unmarked blocks.

However, this will not remove any EH handlers (finally/fault/catch/filter/filter-handler) even if the associated try is not reachable.

Change the logic to only add the EH handler entries corresponding to a try to the reachable set once a reachable try body is found.

This will also require ensuring the related EH table entry is removed if necessary.

@BruceForstall BruceForstall added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 18, 2023
@BruceForstall BruceForstall added this to the 8.0.0 milestone Feb 18, 2023
@BruceForstall BruceForstall self-assigned this Feb 18, 2023
@ghost
Copy link

ghost commented Feb 18, 2023

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

Issue Details

fgRemoveDeadBlocks() removes unreachable code by starting from a set of initial blocks and walking control-flow paths until all reachable blocks are marked, then removing any unmarked blocks.

However, this will not remove any EH handlers (finally/fault/catch/filter/filter-handler) even if the associated try is not reachable.

Change the logic to only add the EH handler entries corresponding to a try to the reachable set once a reachable try body is found.

This will also require ensuring the related EH table entry is removed if necessary.

Author: BruceForstall
Assignees: BruceForstall
Labels:

area-CodeGen-coreclr

Milestone: 8.0.0

@EgorBo
Copy link
Member

EgorBo commented Jul 17, 2023

Example:

int Test() => 1;

void Foo()
{
    if (Test() != 1)
    {
        // unreachable
        try
        {
            Console.WriteLine();
        }
        catch
        {
        }
    }
}

it's expected for Foo to be lowered to no-op, but instead, it emits:

; Assembly listing for method Program:Foo():this (FullOpts)
G_M52879_IG01:
       push     rbp
       sub      rsp, 16
       lea      rbp, [rsp+10H]
       mov      qword ptr [rbp-10H], rsp
						;; size=14 bbWeight=1 PerfScore 2.75
G_M52879_IG02:
       add      rsp, 16
       pop      rbp
       ret      
						;; size=6 bbWeight=1 PerfScore 1.75
G_M52879_IG03:
       int3     
						;; size=1 bbWeight=0 PerfScore 0.00
G_M52879_IG04:
       push     rbp
       sub      rsp, 16
       mov      rbp, qword ptr [rcx]
       mov      qword ptr [rsp], rbp
       lea      rbp, [rbp+10H]
						;; size=16 bbWeight=0 PerfScore 0.00
G_M52879_IG05:
       lea      rax, G_M52879_IG02
						;; size=7 bbWeight=0 PerfScore 0.00
G_M52879_IG06:
       add      rsp, 16
       pop      rbp
       ret      
; Total bytes of code 50

BruceForstall added a commit to BruceForstall/runtime that referenced this issue Oct 13, 2023
Update `fgRemoveDeadBlocks()` to only treat EH handlers as reachable
if their corresponding `try` block entry is reachable.

This causes more dead code to be eliminated in handlers that are
not reachable.

There is a little more work to do to completely clean these up:
1. If a `try` is not reachable, we should get rid of its entire EH table
entry. Currently, its block is generated as `int3` (on xarch).
2. Even though the handler is not reachable, we still generate the prolog
followed by `int3` for the body.

Contributes to dotnet#82335
BruceForstall added a commit that referenced this issue Oct 14, 2023
* Improve EH dead code elimination

Update `fgRemoveDeadBlocks()` to only treat EH handlers as reachable
if their corresponding `try` block entry is reachable.

This causes more dead code to be eliminated in handlers that are
not reachable.

There is a little more work to do to completely clean these up:
1. If a `try` is not reachable, we should get rid of its entire EH table
entry. Currently, its block is generated as `int3` (on xarch).
2. Even though the handler is not reachable, we still generate the prolog
followed by `int3` for the body.

Contributes to #82335

* Fix BBJ_EHFILTERRET handling in fgRemoveBlockAsPred

fgRemoveBlockAsPred had special handling to not decrease the bbRefs count
of the filter-handler targeted by BBJ_EHFILTERRET. This is incorrect, as
the filter-handler already has an extra "beginning of handler" extra
ref count. Possibly this code was never invoked before as filters were
not previously removed.

Also, fix the JitDump output of the filter ret block in block dumps.

* Put back arm32 BBJ_CALLFINALLY/BBJ_ALWAYS pair check
@saucecontrol
Copy link
Member

I frequently use a pattern where I use exception filters with a JIT-time constant value. These are unmanaged callbacks, which SEH unwinds properly on Windows without the catch, so I want to catch only on Linux. The filter makes the catch unreachable on Windows, but the EH block stays (actually I get 2 -- one for the filtered case and one without)

static readonly bool capture = !OperatingSystem.IsWindows();

static int foo(int x)
{
    int res = 0;
    try {
        res = checked(x * x);
    }
    catch when (capture) {
    }
    
    return res;
}

Which compiles to:

       push     rbp
       sub      rsp, 16
       lea      rbp, [rsp+0x10]
       mov      qword ptr [rbp-0x10], rsp
						;; size=14 bbWeight=1 PerfScore 2.75
       xor      eax, eax
       mov      dword ptr [rbp-0x04], eax
						;; size=5 bbWeight=1 PerfScore 1.25
       imul     edi, edi
       jo       SHORT G_M23486_IG04
       mov      dword ptr [rbp-0x04], edi
       jmp      SHORT G_M23486_IG05
						;; size=10 bbWeight=1 PerfScore 6.00
G_M23486_IG04:
       call     CORINFO_HELP_OVERFLOW
       int3     
						;; size=6 bbWeight=0 PerfScore 0.00
G_M23486_IG05:
       mov      eax, dword ptr [rbp-0x04]
						;; size=3 bbWeight=1 PerfScore 1.00
       add      rsp, 16
       pop      rbp
       ret      
						;; size=6 bbWeight=1 PerfScore 1.75
       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
       xor      eax, eax
						;; size=2 bbWeight=0 PerfScore 0.00
       add      rsp, 16
       pop      rbp
       ret      
						;; size=6 bbWeight=0 PerfScore 0.00
       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
       lea      rax, G_M23486_IG05
						;; size=7 bbWeight=0 PerfScore 0.00
       add      rsp, 16
       pop      rbp
       ret      
						;; size=6 bbWeight=0 PerfScore 0.00
; Total bytes of code 97

@BruceForstall
Copy link
Member Author

This is a slightly different case of "dead EH clause": the filter-handler (aka "catch") is dead because the filter always returns zero. Since the filter always returns zero, it's unnecessary.

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

No branches or pull requests

3 participants