Fix devirtualization across genericness in the hierarchy #108442
Conversation
This code was trying to answer question: "Was this method overriden by something else in a more derived class"? It was walking the base hierarchy in canonical form, but that was leading to methods not resolving at all. The fix is to walk the non-canonical hierarchy and canonicalize after we resolved everything. I ran into this in dotnet#108379 that unlocked more whole program devirtualization and `StringSearchValuesBase` is in this shape.
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@dotnet/ilc-contrib anyone have opinion on backport? This is a .NET 9 regression that nobody ran into yet, but it's bad codegen (wrong method getting called). |
|
I think silent bad codegen that is regression from last release meets the servicing bar. The targeted repro looks very simple, likely to be hit by somebody. |
|
I agree, regression from .NET 8 makes it worth taking. I will bring it to tactics |
|
/backport to release/9.0-staging |
|
Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/11136710712 |
|
@agocke an error occurred while backporting to release/9.0-staging, please check the run log for details! Error: The specified backport target branch release/9.0-staging wasn't found in the repo. |
|
/backport to release/9.0 |
|
Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/11136775970 |
|
@agocke backporting to release/9.0 failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix devirtualization across genericness in the hierarchy
Using index info to reconstruct a base tree...
M src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs
CONFLICT (content): Merge conflict in src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix devirtualization across genericness in the hierarchy
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@agocke an error occurred while backporting to release/9.0, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
This code was trying to answer question: "Was this method overriden by something else in a more derived class"? It was walking the base hierarchy in canonical form, but that was leading to methods not resolving at all. The fix is to walk the non-canonical hierarchy and canonicalize after we resolved everything. I ran into this in dotnet#108379 that unlocked more whole program devirtualization and `StringSearchValuesBase` is in this shape.
Conflicts because of #108116. Not the first time random "fix typos"/"reformat code" caused extra busywork for nothing. Now I get to spend 10 minutes on something that should have been 10 seconds. |
This code was trying to answer question: "Was this method overriden by something else in a more derived class"? It was walking the base hierarchy in canonical form, but that was leading to methods not resolving at all. The fix is to walk the non-canonical hierarchy and canonicalize after we resolved everything. I ran into this in dotnet#108379 that unlocked more whole program devirtualization and `StringSearchValuesBase` is in this shape.
…108470) This code was trying to answer question: "Was this method overriden by something else in a more derived class"? It was walking the base hierarchy in canonical form, but that was leading to methods not resolving at all. The fix is to walk the non-canonical hierarchy and canonicalize after we resolved everything. I ran into this in #108379 that unlocked more whole program devirtualization and `StringSearchValuesBase` is in this shape. Co-authored-by: Jeff Schwartz <jeffschw@microsoft.com>
This code was trying to answer question: "Was this method overriden by something else in a more derived class"? It was walking the base hierarchy in canonical form, but that was leading to methods not resolving at all. The fix is to walk the non-canonical hierarchy and canonicalize after we resolved everything. I ran into this in dotnet#108379 that unlocked more whole program devirtualization and `StringSearchValuesBase` is in this shape.
This code was trying to answer question: "Was this method overriden by something else in a more derived class"? It was walking the base hierarchy in canonical form, but that was leading to methods not resolving at all. The fix is to walk the non-canonical hierarchy and canonicalize after we resolved everything.
I ran into this in #108379 that unlocked more whole program devirtualization and
StringSearchValuesBaseis in this shape. It is hittable without that optimization though, like the regression test shows.Cc @dotnet/ilc-contrib