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

[ARM] Some bitwise logic instructions used by cmp-ne-zero are not combined into S-suffixed versions #57122

Closed
fzhinkin opened this issue Aug 12, 2022 · 4 comments
Assignees

Comments

@fzhinkin
Copy link
Contributor

fzhinkin commented Aug 12, 2022

Majority of bitwise logic instructions have 'S'-version which update status flags but only subset of these instructions is supported by the ARMBaseInstrInfo::optimizeCompareInstr peephole.

For example, ORR with shifted second operand is not supported by the peephole:
https://godbolt.org/z/536GvzfP4

bool cmp_ne_orr_rsr(uint32_t a, uint32_t b) {
    return (a | (b << 2)) != 0;
}

is currently compiled into:

cmp_ne_orr_rsr(unsigned int, unsigned int):
        orr     r0, r0, r1, lsl #2
        cmp     r0, #0
        movwne  r0, #1
        bx      lr

but a bit more optimal code will look like:

cmp_ne_orr_rsr(unsigned int, unsigned int):
        orrs    r0, r0, r1, lsl #2
        movwne  r0, #1
        bx      lr

The same is true for AND, ORR, EOR with shifted register operand and all versions of BIC.

The peephole also ignores shift instructions (except T2's LSR and LSL).

@fzhinkin fzhinkin self-assigned this Aug 12, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 12, 2022

@llvm/issue-subscribers-backend-arm

@fzhinkin
Copy link
Contributor Author

Proposed fix: https://reviews.llvm.org/D131786
If there are no objects I'll remove the "draft" status from it.

@davemgreen
Copy link
Collaborator

Sounds OK to me. It's probably worth making sure that it is well tested - perhaps by running a bootstrap.

fzhinkin added a commit that referenced this issue Oct 1, 2022
…eInstr

Combine cmp with zero and all versions of AND, ORR, EOR and BIC instructions into S-suffixed versions.

Related issue: #57122

Reviewed By: efriedma, samtebbs

Differential Revision: https://reviews.llvm.org/D131786
@fzhinkin
Copy link
Contributor Author

fzhinkin commented Oct 3, 2022

Fixed: 945a146

@fzhinkin fzhinkin closed this as completed Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants