-
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
Default DynamicCodeSupport to true for CoreCLR #103594
Conversation
1d2d51e
to
d582d5f
Compare
<!-- Prevent getting DynamicCodeSupport=true default from ILLink targets that are imported | ||
with the Sdk.targets import above, before Native AOT defaults get a chance to set it --> | ||
<DynamicCodeSupport>false</DynamicCodeSupport> |
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.
Anyone have opinions about switching crossgen2 to use the package version of ILC, same as cdacreader and ILC itself? I don't know if it's useful to use the live version and it's probably more trouble than it's worth - looks like we stabilized on using the package version instead.
(This comment is not proposing work for this PR and we'd only do this after #103375 merges.)
@dotnet/ilc-contrib @am11 for thoughts.
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.
Pros: live version gives (an additional) validation from CG2 (a real-world app) at the PR stage.
Cons: it slows down the development a bit; since R2R has separate code paths which are normally stabilized when next preview release is nearing. e.g. #103537 (comment)
I think with ongoing code unification and converging implementations, it appears that we are leaning towards fixing issues earlier during the development phase; so using live builds is a general goodness. Also, some downstream repos like aspnetcore run into issues when a bug makes its way into the package undetected, since they have direct runtime dependency without waiting for the next SDK preview release.
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.
My typical opinion is to use "live versions" when possible.
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.
Anyone have opinions about switching crossgen2 to use the package version of ILC
I would be fine with that. I do not think that maintaining two different schemes is worth it, and switching everything to live build is way too complicated.
it appears that we are leaning towards fixing issues earlier during the development phase; so using live builds is a general goodness. Also, some downstream repos like aspnetcore run into issues when a bug makes its way into the package undetected, since they have direct runtime dependency without waiting for the next SDK preview release.
This will get better with unified build codeflow in not-so-distant future. The unified build codeflow is going to be dotnet/runtime -> dotnet/dotnet and the validation of that flow is going to do the multistage build that exercises live-built toolset. If AOT compiler bug sneaks in, we will see a break in that flow.
Fixes #80398