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

[JIT] X86/X64 - Eliminate redundant 'cmp' instructions #82750

Merged
merged 33 commits into from Apr 5, 2023

Conversation

TIHan
Copy link
Member

@TIHan TIHan commented Feb 28, 2023

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 28, 2023
@ghost ghost assigned TIHan Feb 28, 2023
@ghost
Copy link

ghost commented Feb 28, 2023

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

Issue Details

Resolves #70003

Replaces #81143

Author: TIHan
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

emitPeepholeIterateLastInstrs([&](instrDesc* id) {
switch (id->idIns())
{
case INS_seta:
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really want to specify specific instructions - I'm just doing this for now.

Copy link
Member

Choose a reason for hiding this comment

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

Where did you get this list from?

Copy link
Member Author

@TIHan TIHan Feb 28, 2023

Choose a reason for hiding this comment

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

I'm just aware of the set and jmp instructions and we have the list defined in our instruction header file, so I just copied it from there.

@jakobbotsch
Copy link
Member

Does this fix #81220 and #78383?

@TIHan
Copy link
Member Author

TIHan commented Feb 28, 2023

@jakobbotsch it does look like they have redundant CMP, but I can't tell if this PR would eliminate them due to boundaries. It certainly wouldn't solve the issues if it did but it might help.

@TIHan
Copy link
Member Author

TIHan commented Apr 4, 2023

This PR is almost done, there appears to be a test failure specifically in linux 64 that I will need to reproduce locally to determine what is causing it.

@TIHan TIHan marked this pull request as ready for review April 5, 2023 02:27
@TIHan
Copy link
Member Author

TIHan commented Apr 5, 2023

@dotnet/jit-contrib @BruceForstall This is ready; pending CI.

src/coreclr/jit/emitxarch.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/emitxarch.cpp Outdated Show resolved Hide resolved
TIHan and others added 2 commits April 5, 2023 13:06
Co-authored-by: Bruce Forstall <brucefo@microsoft.com>
Co-authored-by: Bruce Forstall <brucefo@microsoft.com>
@TIHan TIHan merged commit 042a350 into dotnet:main Apr 5, 2023
3 checks passed
@dotnet dotnet locked as resolved and limited conversation to collaborators May 6, 2023
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.

Avoid duplicate cmp instruction, avoid boolean zero extension
4 participants