AMDGPU misses general case to match s_bitset instructions

[arsenm]

We currently do not try very hard to make use of s_bitset0_b32/s_bitset0_b64 or s_bitset1_b32/s_bitset1_b64

; RUN: llc -mtriple=amdgcn-amd-amdpal -mcpu=gfx900 -o - %s

; s_lshl_b32 s1, 1, s1
; s_or_b32 s0, s0, s1
; This can emit s_bitset1_b32
define amdgpu_ps i32 @s_bitset1_b32(i32 inreg %src0, i32 inreg %bit.index) {
  %set.bit.at.index = shl i32 1, %bit.index
  %or = or i32 %src0, %set.bit.at.index
  ret i32 %or
}

; s_lshl_b32 s1, 1, s1
; s_andn2_b32 s0, s0, s1
; This can emit s_bitset0_b32
define amdgpu_ps i32 @s_bitset0_b32(i32 inreg %src0, i32 inreg %bit.index) {
  %set.bit.at.index = shl i32 1, %bit.index
  %other.bits = xor i32 %set.bit.at.index, -1
  %and = and i32 %src0, %other.bits
  ret i32 %and
}

We only recognize and / or with a constant in SIShrinkInstructions. We should be able to recognize cases with a bit set to a variable index

[llvmbot]

@llvm/issue-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

We currently do not try very hard to make use of s_bitset0_b32/s_bitset0_b64 or s_bitset1_b32/s_bitset1_b64

; RUN: llc -mtriple=amdgcn-amd-amdpal -mcpu=gfx900 -o - %s

; s_lshl_b32 s1, 1, s1
; s_or_b32 s0, s0, s1
; This can emit s_bitset1_b32
define amdgpu_ps i32 @<!-- -->s_bitset1_b32(i32 inreg %src0, i32 inreg %bit.index) {
  %set.bit.at.index = shl i32 1, %bit.index
  %or = or i32 %src0, %set.bit.at.index
  ret i32 %or
}

; s_lshl_b32 s1, 1, s1
; s_andn2_b32 s0, s0, s1
; This can emit s_bitset0_b32
define amdgpu_ps i32 @<!-- -->s_bitset0_b32(i32 inreg %src0, i32 inreg %bit.index) {
  %set.bit.at.index = shl i32 1, %bit.index
  %other.bits = xor i32 %set.bit.at.index, -1
  %and = and i32 %src0, %other.bits
  ret i32 %and
}

We only recognize and / or with a constant in SIShrinkInstructions. We should be able to recognize cases with a bit set to a variable index

[BaoshanPang]

@arsenm Can I take this one?

[BaoshanPang]

It seems even the constant and/or operation is not recognised any more. The test case used to verify bitset has been updated as this:

define amdgpu_kernel void @s_clear_msb(ptr addrspace(1) %out, i32 %in) {
 ; SI-LABEL: s_clear_msb:
 ; SI:       ; %bb.0:
 ; SI-NEXT:    s_load_dword s6, s[4:5], 0xb
 ; SI-NEXT:    s_load_dwordx2 s[0:1], s[4:5], 0x9
 ; SI-NEXT:    s_mov_b32 s3, 0xf000
 ; SI-NEXT:    s_mov_b32 s2, -1
 ; SI-NEXT:    s_waitcnt lgkmcnt(0)
 ; SI-NEXT:    s_and_b32 s4, s6, 0x7fffffff
 ; SI-NEXT:    v_mov_b32_e32 v0, s4
 ; SI-NEXT:    buffer_store_dword v0, off, s[0:3], 0
 ; SI-NEXT:    s_endpgm
   %x = and i32 %in, 2147483647
   store i32 %x, ptr addrspace(1) %out
   ret void
 }

 define amdgpu_kernel void @s_set_msb(ptr addrspace(1) %out, i32 %in) {
 ; SI-LABEL: s_set_msb:
 ; SI:       ; %bb.0:
 ; SI-NEXT:    s_load_dword s6, s[4:5], 0xb
 ; SI-NEXT:    s_load_dwordx2 s[0:1], s[4:5], 0x9
 ; SI-NEXT:    s_mov_b32 s3, 0xf000
 ; SI-NEXT:    s_mov_b32 s2, -1
 ; SI-NEXT:    s_waitcnt lgkmcnt(0)
 ; SI-NEXT:    s_or_b32 s4, s6, 0x80000000
 ; SI-NEXT:    v_mov_b32_e32 v0, s4
 ; SI-NEXT:    buffer_store_dword v0, off, s[0:3], 0
 ; SI-NEXT:    s_endpgm
   %x = or i32 %in, 2147483648
   store i32 %x, ptr addrspace(1) %out
   ret void
 }

[arsenm]

The constant case uses are dependent on register allocation decisions so aren't reliable

[BaoshanPang]

The constant case uses are dependent on register allocation decisions so aren't reliable

When not using gloabl-isel, the ll is geneated as this:

# *** IR Dump After SI Shrink Instructions (si-shrink-instructions) ***:
# Machine code for function s_clear_msb: NoPHIs, TracksLiveness, NoVRegs, TiedOpsRewritten, TracksDebugUserValues
Function Live Ins: $sgpr4_sgpr5

bb.0 (%ir-block.0):
  liveins: $sgpr4_sgpr5
  renamable $sgpr6 = S_LOAD_DWORD_IMM renamable $sgpr4_sgpr5, 11, 0 :: (dereferenceable invariant load (s32) from %ir.in.kernarg.offset, addrspace 4)
  renamable $sgpr0_sgpr1 = S_LOAD_DWORDX2_IMM killed renamable $sgpr4_sgpr5, 9, 0 :: (dereferenceable invariant load (s64) from %ir.out.kernarg.offset, align 4, addrspace 4)
  renamable $sgpr3 = S_MOV_B32 61440
  renamable $sgpr2 = S_MOV_B32 -1
  renamable $sgpr4 = S_AND_B32 killed renamable $sgpr6, -16777217, implicit-def dead $scc
  $vgpr0 = V_MOV_B32_e32 killed $sgpr4, implicit $exec, implicit $exec
  BUFFER_STORE_DWORD_OFFSET killed renamable $vgpr0, killed renamable $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, implicit $exec :: (store (s32) into %ir.out.load, addrspace 1)
  S_ENDPGM 0

For the 'and' instruction, a different register is unessaryly used as dest register. Could we improve RA to make it reuse the same registger for such case?

[arsenm]

Could we improve RA to make it reuse the same registger for such case?

Could add allocation hints, but I don't think those help too much. If we wanted to be more aggressive with this, I think we can just always select these for these cases and turn them back to the normal bit operators in convertToThreeAddress

[BaoshanPang]

@arsenm Please help to reivew: #134155