Better codegen for truncated bswap

[davidbolvansky]

Bugzilla Link	51568
Version	trunk
OS	Linux
CC	@RKSimon,@rotateright

Extended Description

unsigned int swap_ull(unsigned long long value)
{
    return ((value & 0x00000000000000ffull) << 56)
            | ((value & 0x000000000000ff00ull) << 40)
            | ((value & 0x0000000000ff0000ull) << 24)
            | ((value & 0x00000000ff000000ull) << 8)
            | ((value & 0x000000ff00000000ull) >> 8)
            | ((value & 0x0000ff0000000000ull) >> 24)
            | ((value & 0x00ff000000000000ull) >> 40)
            | ((value & 0xff00000000000000ull) >> 56);
}

unsigned int swap_ull_builtin(unsigned long long value)
{
    return __builtin_bswap64(value);
}

Trunk -O3:

swap_ull(unsigned long long):                           # @​swap_ull(unsigned long long)
        movabs  rax, 72057589742960640
        and     rax, rdi
        bswap   rax
        shr     rdi, 56
        or      eax, edi
        ret
swap_ull_builtin(unsigned long long):                  # @​swap_ull_builtin(unsigned long long)
        mov     rax, rdi
        bswap   rax
        ret

ICC -O3:

swap_ull(unsigned long long):
        bswap     rdi                                           #​10.51
        mov       eax, edi                                      #​10.51
        ret                                                     #​10.51
swap_ull_builtin(unsigned long long):
        bswap     rdi                                           #​15.12
        mov       eax, edi                                      #​15.12
        ret                                                     #​15.12

unsigned int swap_ull(unsigned long long value)
{
    return ((value & 0x00000000000000ffull) << 56)
            | ((value & 0x000000000000ff00ull) << 40)
            | ((value & 0x0000000000ff0000ull) << 24)
            | ((value & 0x00000000ff000000ull) << 8)
            | ((value & 0x000000ff00000000ull) >> 8)
            | ((value & 0x0000ff0000000000ull) >> 24)
            | ((value & 0x00ff000000000000ull) >> 40)
            | ((value & 0xff00000000000000ull) >> 56);
}

unsigned int swap_ull_builtin(unsigned long long value)
{
    return __builtin_bswap64(value);
}

define i32 @​src(i64 %0) {
%1:
  %2 = and i64 %0, 72057594037927935
  %3 = bswap i64 %2
  %4 = lshr i64 %0, 56
  %5 = or i64 %3, %4
  %6 = trunc i64 %5 to i32
  ret i32 %6
}
=>
define i32 @​tgt(i64 %0) {
%1:
  %2 = bswap i64 %0
  %3 = trunc i64 %2 to i32
  ret i32 %3
}
Transformation seems to be correct!

https://alive2.llvm.org/ce/z/loQjOQ

[rotateright]

Probably need some enhancement to known bits and/or demanded bits to see that we have masked off the high byte (72057594037927935 == 0x00ff...).

[RKSimon]

llvm::recognizeBSwapOrBitReverseIdiom could handle this, but it might bail out due to the trunc() user of the or()

[rotateright]

Do you know why we have the trunc user check? If I just comment it out, this bug is fixed with no regression test failures.

[RKSimon]

It looks like it first appeared in https://reviews.llvm.org/D20591

When I refactored recognizeBSwapOrBitReverseIdiom to better handle truncs in https://reviews.llvm.org/D88316 it looks like I kept it, but I'm not certain if it was necessary any more.

@rotateright Are you up for dealing with this?

[rotateright]

Sure - it should just be a matter of removing that check and cleaning up the code. I'll look over the tests to see if there was a test that became moot with D88316.

[rotateright]

I added a trunc test that I thought might work with:
3a23937
...but it doesn't match either way, so I'll plan to remove the trunc check as the fast fix for this bug.

[RKSimon]

Thanks @rotateright

[davidbolvansky]

Thanks for the fix