Skip to content

Conversation

@sandreenko
Copy link
Contributor

@sandreenko sandreenko commented Jul 22, 2021

Fixes #54841, closes DrewScoggins/performance-2#7131.

coreclr_tests.pmi.Linux.arm64.checked.mch
Total bytes of delta: -36
6 total files with Code Size differences (6 improved, 0 regressed)

libraries_tests.pmi.Linux.arm64.checked.mch
Total bytes of delta: -16
2 total methods with Code Size differences (2 improved, 0 regressed)

coreclr_tests.pmi.windows.arm64.checked.mch
Total bytes of delta: -60
12 total files with Code Size differences (12 improved, 0 regressed)

libraries_tests.pmi.windows.arm64.checked.mch
Total bytes of delta: -16 
2 total files with Code Size differences (2 improved, 0 regressed)

coreclr_tests.pmi.Linux.arm.checked.mch
Total bytes of delta: -12
3 total methods with Code Size differences (3 improved, 0 regressed), 0 unchanged

@sandreenko sandreenko added arch-arm32 arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Jul 22, 2021
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it looks like it is another function that we should use always instead of ins_Store but currently we don't.
I have updated arm64 to match arm32 and left xarch as it was for now.

@sandreenko
Copy link
Contributor Author

/azp run runtime-coreclr jitstress, runtime-coreclr outerloop, runtime-coreclr jitstressregs

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@sandreenko
Copy link
Contributor Author

PTAL @kunalspathak @dotnet/jit-contrib ,
the failures are not related (#56174)

Copy link
Contributor

Choose a reason for hiding this comment

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

This assert will never fire (will always evaluate to "true")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed!

Copy link
Member

Choose a reason for hiding this comment

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

We have a few hits of this it seems:

src\coreclr\jit\valuenum.cpp
8811:                        noway_assert("LOCKADD should not appear before lowering");

src\coreclr\inc\clr_std\vector
296:                    assert("push_back: overflow");
314:                        assert("resize: overflow");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noway_assert

yes, I was fixing it in #44132 but we don't have a check for it and add more from time to time.
I guess we can define a deleted "assert(const char*)" and it will give us a compilation failure, won't it?

@kunalspathak
Copy link
Contributor

You might want to rebase once #56179 is merged.

Copy link
Contributor

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM with minor comments.

@sandreenko sandreenko merged commit 361184f into dotnet:main Jul 23, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-arm32 arch-arm64 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.

[Perf] Changes at 6/28/2021 10:33:32 PM support contained bitcast under STORE_LCL_VAR/FLD for armarch.

4 participants