-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Delete CMake infrastructure for crossgen1 and remove dead code directories #57614
Delete CMake infrastructure for crossgen1 and remove dead code directories #57614
Conversation
Code references have not been removed. The following defines are now never defined and all blocks guarded by them can be considered dead code: - [ ] CROSSGEN_COMPILE - [ ] FEATURE_NATIVE_IMAGE_GENERATION - [ ] FEATURE_READYTORUN_COMPILER (outside of the coreclr/jit directory)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As my former lead Slava used to say, red is always good. Thank you!
Hello @jkoritzinsky! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 minutes, a condition that will be fulfilled in about 6 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
@@ -1,168 +0,0 @@ | |||
::mscorlib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: also found some crossgen1
hits in few other places:
$ git grep -li crossgen1 :/
docs/design/coreclr/botr/vectors-and-intrinsics.md
src/coreclr/crossgen-corelib.proj
src/coreclr/tools/aot/ILCompiler.Diagnostics/PerfMapWriter.cs
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ObjectWriter/SymbolFileBuilder.cs
src/coreclr/tools/r2rtest/Crossgen2Runner.cs
src/coreclr/tools/r2rtest/CrossgenRunner.cs
src/tasks/Crossgen2Tasks/PrepareForReadyToRunCompilation.cs
src/tasks/Crossgen2Tasks/RunReadyToRunCompiler.cs
src/tests/readytorun/crossboundarylayout/crossboundarytests.cmd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove those occurrences in a future PR. I want to keep this one focused on the native build for simplicity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect we also have some residua in the test build infra, around files like CLRTest.Crossgen.targets, R2RTest and such. I'll float their cleanup as one of the ideas for the Quality week next week.
Cc @dotnet/samsung crossgen1 is getting removed here, as indicated before - note this is targeting main branch (.NET 7). .NET 6 got snapped into the release/6.0 branch. |
Could you also remove the references to FEATURE_PREJIT and FEATURE_NGEN_RELOCS_OPTIMIZATIONS from clrdefinitions.cmake? That code is also now completely and totally dead as of crossgen no longer being in our build. |
I cannot resist sharing this small reminder regarding our recent journey. Thanks so much to everyone involved! From: Zach Montoya zamont@microsoft.com Hi Jan, Would you mind taking an hour sometime this week to give me (and possibly Simon & Tomas if interested) a high-level talk on Ready to Run? I have no experience with Ready to Run at the moment so I’d love to get a brain dump from you so I have an improved understanding of it, especially since Sergiy’s new intern will be arriving on Monday and I’ll be coaching her through the internship. If that’s fine with you, then Simon & Tomas let me know as well if you would be interested so we can coordinate a good meeting time. Thanks, Just for context, the intern, Amy Yu, ended up writing the first version of the R2RDump tool. |
Test failures are known and linked. Only build failure is the feed instability. I'm going to merge. |
This removal cut an execution of |
Thanks! Great impactful cleanup indeed! 8-) |
I've split this into individual commits that should each build successfully and can be reviewed either individually or all together.
Remove all CMake targets that were only used to compose crossgen1.
Delete any folders where the files within were only used to compile crossgen1-only CMake targets.
Delete all CMake code based on the
CROSSGEN_COMPONENT
CMake target property.The following defines are now never defined and all blocks guarded by them can be considered dead code. We can now
Contributes to #54129 and #53007
cc: @dotnet/crossgen-contrib @AaronRobinsonMSFT @elinor-fung @jkotas @agocke