JIT: Avoid reporting call-sites in MinOpts on all targets

[jakobbotsch]

When the call-sites have no interesting GC information we avoid reporting them, but this behavior was enabled only for x86/x64. Enable this on other platforms too.

Fix #103917

[jakobbotsch]

@jkotas do you know if there is any dependency on the call site locations by crossgen2? I'm not sure if the PREJIT condition can also be removed here.

[jkotas]

I do not think crossgen2 has any dependency on that. NativeAOT might have (unintentional) dependency on it - you may want to run NativeAOT outer loop.

The primary motivation for this feature was JIT throughput improvement. This condition is likely result of that thinking - spending the extra cycles for AOT is no big deal.

I do not see a problem with removing this condition.

[VSadov]

The change in https://github.com/dotnet/runtime/pull/102680/files#r1653150895 may make us sensitive to missing safepoints.

Mostly just a matter of changing asserts though. We know the caller site is GC-safe. (since we are in a GC-capable callee)

[jakobbotsch]

Do you want me to revert the PREJIT change for now?

[jakobbotsch]

/azp run runtime-nativeaot-outerloop, runtime-coreclr gcstress0x3-gcstress0xc

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[jakobbotsch]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-coreclr gcstress0x3-gcstress0xc

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-nativeaot-outerloop, runtime-coreclr gcstress0x3-gcstress0xc

[jakobbotsch]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[VSadov]

There are bunch of failures on NativeAOT that complain about size increase:

BUG: File size is not in the expected range (1228800 to 1638400 bytes). Did a libraries change regress size of Hello World?
Expected: 100
Actual: 1
END EXECUTION - FAILED

Those are expected, I assume.

However there are failures that look more like real failures:

DeadCodeElimination+TestUnmodifiableInstanceFieldOptimization+WithInitOnlyPropertyWrite
System.Exception: Exception of type 'System.Exception' was thrown.
   at DeadCodeElimination.ThrowIfPresentWithUsableMethodTable(Type, String) + 0x6a
   at DeadCodeElimination.TestTypeOfCodegenBranchElimination.Run() + 0x109
   at DeadCodeElimination.Run() + 0x68
   at TrimmingBehaviors!<BaseAddress>+0x29bf43
   at Program.<<Main>$>g__RunTest|0_0(Func`1, String) + 0x64
===== Test DeadCodeElimination.Run failed =====

DeadCodeElimination+TestUnmodifiableInstanceFieldOptimization+WithInitOnlyPropertyWrite
System.Exception: Exception of type 'System.Exception' was thrown.
   at DeadCodeElimination.ThrowIfPresentWithUsableMethodTable(Type, String) + 0x6a
   at DeadCodeElimination.TestTypeOfCodegenBranchElimination.Run() + 0x109
   at DeadCodeElimination.Run() + 0x68
   at TrimmingBehaviors!<BaseAddress>+0x29bf43
   at Program.<<Main>$>g__RunTest|0_0(Func`1, String) + 0x64
===== Test DeadCodeElimination.Run failed =====

Is that another expectation from the JIT (deadcode elimination), that is not guaranteed by MinOpts?

[MichalStrehovsky]

Those three failures are due to the forced minopts since they all expect optimizations to happen. Smoketests tend to have tests that test optimizations happen. I think we should run nativeaot outerloop with the forced minopts and ignore all failures in tests that are only failing because of the minopts. There shouldn't be many more tests like that.

Unfortunately outerloop is on the floor right now, trying to fix it in #110238.

[VSadov]

Those three failures are due to the forced minopts since they all expect optimizations to happen

Just want to clarify - the failures are only because the tests explicitly check for optimizations (the first case that simply looks for size regressions looks like that). Or if optimizations do not happen the codegen is actually incorrect? I can't tell about ThrowIfPresentWithUsableMethodTable cases.

[MichalStrehovsky]

Those three failures are due to the forced minopts since they all expect optimizations to happen

Just want to clarify - the failures are only because the tests explicitly check for optimizations (the first case that simply looks for size regressions looks like that). Or if optimizations do not happen the codegen is actually incorrect? I can't tell about ThrowIfPresentWithUsableMethodTable cases.

The DeadCodeElimination tests test that branch removal happened and looks for that using side effects (it's hard to test branch removal directly). The side effect in this case is whether we have a full MethodTable for something.

I believe the lines:

    if (IsTargetAbi(CORINFO_NATIVEAOT_ABI))
    {
        theMinOptsValue = true;
    }

in this PR currently set minopts for everything (for testing purposes) so branch removal doesn't happen.

[jakobbotsch]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

I went through all the failures and I do not see any suspicious ones; they all look to be expected failures from turning off optimizations.

[jkotas]

Thanks!