Enable hot/cold splitting of EH funclets #71236

amanasifkhalid · 2022-06-23T22:37:45Z

This PR enables hot/cold splitting for functions containing exception handling. Currently, EH funclets are placed after the function's main body in memory. Via the heuristic that EH funclets are infrequently run, this enables a simple splitting implementation:

If we find a good split point in the function's main body, split there as usual.
Else if the function has exception handling,
- Check that all the EH funclets are cold. If any of the funclets are frequently run, don't bother splitting at all.
- If all of the EH funclets are cold, split at fgFirstFuncletBB, so that all EH funclets are placed in the cold section.

Being able to split each funclet independently would likely be more performant, but this simpler approach is beneficial for various reasons:

We can quickly enable hot/cold splitting for functions with EH -- a nontrivial case.
By either moving all EH funclets to the cold section or not splitting at all, we don't need to re-architect unwind info in the JIT or runtime.
We don't need to change the layout of EH funclets.

ghost · 2022-06-23T22:37:57Z

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

Issue Details

This PR enables hot/cold splitting for functions containing exception handling. Currently, EH funclets are placed after the function's main body in memory. Via the heuristic that EH funclets are infrequently run, this enables a simple splitting implementation:

If we find a good split point in the function's main body, split there as usual.
Else if the function has exception handling,
- Check that all the EH funclets are cold. If any of the funclets are frequently run, don't bother splitting at all.
- If all of the EH funclets are cold, split at fgFirstFuncletBB, so that all EH funclets are placed in the cold section.

Being able to split each funclet independently would likely be more performant, but this simpler approach is beneficial for various reasons:

We can quickly enable hot/cold splitting for functions with EH -- a nontrivial case.
By either moving all EH funclets to the cold section or not splitting at all, we don't need to re-architect unwind info in the JIT or runtime.
We don't need to change the layout of EH funclets.

Author:	amanasifkhalid
Assignees:	-
Labels:	`area-CodeGen-coreclr`
Milestone:	-

amanasifkhalid · 2022-06-23T22:44:47Z

/azp run runtime-jit-experimental

azure-pipelines · 2022-06-23T22:45:02Z

Azure Pipelines successfully started running 1 pipeline(s).

amanasifkhalid · 2022-06-23T22:57:07Z

src/coreclr/jit/emitarm64.cpp

@@ -8473,7 +8470,7 @@ void emitter::emitIns_J(instruction ins, BasicBlock* dst, int instrCount)

    id->idIns(ins);
    id->idInsFmt(fmt);
-    id->idjShort = idjShort;
+    id->idjShort = false;


When branching to a finally funclet on ARM64, we emit an INS_bl_local instruction -- previously, we could guarantee this jump would stay within the hot section, and thus would set idjShort = true. Now that EH funclets can be moved to the cold section, we can no longer assume this, and must set idjKeepLong based on if we're crossing hot/cold sections. Without changing this logic, we'll hit asserts in emitOutputLJ due to the branch being too short.

amanasifkhalid · 2022-06-23T23:52:18Z

@AndyAyersMS Since Bruce is OOF, could you PTAL? Thank you!

AndyAyersMS · 2022-06-24T02:25:52Z

src/coreclr/jit/flowgraph.cpp

+{
+    for (BasicBlock* block = fgFirstFuncletBB; block != nullptr; block = block->bbNext)
+    {
+#if HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION


It looks like we always define HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION to be 1.

Does that mean we're actually never splitting off EH any regions, or am I missing something?

Sorry, I didn't notice HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION is always 1. During testing, I brute-forced splitting at fgFirstFuncletBB via JitStressProcedureSplitting, so I didn't notice this condition was blocking splitting. I've removed it.

It may not be that simple. Presumably HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION exists for some reason. It would be good to know what that reason is.

Looking at the git blame for jit.h this define has been like this for years now. Perhaps it is no longer applicable, or perhaps it reflects some limitation in the way the jit must report EH information back to the runtime.

@jkotas do you know offhand why the jit has this constraint? If not, I can do some digging.

If it is indeed OK to change this, I'd recommend changing the define instead, so that the rest of the jit is freed from this constraint as well, and do that as a prerequisite, stand-alone change.

I do not know the reason behind this. The EE side is using GetCodeAddressForRelOffset method to resolve the handler offset to IP. This method deals with handler entrypoints in the cold section correctly.

AndyAyersMS · 2022-06-24T02:36:20Z

src/coreclr/jit/flowgraph.cpp

+ * section is cold. If any of the funclets are hot, then it may not be
+ * beneficial to split at fgFirstFuncletBB, moving all funclets to the
+ * cold section.
+ */


Can you use the "new style" function comments? They look like

//------------------------------------------------------------------------------ // methodName: short description // // Notes: // (text you have above) // bool Compiler::fgFuncletsAreCold()

Sure thing; fixed.

amanasifkhalid · 2022-06-24T17:17:48Z

/azp run runtime-jit-experimental

azure-pipelines · 2022-06-24T17:18:00Z

Azure Pipelines successfully started running 1 pipeline(s).

amanasifkhalid · 2022-06-24T19:31:14Z

/azp run runtime-jit-experimental

azure-pipelines · 2022-06-24T19:31:24Z

Azure Pipelines successfully started running 1 pipeline(s).

BruceForstall

LGTM.

Can you show an asm diff example of a small test case before/after with EH and stress splitting enabled?

amanasifkhalid · 2022-06-29T06:23:11Z

Rebasing on top of my change disabling HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION.

amanasifkhalid · 2022-06-29T06:23:29Z

/azp run runtime-jit-experimental

azure-pipelines · 2022-06-29T06:23:40Z

Azure Pipelines successfully started running 1 pipeline(s).

amanasifkhalid · 2022-06-29T16:18:45Z

Can you show an asm diff example of a small test case before/after with EH and stress splitting enabled?

Sure thing. Below is a JIT disasm (on Win x64) for a short function that catches a System.AccessViolationException without EH splitting:


G_M45968_IG01:              ;; offset=0000H
       55                   push     rbp
       56                   push     rsi
       4883EC28             sub      rsp, 40
       488D6C2430           lea      rbp, [rsp+30H]
       488965F0             mov      qword ptr [rbp-10H], rsp
						;; size=15 bbWeight=1    PerfScore 3.75
G_M45968_IG02:              ;; offset=000FH
       4863C2               movsxd   rax, edx
       4803C1               add      rax, rcx
       A801                 test     al, 1
       7506                 jne      SHORT G_M45968_IG04
						;; size=10 bbWeight=1    PerfScore 1.75
G_M45968_IG03:              ;; offset=0019H
       480FBF00             movsx    rax, word  ptr [rax]
       EB04                 jmp      SHORT G_M45968_IG05
						;; size=6 bbWeight=0.50 PerfScore 3.00
G_M45968_IG04:              ;; offset=001FH
       480FBF00             movsx    rax, word  ptr [rax]
						;; size=4 bbWeight=0.50 PerfScore 2.00
G_M45968_IG05:              ;; offset=0023H
       4883C428             add      rsp, 40
       5E                   pop      rsi
       5D                   pop      rbp
       C3                   ret      
						;; size=7 bbWeight=1    PerfScore 2.25
G_M45968_IG06:              ;; offset=002AH
       55                   push     rbp
       56                   push     rsi
       4883EC28             sub      rsp, 40
       488B6920             mov      rbp, qword ptr [rcx+32]
       48896C2420           mov      qword ptr [rsp+20H], rbp
       488D6D30             lea      rbp, [rbp+30H]
						;; size=19 bbWeight=0    PerfScore 0.00
G_M45968_IG07:              ;; offset=003DH
       48B9589C5305FC7F0000 mov      rcx, 0x7FFC05539C58      ; System.AccessViolationException
       E800000000           call     CORINFO_HELP_NEWSFAST
       488BF0               mov      rsi, rax
       488BCE               mov      rcx, rsi
       FF1500000000         call     [hackishModuleName:hackishMethodName():this]
       488BCE               mov      rcx, rsi
       E800000000           call     CORINFO_HELP_THROW
       CC                   int3     
						;; size=36 bbWeight=0    PerfScore 0.00

Note the lack of hot/cold splitting. Now, with EH splitting:

G_M45968_IG01:              ;; offset=0000H
       55                   push     rbp
       56                   push     rsi
       4883EC28             sub      rsp, 40
       488D6C2430           lea      rbp, [rsp+30H]
       488965F0             mov      qword ptr [rbp-10H], rsp
						;; size=15 bbWeight=1    PerfScore 3.75
G_M45968_IG02:              ;; offset=000FH
       4863C2               movsxd   rax, edx
       4803C1               add      rax, rcx
       A801                 test     al, 1
       7506                 jne      SHORT G_M45968_IG04
						;; size=10 bbWeight=1    PerfScore 1.75
G_M45968_IG03:              ;; offset=0019H
       480FBF00             movsx    rax, word  ptr [rax]
       EB04                 jmp      SHORT G_M45968_IG05
						;; size=6 bbWeight=0.50 PerfScore 3.00
G_M45968_IG04:              ;; offset=001FH
       480FBF00             movsx    rax, word  ptr [rax]
						;; size=4 bbWeight=0.50 PerfScore 2.00
G_M45968_IG05:              ;; offset=0023H
       4883C428             add      rsp, 40
       5E                   pop      rsi
       5D                   pop      rbp
       C3                   ret      
						;; size=7 bbWeight=1    PerfScore 2.25
************** Beginning of cold code **************

G_M45968_IG06:              ;; offset=002AH
       55                   push     rbp
       56                   push     rsi
       4883EC28             sub      rsp, 40
       488B6920             mov      rbp, qword ptr [rcx+32]
       48896C2420           mov      qword ptr [rsp+20H], rbp
       488D6D30             lea      rbp, [rbp+30H]
						;; size=19 bbWeight=0    PerfScore 0.00
G_M45968_IG07:              ;; offset=003DH
       48B9589C5305FC7F0000 mov      rcx, 0x7FFC05539C58      ; System.AccessViolationException
       E800000000           call     CORINFO_HELP_NEWSFAST
       488BF0               mov      rsi, rax
       488BCE               mov      rcx, rsi
       FF1500000000         call     [hackishModuleName:hackishMethodName():this]
       488BCE               mov      rcx, rsi
       E800000000           call     CORINFO_HELP_THROW
       CC                   int3     
						;; size=36 bbWeight=0    PerfScore 0.00

amanasifkhalid · 2022-06-29T16:20:08Z

@BruceForstall Would you also like to see an asmdiff with/without stress splitting? That implementation still splits after the first basic block, so I'm not sure how well it will demonstrate EH funclet splitting.

BruceForstall · 2022-06-29T16:40:10Z

@BruceForstall Would you also like to see an asmdiff with/without stress splitting? That implementation still splits after the first basic block, so I'm not sure how well it will demonstrate EH funclet splitting.

I guess I was wondering if there is a case where you can see visible asm diff or unwind/EH info diffs, but I guess with funclets there won't be any hot/cold branches that change, because there are no direct branches to/from the funclets, except for BBJ_CALLFINALLY to a finally block. For a catch block that doesn't end in a throw (like the example above), it does load the address of the "continuation" (where the catch should return to), so that needs to be properly relocated.

amanasifkhalid · 2022-06-29T21:59:38Z

The relocation isn't present in the above example because the BBJ_EHCATCHRET jump was optimized out; in other test cases, the relocation was observed. The control flow for reporting relocations looks correct -- this correctness will be verified when we begin testing EH funclet splitting with the Crossgen2 prototype.

Enable hot/cold splitting of EH funclets

f5359e6

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 23, 2022

ghost assigned amanasifkhalid Jun 23, 2022

amanasifkhalid commented Jun 23, 2022

View reviewed changes

amanasifkhalid marked this pull request as ready for review June 23, 2022 23:52

AndyAyersMS reviewed Jun 24, 2022

View reviewed changes

Removed condition blocking splitting; fixed style

957dab5

amanasifkhalid force-pushed the split-eh branch from ae44c67 to 957dab5 Compare June 24, 2022 19:29

amanasifkhalid mentioned this pull request Jun 24, 2022

Disable HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION #71273

Merged

BruceForstall approved these changes Jun 28, 2022

View reviewed changes

Merge branch 'dotnet:main' into split-eh

d71e69f

amanasifkhalid merged commit 3401228 into dotnet:main Jun 29, 2022

amanasifkhalid deleted the split-eh branch June 29, 2022 22:23

cshung mentioned this pull request Jun 30, 2022

[Hot cold splitting] Support split-eh dotnet/runtimelab#1918

Closed

10 tasks

JulieLeeMSFT mentioned this pull request Jul 7, 2022

What's new in .NET 7 Preview 6 [WIP] dotnet/core#7454

Closed

AndyAyersMS mentioned this pull request Jul 8, 2022

Dynamic PGO work planned for .NET 7 #64659

Closed

10 tasks

JulieLeeMSFT mentioned this pull request Jul 28, 2022

What's new in .NET 7 Preview 7 [WIP] dotnet/core#7455

Closed

ghost locked as resolved and limited conversation to collaborators Jul 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable hot/cold splitting of EH funclets #71236

Enable hot/cold splitting of EH funclets #71236

amanasifkhalid commented Jun 23, 2022

ghost commented Jun 23, 2022

amanasifkhalid commented Jun 23, 2022

azure-pipelines bot commented Jun 23, 2022

amanasifkhalid Jun 23, 2022

amanasifkhalid commented Jun 23, 2022

AndyAyersMS Jun 24, 2022

amanasifkhalid Jun 24, 2022

AndyAyersMS Jun 24, 2022

jkotas Jun 24, 2022

AndyAyersMS Jun 24, 2022

amanasifkhalid Jun 24, 2022

amanasifkhalid commented Jun 24, 2022

azure-pipelines bot commented Jun 24, 2022

amanasifkhalid commented Jun 24, 2022

azure-pipelines bot commented Jun 24, 2022

BruceForstall left a comment

amanasifkhalid commented Jun 29, 2022

amanasifkhalid commented Jun 29, 2022

azure-pipelines bot commented Jun 29, 2022

amanasifkhalid commented Jun 29, 2022

amanasifkhalid commented Jun 29, 2022

BruceForstall commented Jun 29, 2022

amanasifkhalid commented Jun 29, 2022

Enable hot/cold splitting of EH funclets #71236

Enable hot/cold splitting of EH funclets #71236

Conversation

amanasifkhalid commented Jun 23, 2022

ghost commented Jun 23, 2022

amanasifkhalid commented Jun 23, 2022

azure-pipelines bot commented Jun 23, 2022

amanasifkhalid Jun 23, 2022

Choose a reason for hiding this comment

amanasifkhalid commented Jun 23, 2022

AndyAyersMS Jun 24, 2022

Choose a reason for hiding this comment

amanasifkhalid Jun 24, 2022

Choose a reason for hiding this comment

AndyAyersMS Jun 24, 2022

Choose a reason for hiding this comment

jkotas Jun 24, 2022

Choose a reason for hiding this comment

AndyAyersMS Jun 24, 2022

Choose a reason for hiding this comment

amanasifkhalid Jun 24, 2022

Choose a reason for hiding this comment

amanasifkhalid commented Jun 24, 2022

azure-pipelines bot commented Jun 24, 2022

amanasifkhalid commented Jun 24, 2022

azure-pipelines bot commented Jun 24, 2022

BruceForstall left a comment

Choose a reason for hiding this comment

amanasifkhalid commented Jun 29, 2022

amanasifkhalid commented Jun 29, 2022

azure-pipelines bot commented Jun 29, 2022

amanasifkhalid commented Jun 29, 2022

amanasifkhalid commented Jun 29, 2022

BruceForstall commented Jun 29, 2022

amanasifkhalid commented Jun 29, 2022