Split out and generalize tailcall IR validation/tailcall profile adjustments #69941
Conversation
* Split the IR validation after tailcalls into a separate function. Previously it was intertwined with updating of profile weights for follow-up blocks. * Generalize the validation to use a tree walk and handle more cases. This fixes an assertion failure seen in some PGO runs. * Generalize the updating of profile weights for follow-up blocks. Previously this was only updating profile weights for one follow up blocks, but there can be an arbitrary number of successor blocks due to inlining. Fix dotnet#69939
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
ghost
assigned 
|
Tagging subscribers to this area: @JulieLeeMSFT Issue Details
Fix #69939 I was not able to come up with a simple test case. It requires a quite specific sequence of inlines and GDVs to produce IR that ends up with the NOP we were failing on.
|
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
cc @dotnet/jit-contrib PTAL @AndyAyersMS |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
that piece of validation caused problems many times already, maybe it's time to just delete it 😄 |
I think the problematic part here is having the complicated IR after tailcalls, ideally we should teach the inliner to avoid all this no-op IR it adds instead so that we can validate this condition much easier. I do think we should have validation that the tailcalls we are doing are legal. |
|
Gonna rerun jitstress now that we've merged #69946... |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Most of the libraries-jitstress runs failed with "A call to an Azure DevOps api returned 401, which may indicate a bad 'System.AccessToken' value.", will give it another retry. |
|
/azp run runtime-core libraries-jitstress |
|
No pipelines are associated with this pull request. |
|
/azp run runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Sigh, the failure was #69854. Hopefully I've fixed it for libraries-jitstress now. |
|
/azp run runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Previously it was intertwined with updating of profile weights for
follow-up blocks.
This fixes [libraries-pgo] Assertion failed 'asgNode->OperIs(GT_ASG)' in during 'Morph - Global' #69939.
Previously this was only updating profile weights for one follow up
blocks, but there can be an arbitrary number of successor blocks due
to inlining.
Fix #69939
I was not able to come up with a simple test case. It requires a quite specific sequence of inlines and GDVs to produce IR that ends up with the NOP we were failing on.