Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix optComputeLoopSideEffects to account for HWI stores #61911

Merged

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Nov 22, 2021

Otherwise we can end up not seeing the loop has memory havoc.

Also added an assert that will prevent this issue from arising in the future.

A few small diffs in methods where we had such loops.

Win-x64 diffs

benchmarks.run.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 7065605 (overridden on cmd)
Total bytes of diff: 7065604 (overridden on cmd)
Total bytes of delta: -1 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
          -1 : 21732.dasm (-0.07% of base)

1 total files with Code Size differences (1 improved, 0 regressed), 0 unchanged.

Top method improvements (bytes):
          -1 (-0.07% of base) : 21732.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this

Top method improvements (percentages):
          -1 (-0.07% of base) : 21732.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this

1 total methods with Code Size differences (1 improved, 0 regressed), 0 unchanged.


libraries.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 45295203 (overridden on cmd)
Total bytes of diff: 45295192 (overridden on cmd)
Total bytes of delta: -11 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -11 : 174250.dasm (-0.72% of base)

1 total files with Code Size differences (1 improved, 0 regressed), 0 unchanged.

Top method improvements (bytes):
         -11 (-0.72% of base) : 174250.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this

Top method improvements (percentages):
         -11 (-0.72% of base) : 174250.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this

1 total methods with Code Size differences (1 improved, 0 regressed), 0 unchanged.


Full diffs.

Otherwise we can end up not seeing the loop has memory havoc.

Also added an assert that will prevent this issue from arising in the future.
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 22, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed community-contribution Indicates that the PR has been added by a community member labels Nov 22, 2021
@ghost
Copy link

ghost commented Nov 22, 2021

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

Otherwise we can end up not seeing the loop has memory havoc.

Also added an assert that will prevent this issue from arising in the future.

A few small diffs in methods where we had such loops.

Win-x64 diffs

benchmarks.run.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 7065605 (overridden on cmd)
Total bytes of diff: 7065604 (overridden on cmd)
Total bytes of delta: -1 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
          -1 : 21732.dasm (-0.07% of base)

1 total files with Code Size differences (1 improved, 0 regressed), 0 unchanged.

Top method improvements (bytes):
          -1 (-0.07% of base) : 21732.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this

Top method improvements (percentages):
          -1 (-0.07% of base) : 21732.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this

1 total methods with Code Size differences (1 improved, 0 regressed), 0 unchanged.


libraries.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 45295203 (overridden on cmd)
Total bytes of diff: 45295192 (overridden on cmd)
Total bytes of delta: -11 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -11 : 174250.dasm (-0.72% of base)

1 total files with Code Size differences (1 improved, 0 regressed), 0 unchanged.

Top method improvements (bytes):
         -11 (-0.72% of base) : 174250.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this

Top method improvements (percentages):
         -11 (-0.72% of base) : 174250.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this

1 total methods with Code Size differences (1 improved, 0 regressed), 0 unchanged.


Full diffs pending SPMI in CI.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@SingleAccretion SingleAccretion marked this pull request as ready for review November 22, 2021 15:05
@SingleAccretion
Copy link
Contributor Author

@dotnet/jit-contrib, @tannergooding

Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
@jakobbotsch jakobbotsch merged commit 6069861 into dotnet:main Nov 29, 2021
@jakobbotsch
Copy link
Member

Thanks for the fix!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants