Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fix ADDR(IND(tree))=>tree transformation in morph. #27108

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

erozenfeld
Copy link
Member

ADDR(IND(tree)) => tree transformation was losing zero field
sequence annotation that was causing incorrect value numbering
and asserts in the CSE optimization.

Fixes #27107.

ADDR(IND(tree)) => tree transformation was losing zero field
sequence annotation that was causing incorrect value numbering
and asserts in the CSE optimization.

Fixes #27107.
@erozenfeld
Copy link
Member Author

@dotnet/jit-contrib PTAL

@erozenfeld
Copy link
Member Author

x64 frameworks diffs:

PMI Diffs for System.Private.CoreLib.dll, framework assemblies for  default jit
Summary:
(Lower is better)
Total bytes of diff: -7 (-0.00% of base)
    diff is an improvement.
Top file improvements by size (bytes):
          -4 : System.Private.Xml.dasm (-0.00% of base)
          -3 : System.Private.DataContractSerialization.dasm (-0.00% of base)
2 total files with size differences (2 improved, 0 regressed), 127 unchanged.
Top method improvements by size (bytes):
          -4 (-0.37% of base) : System.Private.Xml.dasm - XmlTextReaderImpl:ReadData():int:this
          -3 (-0.22% of base) : System.Private.DataContractSerialization.dasm - XmlCanonicalWriter:WriteStartAttribute(ref,int,int,ref,int,int):this
Top method improvements by size (percentage):
          -4 (-0.37% of base) : System.Private.Xml.dasm - XmlTextReaderImpl:ReadData():int:this
          -3 (-0.22% of base) : System.Private.DataContractSerialization.dasm - XmlCanonicalWriter:WriteStartAttribute(ref,int,int,ref,int,int):this
2 total methods with size differences (2 improved, 0 regressed), 202974 unchanged.
1 files had text diffs but not size diffs.
System.Private.CoreLib.dasm had 762 diffs

Copy link

@briansull briansull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@erozenfeld
Copy link
Member Author

x64 benchmarks diffs:

PMI Diffs for benchstones and benchmarks game in f:\coreclr6_1\bin\tests\Windows_NT.x64.Release for  default jit
Summary:
(Lower is better)
Total bytes of diff: -11 (-0.00% of base)
    diff is an improvement.
Top file improvements by size (bytes):
         -11 : Benchstones\BenchI\TreeInsert\TreeInsert\TreeInsert.dasm (-1.13% of base)
1 total files with size differences (1 improved, 0 regressed), 81 unchanged.
Top method improvements by size (bytes):
         -11 (-7.14% of base) : Benchstones\BenchI\TreeInsert\TreeInsert\TreeInsert.dasm - TreeInsert:Bench():bool:this
Top method improvements by size (percentage):
         -11 (-7.14% of base) : Benchstones\BenchI\TreeInsert\TreeInsert\TreeInsert.dasm - TreeInsert:Bench():bool:this
1 total methods with size differences (1 improved, 0 regressed), 2054 unchanged.

@erozenfeld
Copy link
Member Author

The change unblocked some CSEs in the benchmark method showing an improvement.

@erozenfeld erozenfeld merged commit 1c34c94 into dotnet:master Oct 9, 2019
@erozenfeld erozenfeld deleted the Fix27107 branch October 9, 2019 22:13
erozenfeld added a commit to erozenfeld/coreclr that referenced this pull request Oct 9, 2019
dotnet#27108 fixed #27107 but the tests had an incorrect placeholder name.

Rename the test directory and files.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JIT: assert in CSE optimization
3 participants