exclude crossgen from build.

[mangod9]

flighting a change, I am seeing a weird build break after removing cg1 so would be interesting to see if that repros on CI.

[jkoritzinsky]

Based on the error I see in the CI build, you can just remove the install_clr call at line 17 of crosscomponents.cmake if you're removing crossgen1 from the build.

[mangod9]

Thanks, trying it now.

[jkoritzinsky]

Looks like all of the failing tests were specifically testing crossgen1 in some fashion. Some of them can be changed to use crossgen2. Others can have the crossgen1 portion deleted or might be able to be fully deleted.

[mangod9]

yeah will have to switch them to use cg2.

[mangod9]

@dotnet/crossgen-contrib FYI. Some notes:

1. This is a first step in removing crossgen artifacts from the repo. We need to decide how far we go with it in 6.
2. I didn't have to update anything in the installer, so need to figure out how sdk consumes cg1 and what changes are required on SDK side @AntonLapounov
3. I was hitting a weird failure in Microsoft.NET.CrossGen.targets writing PDBs after excluding crossgen from building. That doesnt repro on CI jobs, so must something local to my dev machine. Though CI builds components separately (clr, installers) so that could be a difference.

[trylek]

For the installer, this is the bit that chooses whether we're using CG1 vs. CG2 in the SDK:

runtime/src/installer/pkg/sfx/Microsoft.NETCore.App/ReadyToRun.targets

Line 21 in 57fc4c2

<PublishReadyToRunUseCrossgen2>true</PublishReadyToRunUseCrossgen2>

(That was the one that disappeared due to a bad merge I made earlier this year and that's why CG2 switchover of the framework actually only took place in P4 rather than in P2 as we had originally believed.)

As a meta-point, I believe that the overall goal for .NET 6 is to suppress Crossgen1 by default but not making it super difficult for specific customers to downlevel to Crossgen1 in case they happen to be using some obscure construct that turns out to hit a previously unseen CG2 bug. The bit in CoreCLR build that decides whether to compile SPC using Crossgen1 vs. Crossgen2 is UseCrossgen2,

runtime/src/coreclr/crossgen-corelib.proj

Line 6 in 57fc4c2

<UseCrossgen2 Condition="'$(UseCrossgen2)' == ''">true</UseCrossgen2>

Could we perhaps re-activate Crossgen1 by setting the environment variable UseCrossgen2 to false?

[trylek]

For what actually happens in the SDK repo, this is the MSBuild script that drives Crossgen compilation:

https://github.com/dotnet/sdk/blob/main/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

The C# code dealing with lower-level details of CG compilation is in the following three files:

https://github.com/dotnet/sdk/blob/main/src/Tasks/Microsoft.NET.Build.Tasks/PrepareForReadyToRunCompilation.cs
https://github.com/dotnet/sdk/blob/main/src/Tasks/Microsoft.NET.Build.Tasks/ResolveReadyToRunCompilers.cs
https://github.com/dotnet/sdk/blob/main/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs

[trylek]

As an additional bit of fun there's a backwards compatibility requirement in the SDK repo, basically a particular version of the SDK repo must be capable of building all earlier versions of the runtime. The idea is that you may need to choose at build-time what version you're targeting and the SDK must be able to pull down the proper nugets and do the right thing using possibly an older version of the compiler / runtime.

[trylek]

It would be also very interesting to hear what @davidwrighton has to say here as his efforts on streamlining the parallel runtime / SDK development hit exactly these areas.

[trylek]

Frankly speaking, my personal opinion is that the SDK code is hugely overcomplicated and some of it should be siphoned off to the runtime repo so that the SDK can use an easy runtime hamburger to "Crossgen the darn thing" that would naturally evolve in the runtime repo next to the compiler. The tricky part is to design a clean boundary that would allow this refactoring, I believe that's what David's currently working on.

[trylek]

Also explicitly adding @jkotas and @richlander as this is a fundamental transition they need to be aware of and participate on. I think the primary question is whether we want to continue publishing the Crossgen1 compiler in .NET 6 nuget packages. As a first cut I think we probably do as that would let customers downlevel to Crossgen1 by just setting PublishReadyToRunUsingCrossgen2 to false. Forcing customers hitting potential CG2 bugs to use a custom build of the runtime repo is something I don't feel competent to estimate an impact of.

[jkotas]

I believe that the overall goal for .NET 6 is to suppress Crossgen1 by default but not making it super difficult for specific customers to downlevel to Crossgen1 in case they happen to be using some obscure construct that turns out to hit a previously unseen CG2 bug.

We have been dropping test coverage for CG1. It is likely that there are CG1 bugs that we do not know about already.

I do not think we need to keep CG1 as a fallback option. If somebody runs into blocking CG2 bug after we ship, we will service it like any other bug.

[mangod9]

I am also of the opinion that we should roll forward with any fixes rather than have to fall back to crossgen. Hoping any blocking issues would be reported in preview4 or 5 since cg2 is the default in the sdk.

[mangod9]

Hi @trylek, I am seeing few failures in R2R outerloop runs (after switching them to use cg2) which appear to type layout issues. Assume some of these would be resolved with your PR?

[trylek]

I hope so, in fact I've already received a green outerloop run including the CG2 legs on the PR.

[BruceForstall]

These seem like weird changes: why not just set RunCrossGen2=1 in the caller instead?

[mangod9]

yeah ideally should change the callers, but wanted to keep the changes limited in case we have to revert. Also going forward once we really remove cg1, RunCrossgen==RunCrossgen2, and actually the line below this should be removed.

[BruceForstall]

Please run the "r2r" and "r2r-extra" pipelines

[BruceForstall]

And 'outerloop'

[mangod9]

sure will run r2r*. Have already run outerloop, few 32-bit failures are being fixed by another PR.

[trylek]

Please note that today r2r pipelines are hard-coded to use Crossgen1, we'll need to adjust the yaml scripts to start using Crossgen2 otherwise they naturally blow up in the absence of Crossgen1.

[BruceForstall]

@trylek Won't the change in this PR to run-test-job.yml force all jobs that "look like" they're going to do crossgen1 actually test crossgen2 instead?

[trylek]

@BruceForstall I think you're right - it's not actually the change to run-test-job.yaml, that's just changing the default for the input variable, but the change to publishhelixwitharcade does force the runtime to use CG2 always.

[mangod9]

@BruceForstall r2r results are comparable to outerloop (not sure if there is any difference in their configs). r2r-extra had similar failures in 32-bit runs which hoping will be resolved by Tomas' PR. There are one or two failures in x64 which might need investigation but don't seem to be a blocker.

[BruceForstall]

Should the r2r and crossgen2 pipelines be retired, in favor of crossgen2-outerloop? It seems like the crossgen2-outerloop job is a superset. Although it is scheduled less frequently.

[clamp03]

Wow. To me who should keep FNV internally, it is really surprising!!!
@mangod9 Could you make a Github issue that manages crossgen1 removal? It is really helpful to me. Thank you.

[mangod9]

Hi @clamp03 -- we have been tracking replacing cg1 with cg2 as part of this userstory: #43653. What info are you looking for as part of the crossgen1 removal issue?

[clamp03]

@mangod9 Thanks for the comment.
I think many PRs will remove and clean up crossgen1 codes in runtime.
I need a way to know the PRs as soon as possible because if I miss some, I think that keeping FNV may be a very hard job.

[AntonLapounov]

@clamp03 I think the plan is to keep the old crossgen code for .NET 6 and remove it later.

[mangod9]

I have created this to track removal: #54129. We will update as cleanup progresses.

[clamp03]

@mangod9 Thank you so much!!!