Fix wrong copy prop after ASG(promoted LCL_VAR with 1 field, call).#41197
Fix wrong copy prop after ASG(promoted LCL_VAR with 1 field, call).#41197sandreenko merged 4 commits intodotnet:masterfrom
ASG(promoted LCL_VAR with 1 field, call).#41197Conversation
ASG(promoted LCL_VAR with 1 field, call).ASG(promoted LCL_VAR with 1 field, call).
|
PTAL @AndyAyersMS @dotnet/jit-contrib |
AndyAyersMS
left a comment
There was a problem hiding this comment.
Changes LGTM.
You looked into the test failures? I took a quick look and the all seem unrelated.
Can you open a follow-up about potential missing VNs? Or maybe more broadly review all the places where we use lvaInSsa?
All tests were passing before the last commit (comment only change), so I think |
|
/backport to release/5.0 |
|
Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/221470846 |
Diffs for the library from #41100 :
diffs in
PEMethodSymbol:get_ExplicitInterfaceImplementationsare the fix, diffs inDeriveParametersandMakeSubmissionInitializationare correct, but the base version asm was correct as well (a replacement was wrong but the result was valid).0 diffs in master framework libraries (crossgen), pmi diffs:
The regression in
MethodBodySynthesizer:ConstructFieldLikeEventAccessorBody_Regularis the result of using an unstable mechanism to choose the copy prop replacement, so additional inserts (because we now add forASG(LCL_VAR, call)) change one replacement and it was less optimal. Both the base and the diff were producing correct asm.The same for
Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:BindLambdaBody(LambdaSymbol,DiagnosticBag,byref):BoundBlock:thisandLocalRewriter:RewriteWinRtEvent(BoundAddRemoveHandlerStatement,BoundEventAccess,bool):BoundStatement:this.Example of the copy diff.
in the base we were replacing
000595withV66from000033, in the diff we replace it withV07from33becauseV07andV66switched the order in the hashtable as a random side-effect from correct handling ofthere.
The BING spmi was showing me 3 diffs but it was actually the same method 3 times and it was generating a bad code before, now it is fixed, not sure why
removeDuplicatedid not delete them (-c 105012, 456212, 449211).The pri1 spmi collection has not found any diffs not caught by PMI.
Fixes #41100, will be ported to RC1.
Thanks Andy for the analysis, the repro test, and the proposed fix.