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 Optimization: Eliminate duplicate bswap #66249

Closed
deeprobin opened this issue Mar 5, 2022 · 7 comments
Closed

JIT Optimization: Eliminate duplicate bswap #66249

deeprobin opened this issue Mar 5, 2022 · 7 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Comments

@deeprobin
Copy link
Contributor

An bswap swaps the endianess. If we emit an bswap twice we get back to the initial state, so this can be optimized (similar to what GCC and Clang do).

Reproduction: https://godbolt.org/z/KMEn8bYsc

Expected diff

    mov eax, edx
-   bswap eax
-   bswap eax
    ret
@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 untriaged New issue has not been triaged by the area owner labels Mar 5, 2022
@ghost
Copy link

ghost commented Mar 5, 2022

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

Issue Details

An bswap swaps the endianess. If we emit an bswap twice we get back to the initial state, so this can be optimized (similar to what GCC and Clang do).

Reproduction: https://godbolt.org/z/KMEn8bYsc

Expected diff

    mov eax, edx
-   bswap eax
-   bswap eax
    ret
Author: deeprobin
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@EgorBo
Copy link
Member

EgorBo commented Mar 5, 2022

As we discussed in the Discord - this optimization looks super niche
if it's two lines of code and it actually helps with some real-world code - we can accept it I'd guess, but overall I'd personally not bother.

@deeprobin
Copy link
Contributor Author

@EgorBo

I asked in the LLVM reviews what the real-world case is (See D6407: [InstCombine] Minor optimization for bswap with binary ops).
Maybe this will give us some insight.

@stephentoub
Copy link
Member

I asked in the LLVM reviews what the real-world case is

In the future, please only open issues when the real-world use case is known. Thanks.

@JulieLeeMSFT
Copy link
Member

As we discussed in the Discord - this optimization looks super niche

Closing now. Let's reopen this issue when there is a real world case.

@JulieLeeMSFT JulieLeeMSFT removed the untriaged New issue has not been triaged by the area owner label Mar 7, 2022
@deeprobin
Copy link
Contributor Author

deeprobin commented Mar 14, 2022

The main reason to merge this into LLVM was the ARGB - RGBA Pixel Code Swizzling (see LLVM Phabricator conversation)

I could imagine that this problem might occur with the API proposal #48615.

Tanner do you think the ARGB-RGB Pixel Swizzling could be a hot path for certain use cases (e.g. WinForms)? If so, the optimization would not be so bad in my opinion.

/cc @RKSimon
/cc @JeremyKuhne
/cc @tannergooding

@davidbolvansky
Copy link

The main reason to merge this into LLVM was the ARGB - RGBA Pixel Code Swizzling (see LLVM Phabricator conversation)

no, “this” has nothing common with that LLVM review. Two different transformations.

@dotnet dotnet locked as resolved and limited conversation to collaborators Apr 13, 2022
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

No branches or pull requests

5 participants