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

[Hot cold splitting] Support split-eh #1918

Closed
10 tasks done
cshung opened this issue Jun 30, 2022 · 4 comments
Closed
10 tasks done

[Hot cold splitting] Support split-eh #1918

cshung opened this issue Jun 30, 2022 · 4 comments
Labels
area-hot-cold-splitting Hot cold splitting support in crossgen2

Comments

@cshung
Copy link
Member

cshung commented Jun 30, 2022

As the PR dotnet/runtime#71236 is merged, we should update crossgen2 to support cold funclets.

Here are a few things to do:

On the JIT side:

  • Consider making eh-split configurable. As you can see, there is a lot of work on the crossgen2/vm side to catch up. It would be bad if we can't update the lab branch before we have done all of these.

  • Make sure the JIT reports the unwind info accurately. The expectation is we get exactly one pair of reserveUnwindInfo and allocUnwindInfo for each runtime function. There are only two cases, case 1 is that it is the main function or a funclet, then in the JIT, we should have generated the right unwind info data, we need to right data and the right flags. Another case is that it is a cold block that is not a funclet, and we would like to get a callback with the right range but with a unwindSize == 0 so that we know we have to generate a chained unwind info.

On the crossgen2 side:

  • Expect more than 1 calls to reserveUnwindInfo here and allocUnwindInfo here for the cold code. They are both in CorInfoImpl.cs and it is our entry point to the JIT questions. Before eh-splitting, the only cold funclet appears to be always zero sized, so I just ignored it here. After eh-splitting, there could be non-trivial unwind-info allocated with the cold funclet, and we need to remember them in a structure parallel to _frameInfos.

  • Adapt MethodGCInfoNode.CalculateFuncletOffsets here. Before eh-split, the cold code is always just some code that has no interest unwind info on its own, so we are expected to always generate exactly a default chained unwind info for it. Therefore, if we have cold code, the offset to that cold code unwind info is always at the end, and there is always exactly one of it. This layout (i.e. generating the cold code unwind info right after its hot counter part) is not ideal from the density point of view, and the work to optimize it is tracked as this issue. Regardless of whether we pack or not, we need to return the right offsets to the cold funclet unwind data.

  • AdaptMethodGCInfoNode.GetData here to emit the cold funclet unwind data. Note how I generated the chained unwind info here. The rest should follow a similar pattern.

  • Adapt RuntimeFunctionsTableNode.GetData here to generate the runtime function table entry. With the CalculateFuncletOffsets change, we should get the offsets to the unwind info for the cold code, but we also need to generate the entries in the runtime function table to point to them. Note that we are hard coded to generate exactly one entry in the table in this block, so we need to generate the right amount of entries pointing to the right locations. That information should be collected earlier in the parallel structure. Be careful about the mapping variable so that we generate only one pair per cold block (not per runtime function entry).

On the VM side:

  • Adapt ReadyToRunJitManager::JitCodeToMethodInfo here to account for the possibility of cold funclets. In particular, if the cold block contains multiple funclets, it is possible that the corresponding runtime function is not on the scratch map, so this check will fail. We need some solution to figure out how to solve that.

  • Avoid reporting the cold code is not a funclet here. But eliminating the loop is wrong because it could be a cold block that is not a funclet (which means it's start address is not the same as the function entry point), we probably need to inspect the unwind info and look for the chained unwind info flag.

  • Adapt ReadyToRunJitManager:: JitTokenToMethodRegionInfo here. Right now the code assumed the method can have only one cold runtime function, so it only subtract the size of one runtime function.

  • Adapt ReadyToRunJitManager::JitCodeToMethodInfo here. Right now, the m_relOffset is computed simply by subtracting the current program counter from the beginning of the method, that will include space between the hot and cold code, we need to discount that.

The above is all that I am aware of, there is, obviously, the possibility of things that I am unaware of. We need to make sure we test and fix them all.

@cshung cshung added the area-hot-cold-splitting Hot cold splitting support in crossgen2 label Jun 30, 2022
@amanasifkhalid
Copy link
Member

amanasifkhalid commented Jun 30, 2022

Thanks for getting this started!

Consider making eh-split configurable.

There is COMPlus_JitNoProcedureSplittingEH in the JIT already; setting this to "*" would disable splitting for all methods with EH funclets. Can we use this for now?

Make sure the JIT reports the unwind info accurately.

I've started looking into this; looks like the JIT doesn't have hot/cold unwind info reporting implemented at all at the moment. For example, callers to eeReserveUnwindInfo are hard-coded to set unwindSize = 0 for cold code. For testing the JIT with fake-splitting, we treat all code as "hot" when generating unwind info, so generating hot and cold unwind info hasn't been tested yet. Now that the prototype is up and running, I can make implementing this my first priority.

@amanasifkhalid
Copy link
Member

amanasifkhalid commented Jun 30, 2022

I've pulled from runtime-main to add JIT support for EH splitting, and have disabled such splitting by default.

Note: We can use COMPlus_JitNoProcedureSplittingEH to disable EH splitting, but the CONFIG_METHODSET does not support default values, so I couldn't automatically set this variable without re-architecting the method. To minimize churn, I've instead hard-coded opts.compProcedureSplittingEH = false in src/coreclr/jit/compiler.cpp. We can just delete this line to test EH splitting, and ultimately remove it once this issue has been resolved.

@amanasifkhalid
Copy link
Member

Draft PR of Crossgen work here: #1926

@amanasifkhalid
Copy link
Member

Eugenio's PR closes out the final VM tasks: #1940

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-hot-cold-splitting Hot cold splitting support in crossgen2
Projects
None yet
Development

No branches or pull requests

2 participants