JIT: Generalize check for full interruptibility #95299
Conversation
Get rid of the dependencies on dominators and reachability and switch it to a simple cycle check. Switch MinOpts to also use the cycle check. This has a bit mixed results, but our largest SPMI contexts show that this is beneficial TP wise, and it should generate smaller GC info.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
ghost
assigned 
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsGet rid of the dependencies on dominators and reachability and switch it to a simple cycle check. Switch MinOpts to also use the cycle check. This has a bit mixed results, but our largest SPMI contexts show that this is beneficial TP wise, and it should generate smaller GC info.
|
|
This change finds 79 cases in our win-x64 collections where the old logic did not mark things as fully interruptible but where the new logic does. They are all pretty gnarly, but one of them is this one, for this function. The new logic finds the cycle BB03 -> BB04 -> BB05 -> BB21 -> BB03 which does not involve any GC safe points. I think that's the |
|
/azp run runtime-coreclr gcstress0x3-gcstress0xc |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
cc @dotnet/jit-contrib PTAL @amanasifkhalid @AndyAyersMS. This fixes the issue we discussed in #94239 @amanasifkhalid, and gets rid of one loop headache of the future (the reliance of dominators/reachability sets). Diffs. Some minor throughput regressions in FullOpts; in MinOpts there are diffs in both directions in terms of throughput, so I decide to take this churn in order to simplify the code and make MinOpts less conservative. |
There was a problem hiding this comment.
Nice cleanup! Glad to see the biggest MinOpts TP regressions still aren't as bad as the ~0.5% hit we got in #94239 when I accidentally fixed the partial reporting bug.
Yeah, seems likely we could be smarter. IIRC the GC info format also supports mixing partial and full interruptibility within the same method, so we could only report the cycles as fully interruptible. |
Get rid of the dependencies on dominators and reachability and switch it to a simple cycle check.
Switch MinOpts to also use the cycle check. This has a bit mixed results, but our largest SPMI contexts show that this is beneficial TP wise, and it should generate smaller GC info. At the same time it fixes the bug identified in #94239 (comment).