Skip to content

JIT: give inlinees their own EH table#112968

Merged
AndyAyersMS merged 2 commits intodotnet:mainfrom
AndyAyersMS:InlineMethodsWithEHPrereq
Feb 27, 2025
Merged

JIT: give inlinees their own EH table#112968
AndyAyersMS merged 2 commits intodotnet:mainfrom
AndyAyersMS:InlineMethodsWithEHPrereq

Conversation

@AndyAyersMS
Copy link
Member

@AndyAyersMS AndyAyersMS commented Feb 26, 2025

For a long time now inlinees have "inherited" the root method EH table. Looks like the only reason we were doing this was to make certain pinvoke inlining checks simpler.

Rework those to handle the inline case directly. Quirk one bit of profitability logic for now to avoid diffs.

Part of #108900.

@AndyAyersMS
JIT: give inlinees their own EH table
400611e 
For a long time now inlinees have "inherited" the root method EH table. Looks like
the only reason we were doing this was to make certain pinvoke inlining checks simpler.

Rework those to handle the inline case directly. Quirk one bit of profitability logic
for now to avoid diffs.

Part of dotnet#108900.
Copilot AI review requested due to automatic review settings February 26, 2025 21:26
Copilot AI reviewed Feb 26, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 26, 2025
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Feb 26, 2025

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

@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented Feb 26, 2025

@EgorBo @BruceForstall PTAL
cc @dotnet/jit-contrib

No diffs; doesn't enable any new behavior.

EgorBo
EgorBo approved these changes Feb 26, 2025
View reviewed changes
Copy link
Member

@EgorBo EgorBo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM if CI is happy, I think last time I tried, I hit some assert in VM API but most likely that was my incorrect change

@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented Feb 26, 2025 •
edited
Loading

LGTM if CI is happy, I think last time I tried, I hit some assert in VM API but most likely that was my incorrect change

There are VM-side changes needed to enable inlining methods with EH. Those will show up in a follow-up PR.

@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented Feb 26, 2025

Sneak preview of the above (not quite working yet, thanks in part to the confusion I'm fixing here)

using System.Runtime.CompilerServices;

Bar();

[MethodImpl(MethodImplOptions.AggressiveInlining)]
void Bar()
{
    try
    {
        Console.WriteLine("Bar");
    }
    finally
    {
        Console.WriteLine("Bar finally");
    }
}
INLINER: inlineInfo.tokenLookupContextHandle for Program:<<Main>$>g__Bar|0_0() set to 0x00007FFD084C32C9:

Invoking compiler for the inlinee method Program:<<Main>$>g__Bar|0_0() :
IL to import:
IL_0000  72 01 00 00 70    ldstr        0x70000001
IL_0005  28 0e 00 00 0a    call         0xA00000E
IL_000a  de 0b             leave.s      11 (IL_0017)
IL_000c  72 09 00 00 70    ldstr        0x70000009
IL_0011  28 0e 00 00 0a    call         0xA00000E
IL_0016  dc                endfinally  
IL_0017  2a                ret         

INLINER impTokenLookupContextHandle for Program:<<Main>$>g__Bar|0_0() is 0x00007FFD084C32C9.
0 return registers for return type void 
*************** In compInitDebuggingInfo() for Program:<<Main>$>g__Bar|0_0()
info.compStmtOffsetsCount    = 0
info.compStmtOffsetsImplicit = 0005h ( STACK_EMPTY CALL_SITE )
*************** In fgFindBasicBlocks() for Program:<<Main>$>g__Bar|0_0()
Jump targets:
  IL_0000
  IL_000c
  IL_0017
New Basic Block BB01 [0001] created.
BB01 [0001] [000..00C)
New Basic Block BB02 [0002] created.
BB02 [0002] [00C..017)
New Basic Block BB03 [0003] created.
BB03 [0003] [017..018)
setting likelihood of BB01 -> BB03 to 1
EH clause #0:
  Flags:         0x2 (finally)
  TryOffset:     0x0
  TryLength:     0xc
  HandlerOffset: 0xc
  HandlerLength: 0xb
  ClassToken:    0x0
*************** After fgFindBasicBlocks() has created the EH table

***************  Exception Handling table
index  eTry, eHnd
  0  ::            - Try at BB01..BB01 [000..00C), Finally at BB02..BB02 [00C..017)
*************** In fgNormalizeEH()

---------------------------------------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight   [IL range]   [jump]                            [EH region]        [flags]
---------------------------------------------------------------------------------------------------------------------------------------------------------------------
BB01 [0001]  1  0                          1    [000..00C)-> BB03(1)                 (leave ) T0      try { }     keep
BB02 [0002]  1     0                       1    [00C..017)-> ????                    (finret)    H0   finally { } keep
BB03 [0003]  1       BB01                  1    [017..018)                           (return)                     
---------------------------------------------------------------------------------------------------------------------------------------------------------------------

...


Inlines into 06000001 [via ExtendedDefaultPolicy] Program:<Main>$(System.String[]):
  [INL01 IL=-572662307 TR=000000 06000003] [INLINED: callee: aggressive inline attribute] Program:<<Main>$>g__Bar|0_0()
    [INL00 IL=0005 TR=000003 06000092] [FAILED: callee: noinline per IL/cached result] System.Console:WriteLine(System.String)
    [INL00 IL=0017 TR=000005 06000092] [FAILED: callee: noinline per IL/cached result] System.Console:WriteLine(System.String)

*************** Finishing PHASE Morph - Inlining
Trees after Morph - Inlining

---------------------------------------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight   [IL range]   [jump]                            [EH region]        [flags]
---------------------------------------------------------------------------------------------------------------------------------------------------------------------
BB01 [0000]  1                             1    [000..005)-> BB06(1)                 (always)                     i
BB06 [0004]  1       BB01                  1    [000..001)-> BB03(1)                 (always)                     i internal
BB03 [0001]  1  0    BB06                  1    [000..001)-> BB07(1)                 (always) T0      try { }     i keep
BB07 [0005]  1       BB03                  1    [000..001)-> BB04(1)                 (callf )                     i internal
BB08 [0006]  0                             1    [000..001)-> BB05(1)                 (callfr)                     i internal
BB04 [0002]  2     0 BB07                  1    [000..001)-> ????                    (finret)    H0   finally { } i keep
BB05 [0003]  1       BB08                  1    [000..001)-> BB02(1)                 (always)                     i
BB02 [0007]  1       BB05                  1    [005..006)                           (return)                     i
---------------------------------------------------------------------------------------------------------------------------------------------------------------------

This was referenced Feb 26, 2025
Closed
Closed
@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented Feb 27, 2025

Need to tweak the logic for NAOT.

@build-analysis build-analysis bot mentioned this pull request Feb 27, 2025
Open
@AndyAyersMS AndyAyersMS merged commit b54529f into dotnet:main Feb 27, 2025
112 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@EgorBo EgorBo EgorBo approved these changes

+1 more reviewer

@BruceForstall BruceForstall BruceForstall approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@AndyAyersMS AndyAyersMS

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AndyAyersMS @EgorBo @BruceForstall