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

Fix unwind info generation for EH funclets #1923

Merged
merged 2 commits into from
Jul 1, 2022

Conversation

amanasifkhalid
Copy link
Member

@amanasifkhalid amanasifkhalid commented Jul 1, 2022

This PR addresses the second task in #1918 by fixing unwind info generation for EH funclets in the JIT on x64. Additionally, a bug was found in fgDetermineFirstColdBlock where a block after fgFirstFuncletBB could be selected as a candidate for fgFirstColdBlock, potentially splitting the funclet section; this has been fixed by adding some additional logic to short-circuit the search if we reach the first funclet block.

Note that these changes apply specifically to x64; at some point, similar changes will be needed for ARM64, etc. I can do this work in a follow-up PR.

Below are some examples of unwind info generation for functions with EH, collected from various JIT dumps:

Function with no splitting, plus hot EH funclets:

Hot  code size = 0x263 bytes
Cold code size = 0x0 bytes
reserveUnwindInfo(isFunclet=false, isColdCode=false, unwindSize=0xe)
reserveUnwindInfo(isFunclet=true, isColdCode=false, unwindSize=0xe)
...
Unwind Info:
  >> Start offset   : 0x000000 (not in unwind data)
  >>   End offset   : 0x000230 (not in unwind data)
  Version           : 1
  Flags             : 0x00
  SizeOfProlog      : 0x08
  CountOfUnwindCodes: 5
  FrameRegister     : none (0)
  FrameOffset       : N/A (no FrameRegister) (Value=0)
  UnwindCodes       :
    CodeOffset: 0x08 UnwindOp: UWOP_ALLOC_SMALL (2)     OpInfo: 12 * 8 + 8 = 104 = 0x68
    CodeOffset: 0x04 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rbx (3)
    CodeOffset: 0x03 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rsi (6)
    CodeOffset: 0x02 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rdi (7)
    CodeOffset: 0x01 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rbp (5)
allocUnwindInfo(pHotCode=0x000001F9E1B23250, pColdCode=0x0000000000000000, startOffset=0x0, endOffset=0x230, unwindSize=0xe, pUnwindBlock=0x000001F9E3D63122, funKind=0 (main function))
Unwind Info:
  >> Start offset   : 0x000230 (not in unwind data)
  >>   End offset   : 0x000261 (not in unwind data)
  Version           : 1
  Flags             : 0x00
  SizeOfProlog      : 0x08
  CountOfUnwindCodes: 5
  FrameRegister     : none (0)
  FrameOffset       : N/A (no FrameRegister) (Value=0)
  UnwindCodes       :
    CodeOffset: 0x08 UnwindOp: UWOP_ALLOC_SMALL (2)     OpInfo: 4 * 8 + 8 = 40 = 0x28
    CodeOffset: 0x04 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rbx (3)
    CodeOffset: 0x03 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rsi (6)
    CodeOffset: 0x02 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rdi (7)
    CodeOffset: 0x01 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rbp (5)
allocUnwindInfo(pHotCode=0x000001F9E1B23250, pColdCode=0x0000000000000000, startOffset=0x230, endOffset=0x261, unwindSize=0xe, pUnwindBlock=0x000001F9E3D6335A, funKind=1 (handler))

Function with main body split, plus cold EH funclets:

Hot  code size = 0x35B bytes
Cold code size = 0x106 bytes
reserveUnwindInfo(isFunclet=false, isColdCode=false, unwindSize=0x18)
reserveUnwindInfo(isFunclet=false, isColdCode=true, unwindSize=0x0)
reserveUnwindInfo(isFunclet=true, isColdCode=true, unwindSize=0x16)
...
Unwind Info:
  >> Start offset   : 0x000000 (not in unwind data)
  >>   End offset   : 0x00035b (not in unwind data)
  Version           : 1
  Flags             : 0x00
  SizeOfProlog      : 0x13
  CountOfUnwindCodes: 10
  FrameRegister     : none (0)
  FrameOffset       : N/A (no FrameRegister) (Value=0)
  UnwindCodes       :
    CodeOffset: 0x13 UnwindOp: UWOP_ALLOC_LARGE (1)     OpInfo: 0 - Scaled small  
      Size: 17 * 8 = 136 = 0x00088
    CodeOffset: 0x0C UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rbx (3)
    CodeOffset: 0x0B UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rsi (6)
    CodeOffset: 0x0A UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rdi (7)
    CodeOffset: 0x09 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: r12 (12)
    CodeOffset: 0x07 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: r13 (13)
    CodeOffset: 0x05 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: r14 (14)
    CodeOffset: 0x03 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: r15 (15)
    CodeOffset: 0x01 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rbp (5)
allocUnwindInfo(pHotCode=0x000001F9E1BDF760, pColdCode=0x0000000000000000, startOffset=0x0, endOffset=0x35b, unwindSize=0x18, pUnwindBlock=0x000001F9E3D61168, funKind=0 (main function))
Unwind Info COLD:
  >> Start offset   : 0x00035b (not in unwind data)
  >>   End offset   : 0x000401 (not in unwind data)
allocUnwindInfo(pHotCode=0x000001F9E1BDF760, pColdCode=0x000001F9E1BDFB20, startOffset=0x0, endOffset=0xa6, unwindSize=0x0, pUnwindBlock=0x0000000000000000, funKind=0 (main function))
Unwind Info COLD:
  >> Start offset   : 0x000401 (not in unwind data)
  >>   End offset   : 0x000461 (not in unwind data)
allocUnwindInfo(pHotCode=0x000001F9E1BDF760, pColdCode=0x000001F9E1BDFB20, startOffset=0xa6, endOffset=0x106, unwindSize=0x0, pUnwindBlock=0x0000000000000000, funKind=1 (handler))

Function with main body NOT split, plus cold EH funclets:

Hot  code size = 0x467 bytes
Cold code size = 0x124 bytes
reserveUnwindInfo(isFunclet=false, isColdCode=false, unwindSize=0x10)
reserveUnwindInfo(isFunclet=true, isColdCode=true, unwindSize=0xe)
reserveUnwindInfo(isFunclet=true, isColdCode=true, unwindSize=0xe)
reserveUnwindInfo(isFunclet=true, isColdCode=true, unwindSize=0xe)
reserveUnwindInfo(isFunclet=true, isColdCode=true, unwindSize=0xe)
...
Unwind Info:
  >> Start offset   : 0x000000 (not in unwind data)
  >>   End offset   : 0x000467 (not in unwind data)
  Version           : 1
  Flags             : 0x00
  SizeOfProlog      : 0x0B
  CountOfUnwindCodes: 6
  FrameRegister     : none (0)
  FrameOffset       : N/A (no FrameRegister) (Value=0)
  UnwindCodes       :
    CodeOffset: 0x0B UnwindOp: UWOP_ALLOC_LARGE (1)     OpInfo: 0 - Scaled small  
      Size: 17 * 8 = 136 = 0x00088
    CodeOffset: 0x04 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rbx (3)
    CodeOffset: 0x03 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rsi (6)
    CodeOffset: 0x02 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rdi (7)
    CodeOffset: 0x01 UnwindOp: UWOP_PUSH_NONVOL (0)     OpInfo: rbp (5)
allocUnwindInfo(pHotCode=0x000001F9E3EBE480, pColdCode=0x0000000000000000, startOffset=0x0, endOffset=0x467, unwindSize=0x10, pUnwindBlock=0x000001F9E4178288, funKind=0 (main function))
Unwind Info COLD:
  >> Start offset   : 0x000467 (not in unwind data)
  >>   End offset   : 0x00049b (not in unwind data)
allocUnwindInfo(pHotCode=0x000001F9E3EBE480, pColdCode=0x000001F9E3989DB0, startOffset=0x0, endOffset=0x34, unwindSize=0x0, pUnwindBlock=0x0000000000000000, funKind=1 (handler))
Unwind Info COLD:
  >> Start offset   : 0x00049b (not in unwind data)
  >>   End offset   : 0x0004c5 (not in unwind data)
allocUnwindInfo(pHotCode=0x000001F9E3EBE480, pColdCode=0x000001F9E3989DB0, startOffset=0x34, endOffset=0x5e, unwindSize=0x0, pUnwindBlock=0x0000000000000000, funKind=1 (handler))
Unwind Info COLD:
  >> Start offset   : 0x0004c5 (not in unwind data)
  >>   End offset   : 0x000557 (not in unwind data)
allocUnwindInfo(pHotCode=0x000001F9E3EBE480, pColdCode=0x000001F9E3989DB0, startOffset=0x5e, endOffset=0xf0, unwindSize=0x0, pUnwindBlock=0x0000000000000000, funKind=1 (handler))
Unwind Info COLD:
  >> Start offset   : 0x000557 (not in unwind data)
  >>   End offset   : 0x00058b (not in unwind data)
allocUnwindInfo(pHotCode=0x000001F9E3EBE480, pColdCode=0x000001F9E3989DB0, startOffset=0xf0, endOffset=0x124, unwindSize=0x0, pUnwindBlock=0x0000000000000000, funKind=1 (handler))

@amanasifkhalid amanasifkhalid marked this pull request as ready for review July 1, 2022 15:02
@amanasifkhalid
Copy link
Member Author

@cshung @BruceForstall PTAL. Since this touches fgDetermineFirstColdBlock, I think Bruce should check this out, too. Thank you!

@amanasifkhalid
Copy link
Member Author

amanasifkhalid commented Jul 1, 2022

I re-measured some hot/cold splitting metrics on SuperPMI's benchmarks, libraries, and aspnet collections: There are slight reductions in total functions and functions with EH split (due to us not splitting in the funclet section anymore), but metrics are largely the same.

Benchmarks:

Total functions: 37730
Total functions split: 5275
Percent functions split: 13.980917042141533

Total bytes generated: 10613670
Total cold bytes: 547832
Percent bytes cold: 5.1615699376370285

Total functions with EH: 1843
Total functions with EH split: 1470
Percent EH split: 79.76125881714596

Libraries:

Total functions: 243397
Total functions split: 45923
Percent functions split: 18.86752918072121

Total bytes generated: 49846554
Total cold bytes: 3711530
Percent bytes cold: 7.445910904894248

Total functions with EH: 13349
Total functions with EH split: 10771
Percent EH split: 80.68769196194472

ASP.NET (has some PGO data):

Total functions: 67559
Total functions split: 17963
Percent functions split: 26.588611435930076

Total bytes generated: 22770893
Total cold bytes: 2705908
Percent bytes cold: 11.883187892543344

Total functions with EH: 6641
Total functions with EH split: 3417
Percent EH split: 51.45309441349194

@cshung
Copy link
Member

cshung commented Jul 1, 2022

Judging from the output, it appears that while we have the right number of calls, the unwind info for cold funclet is missing? In particular, all the calls to allocUnwindInfo for cold funclet has unwindSize == 0.
You probably also need to change unwindEmitFuncHelper just like you did in unwindReserveFuncHelper.

@amanasifkhalid
Copy link
Member Author

Thanks for catching that; just pushed a fix. Here's what the allocUnwindInfo calls look like now:

Function with no splitting, plus hot EH:

allocUnwindInfo(pHotCode=0x000001E1F6A07C50, pColdCode=0x0000000000000000, startOffset=0x0, endOffset=0x230, unwindSize=0xe, pUnwindBlock=0x000001E1F74B5112, funKind=0 (main function))
allocUnwindInfo(pHotCode=0x000001E1F6A07C50, pColdCode=0x0000000000000000, startOffset=0x230, endOffset=0x261, unwindSize=0xe, pUnwindBlock=0x000001E1F74B534A, funKind=1 (handler))

Function with main body split, plus cold EH:

allocUnwindInfo(pHotCode=0x000001E1F69F9820, pColdCode=0x0000000000000000, startOffset=0x0, endOffset=0x35b, unwindSize=0x18, pUnwindBlock=0x000001E1F4F34468, funKind=0 (main function))
allocUnwindInfo(pHotCode=0x000001E1F69F9820, pColdCode=0x000001E1F69F9170, startOffset=0x0, endOffset=0xa6, unwindSize=0x0, pUnwindBlock=0x0000000000000000, funKind=0 (main function))
allocUnwindInfo(pHotCode=0x000001E1F69F9820, pColdCode=0x000001E1F69F9170, startOffset=0xa6, endOffset=0x106, unwindSize=0x16, pUnwindBlock=0x000001E1F4F346A2, funKind=1 (handler))

Function with main body NOT split, plus cold EH:

allocUnwindInfo(pHotCode=0x000001E1F6A2CE60, pColdCode=0x0000000000000000, startOffset=0x0, endOffset=0x467, unwindSize=0x10, pUnwindBlock=0x000001E1F7816278, funKind=0 (main function))
allocUnwindInfo(pHotCode=0x000001E1F6A2CE60, pColdCode=0x000001E1F6A2CA60, startOffset=0x0, endOffset=0x34, unwindSize=0xe, pUnwindBlock=0x000001E1F78164B2, funKind=1 (handler))
allocUnwindInfo(pHotCode=0x000001E1F6A2CE60, pColdCode=0x000001E1F6A2CA60, startOffset=0x34, endOffset=0x5e, unwindSize=0xe, pUnwindBlock=0x000001E1F78166EA, funKind=1 (handler))
allocUnwindInfo(pHotCode=0x000001E1F6A2CE60, pColdCode=0x000001E1F6A2CA60, startOffset=0x5e, endOffset=0xf0, unwindSize=0xe, pUnwindBlock=0x000001E1F7816922, funKind=1 (handler))
allocUnwindInfo(pHotCode=0x000001E1F6A2CE60, pColdCode=0x000001E1F6A2CA60, startOffset=0xf0, endOffset=0x124, unwindSize=0xe, pUnwindBlock=0x000001E1F7816B5A, funKind=1 (handler))

Copy link
Member

@cshung cshung left a comment

Choose a reason for hiding this comment

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

The outputs look good to me.

@cshung cshung merged commit b4fccbf into dotnet:feature/hot-cold-splitting Jul 1, 2022
Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

I'm late to this, but the changes look good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants