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.
3 participants
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