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

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