Initial PDB / PerfMap support in Crossgen2 + System.Private.CoreLib switchover to use Crossgen2 #47019
Conversation
trylek
requested review from
nattress,
cshung,
davidwrighton and
AntonLapounov
|
/cc @dotnet/crossgen-contrib |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ObjectWriter/MapFileBuilder.cs
Outdated
Show resolved
Hide resolved
| @@ -205,6 +226,20 @@ public void SaveCsv(string nodeStatsCsvFileName, string mapCsvFileName) | |||
| } | |||
| } | |||
|
|
|||
| public void SavePdb(string pdbPath, string dllFileName) | |||
There was a problem hiding this comment.
It feels weird having MapFileBuilder be the thing that writes PDBs and Perf maps since its purpose is to write a text summary file at the end of compilation. The map file builder's data structures at this point are a nice model of node to file location which makes it handy for these purposes. Maybe we could split the concepts into a class containing mapping details (shareable by map, perfmap, pdb generation)?
There was a problem hiding this comment.
Fixed in 9th commit - I have split the logic into an "output file info collector" (OutputInfoBuilder) and the actual emitters using it - MapFileBuilder and SymbolFileBuilder, thanks for the suggestion.
|
Appears there are still some lingering issues on x86? |
I have moved the PDB writer code to a new assembly ILCompiler.Diagnostics so that it can be reused by Crossgen2. I have also added an initial trivial implementation of the PerfMap writer. Thanks Tomas
Based on Simon's suggestion I moved the PDB / PerfMap-specific code to a new file SymbolFileBuilder as I concur that its squatting in the MapFileBuilder was somewhat hacky. Both classes use a new helper class ObjectInfoBuilder that collects the information common to both - I was reluctant to make it a base class of the *FileBuilder's as we'd have to collect all the elements twice. Thanks Tomas
|
I have addressed initial Manish's and Simon's PR feedback and the change finally passes PR testing. I have launched an outerloop run and I believe the change is ready to be merged in. Please take another look when you have a chance and let me know what additional comments need addressing before this goes in. |
ghost
locked as resolved and limited conversation to collaborators
(*) Move PDB writer support out of R2RDump to a new assembly ILCompiler.Diagnostics;
(*) Add initial PerfMap writer support parallel to PDB emitter;
(*) Add Crossgen2 support for emitting PDB / PerfMap using the ILCompiler.Diagnostics assembly;
(*) Add support for PDB generation to R2RTest;
(*) Add support for optional PDB generation to the src\tests\build.cmd script (PDB is only supported on Windows).
Thanks
Tomas