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

Removal of IBC infrastructure #68717

Merged
merged 8 commits into from
May 16, 2022

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Apr 29, 2022

This represents a course removal of the IBC infrastructure.

Questions on what else to remove can start with looking at the following under src/coreclr.

szZapBBInstr
szZapBBInstrDir
void Module::CreateProfilingData();
void Module::DeleteProfilingData();

Files:
src/coreclr/inc/corbbtprof.h
Under src/coreclr/tools/aot/ILCompiler.ReadyToRun/IBC

/cc @davidwrighton

Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

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

Good progress. I found 1 thing that shouldn't have been removed which is quite important (and will cause perf problems if you don't fix), but I also found a bunch of more stuff to delete.

src/coreclr/vm/clsload.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/ceeload.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/ceeload.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/ceeload.cpp Show resolved Hide resolved
src/coreclr/vm/zapsig.cpp Show resolved Hide resolved
src/coreclr/vm/ceeload.h Show resolved Hide resolved
@ghost ghost added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Apr 29, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review May 6, 2022 23:06
@davidwrighton
Copy link
Member

We also need to delete the /tuning switch, which exists to make use of the ProfileDataNode stuff you just deleted.

crossgen2\CommandLineOptions.cs: public bool Tuning;
crossgen2\CommandLineOptions.cs: syntax.DefineOption("tuning", ref Tuning, SR.TuningImageOption);
crossgen2\Program.cs: .UseIbcTuning(_commandLineOptions.Tuning)
crossgen2\Properties\Resources.resx:
ILCompiler.ReadyToRun\Compiler\ReadyToRunCodegenCompilationBuilder.cs: private bool _ibcTuning;
ILCompiler.ReadyToRun\Compiler\ReadyToRunCodegenCompilationBuilder.cs: public ReadyToRunCodegenCompilationBuilder UseIbcTuning(bool ibcTuning)
ILCompiler.ReadyToRun\Compiler\ReadyToRunCodegenCompilationBuilder.cs: _ibcTuning = ibcTuning;
ILCompiler.ReadyToRun\Compiler\ReadyToRunCodegenCompilationBuilder.cs: if (_ibcTuning)

All of that should go.


In reply to: 958156947

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 4f01044 into dotnet:main May 16, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the remove_ibc branch May 16, 2022 20:18
@ghost ghost locked as resolved and limited conversation to collaborators Jun 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants