llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.83k stars 11.91k forks source link

[AArch64] Fold `and` and `cmp` into `tst` #102703

Closed Kmeakin closed 1 month ago

Kmeakin commented 2 months ago

https://godbolt.org/z/Mh7q8TY99

InstCombine is able to fold (x & 0xFF) < C) into (x & -C) == 0

eg:

bool ult32_u32(u32 x) { return (x & 0xFF) < 32; }

produces

ult32_u32:
        tst     w0, #0xe0
        cset    w0, eq
        ret

But the same transform is not done in later stages, so if the and is introduced due to eg passing a u8 in a u32 register, the fold is not performed:

bool ult32_u8(u8 x) { return x < 32; }
ult32_u8:
        and     w8, w0, #0xff
        cmp     w8, #32
        cset    w0, lo
        ret
llvmbot commented 2 months ago

@llvm/issue-subscribers-backend-aarch64

Author: Karl Meakin (Kmeakin)

https://godbolt.org/z/Mh7q8TY99 InstCombine is able to fold `(x & 0xFF) < C)` into `(x & -C) != 0` eg: ```c bool ult32_u32(u32 x) { return (x & 0xFF) < 32; } ``` produces ```asm ult32_u32: tst w0, #0xe0 cset w0, eq ret ``` But the same transform is not done in later stages, so if the and is introduced due to eg passing a u8 in a u32 register, the fold is not performed: ```c bool ult32_u8(u8 x) { return x < 32; } ``` ```asm ult32_u8: and w8, w0, #0xff cmp w8, #32 cset w0, lo ret ```
pvimal816 commented 2 months ago

Hi @Kmeakin Orthogonal to the root issue, shouldn't (x & -C) != 0 be (x & -C) == 0 instead?

Because, as per my understanding, tst a, b will set Z flag only if a&b is zero, and unless Z flag is set cset a, eq will not set a.

jf-botto commented 2 months ago

I'd like to work on this issue, if possible? Please assign me?

Kmeakin commented 1 month ago

Hi @Kmeakin Orthogonal to the root issue, shouldn't (x & -C) != 0 be (x & -C) == 0 instead?

Because, as per my understanding, tst a, b will set Z flag only if a&b is zero, and unless Z flag is set cset a, eq will not set a.

Yes, that was a typo, thanks