Enable scripting of SuperPMI collection of crossgen2 compilations #47513
Conversation
`superpmi.py collect` has a new `--crossgen2` option that specifies to run crossgen2 on all the assemblies specified with the `-assemblies` option. For example: ``` py c:\rt\src\coreclr\scripts\superpmi.py collect --crossgen2 -output_mch_path c:\win-x64-cg2.mch -assemblies c:\rt\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\System.Private.CoreLib.dll ``` There are currently quite a few failures during the replay "cleaning" phase with a System.Private.CoreLib collection: 5654 compilation failures out of 14705 functions, including 100 JIT asserts. In contrast, the crossgen(1) collection has 3 failures out of 20165 functions. So, there is some work to be done to make SuperPMI collection of crossgen2 compilations work well. The change in spmirecordhelper.h is to work around what appears to be a crossgen2 bug: when the JIT calls `getCallInfo`, sometimes crossgen2 sets sig.sigInst.methInstCount in the result CORINFO_CALL_INFO to non-zero, but it sets sig.sigInst.methInst to `nullptr`, which should be illegal: the count indicates how many elements are in the array, so you can?t have a null array with a non-zero count. Maybe the JIT doesn?t look at these fields in this scenario for some reason, but SPMI tries to serialize/de-serialize the array, and was crashing. Fixes dotnet#45909
ghost
added
the
area-CodeGen-coreclr
|
cc @dotnet/jit-contrib @dotnet/crossgen-contrib |
AndyAyersMS
left a comment
AndyAyersMS
left a comment
There was a problem hiding this comment.
LGTM. Thanks for setting this up.
trylek
left a comment
trylek
left a comment
There was a problem hiding this comment.
Looks good, thanks Bruce! I'll discuss the NotImplemented exceptions at our today Crossgen2 weekly sync-up meeting, these definitely need fixing.
FWIW, this is a "feature": runtime/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs Lines 654 to 655 in c084072 Populating this array is not cheap since we don't have unmanaged pointers to hand out as handles (like CoreCLR does). We need to actually make a handle and a pinned array and it's useless work if JIT is not going to look at them. If JIT starts to use them, we'll need to fix that, but for now we just don't do the work. |
| rsp_file_handle, rsp_filepath = tempfile.mkstemp(suffix=".rsp", prefix=root_output_filename, dir=self.temp_location) | ||
| with open(rsp_file_handle, "w") as rsp_write_handle: | ||
| rsp_write_handle.write(assembly + "\n") | ||
| rsp_write_handle.write("-o:" + crossgen2_output_assembly_filename + "\n") |
There was a problem hiding this comment.
Do we need to include all these assemblies for each test/library? Is R2R test doing the same?
There was a problem hiding this comment.
I added the same set of references that the one example test wrapper script I looked at was adding. I'm not really sure what the best minimal set is, or whether it needs to change per-scenario.
they could be not spmi specific, #47308 |
ghost
locked as resolved and limited conversation to collaborators
6 participants
superpmi.py collecthas a new--crossgen2option that specifiesto run crossgen2 on all the assemblies specified with the
-assembliesoption.For example:
There are currently quite a few failures during the replay
"cleaning" phase with a System.Private.CoreLib collection:
5654 compilation failures out of 14705 functions, including 100
JIT asserts.
In contrast, the crossgen(1) collection has 3 failures out
of 20165 functions.
So, there is some work to be done to make SuperPMI collection
of crossgen2 compilations work well.
The change in spmirecordhelper.h is to work around what appears to
be a crossgen2 bug: when the JIT calls
getCallInfo, sometimes crossgen2sets sig.sigInst.methInstCount in the result CORINFO_CALL_INFO to non-zero,
but it sets sig.sigInst.methInst to
nullptr, which should be illegal:the count indicates how many elements are in the array, so you can?t have a
null array with a non-zero count. Maybe the JIT doesn?t look at these fields in this
scenario for some reason, but SPMI tries to serialize/de-serialize the array,
and was crashing.
Fixes #45909