Skip native composite ReadyToRun images as inputs to Crossgen2

[trylek]

According to the issue #49247
a known pre-existing Crossgen2 bug is that it fails when presented
with the components of a previous Crossgen2 compilation in the
composite mode. This is because Crossgen2 lacks proper logic to
recognize the composite images and mistakes them for currently
unsupported managed C++ MSIL assemblies. This change adds the extra
check; I have also unified it between Crossgen2 and R2RDump.

Thanks

Tomas

Fixes: #49247
Fixes: #60381

[jkoritzinsky]

This test uses crossgen2 directly in its custom scripting. I think we should mark this with CrossgenIncompatible in the project file.

[jkoritzinsky]

Ignore my last comment. We don't have a CrossgenIncompatible property similar to GCStressIncompatible. Using issues.targets is the right thing here.

[trylek]

@jkoritzinsky - well, we do have the property CrossGenTest, e.g. here:

runtime/src/tests/Loader/classloader/generics/Instantiation/Negative/abstract04.ilproj

Line 7 in a0824ba

<CrossGenTest>false</CrossGenTest>

I do see that the test uses explicit CG2 invocation and I believe it should be trivial to convert it to use the standard test scripting logic akin to what we do e.g. in the test

https://github.com/dotnet/runtime/blob/main/src/tests/readytorun/crossgen2/crossgen2smoke.csproj

I still however find it weird how come the test only seems to fail in CG2 runs - or are you aware of instances where it fails in JITted outerloop runs?

Thanks

Tomas

[jkoritzinsky]

That's the name... I didn't realize it didn't follow the pattern of the rest. Either using that property or following a similar mechanism as the smoke test would be something I'd agree with.

[trylek]

Thanks Jeremy for your feedback. I have converted the test to our standard infra logic for running tests with forced CG2 execution. According to my local testing the test seems to be passing so for now I removed the issues.targets entry as I suspect the apparent failures may have been unintended side effects of the custom CG2 execution logic. For now I plan to close the issue assuming the PR test run finishes fine; we will reopen the issue if the test turns out to still fail in some situations.

[jkoritzinsky]

I don't think your change has the same semantics as the old scripts. Can you take another look?

[trylek]

Thanks Jeremy, I have fixed the execution argument --runCustomTest, are you aware of any additional differences between the current test behavior and the simplified version using the stock CG2 compilation scripting?

[jkoritzinsky]

Yes, it looks like the test wants both the non-R2R and the R2R versions of the DLL. It wants the R2R one in a separate R2R subfolder.

Additionally, it uses custom scripts to actually do the test run: https://github.com/dotnet/runtime/blob/main/src/tests/baseservices/TieredCompilation/RunBasicTestWithMcj.cmd
https://github.com/dotnet/runtime/blob/main/src/tests/baseservices/TieredCompilation/RunBasicTestWithMcj.sh

[trylek]

Thanks Jeremy for pointing that out, I was originally confused by the fact that the custom scripts are named the same as the default scripts generated by CLRTest.Execute.targets. In such case neither of the options I proposed is the right fix for the issue, which is in fact

#49247

When the test runs in a Crossgen2 composite run, the custom tooling for running Crossgen2 actually causes the compiler to be run twice and a known deficiency makes the second compilation fail because it cannot make sense of the composite image produced by the prior compilation. I'll update this PR by removing changes to the scripting and by fixing the Crossgen2 issue. The scripting can be also cleaned up but that's orthogonal to the root issue.

Thanks

Tomas

[trylek]

/cc @dotnet/crossgen-contrib

[janvorli]

LGTM, thank you!

[trylek]

The assertion failure op2->TypeIs(TYP_LONG) is apparently known and tracked under #60624, merging in.