AMDGPU: Fix counting source modifiers as literal constants

[arsenm]

Summary

This fixes over estimating code size. This was broken by
79f52af4cd9a76485dd50bcdbb5d393eb7a70103.

[Joe_Nash]

On the other patch, you wrote "This change broke the handling of source modifiers. we're now treating any source modifier operand as a literal" You mean just the code size calculation is broken, right?

Can you please add test cases for GFX11, and something that emits fmamk/fmaak? Those were the motivating cases for the change.

[arsenm]

In D157103#4560958, @Joe_Nash wrote:

On the other patch, you wrote "This change broke the handling of source modifiers. we're now treating any source modifier operand as a literal" You mean just the code size calculation is broken, right?

Can you please add test cases for GFX11, and something that emits fmamk/fmaak? Those were the motivating cases for the change.

Turns out those are broken. I think the API makes less sense now. "isInlineConstant" only makes sense for a vsrc operand and not an arbitary one. A counts-as-literal-use-something-in-any-operand would make more sense (inverted)

[arsenm]

Explicitly handle all operand types and fix fmaak case

[Joe_Nash]

LGTM

[arsenm]

4b1702e87a2687569b197aea4721353f8b788182

[foad]

4b1702e87a2687569b197aea4721353f8b788182

This is causing:

$ cat r.ll
define amdgpu_ps <2 x half> @f(half %arg) {
bb:
  %i = fmul contract half %arg, 0xH3A66
  %i1 = fadd contract half %i, 0xH2E66
  %i2 = fmul contract half %arg, 0xH0000
  %i3 = fsub contract half 0xH3A66, %i2
  %i4 = insertelement <2 x half> poison, half %i1, i32 0
  %i5 = insertelement <2 x half> %i4, half %i3, i32 1
  ret <2 x half> %i5
}
$ llc -march=amdgcn -mcpu=gfx1010 -o /dev/null r.ll 
error: Illegal instruction detected: VOP2/VOP3 instruction uses more than one literal
renamable $vgpr1 = contract nofpexcept V_FMAAK_F16 14950, $vgpr0, 11878, implicit $mode, implicit $exec

Can you please fix or revert?

[Joe_Nash]

In D157103#4579608, @foad wrote:

4b1702e87a2687569b197aea4721353f8b788182

This is causing:

$ cat r.ll
define amdgpu_ps <2 x half> @f(half %arg) {
bb:
  %i = fmul contract half %arg, 0xH3A66
  %i1 = fadd contract half %i, 0xH2E66
  %i2 = fmul contract half %arg, 0xH0000
  %i3 = fsub contract half 0xH3A66, %i2
  %i4 = insertelement <2 x half> poison, half %i1, i32 0
  %i5 = insertelement <2 x half> %i4, half %i3, i32 1
  ret <2 x half> %i5
}
$ llc -march=amdgcn -mcpu=gfx1010 -o /dev/null r.ll 
error: Illegal instruction detected: VOP2/VOP3 instruction uses more than one literal
renamable $vgpr1 = contract nofpexcept V_FMAAK_F16 14950, $vgpr0, 11878, implicit $mode, implicit $exec

Can you please fix or revert?

I think https://reviews.llvm.org/D157624 will fix it.

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

bool SIInstrInfo::isInlineConstant(const APInt &Imm) const {
   default:
     llvm_unreachable("invalid bitwidth");
   }
 }
 
 bool SIInstrInfo::isInlineConstant(const MachineOperand &MO,
                                    uint8_t OperandType) const {
   assert(!MO.isReg() && "isInlineConstant called on register operand!");
-  if (!MO.isImm() ||
-      OperandType < AMDGPU::OPERAND_SRC_FIRST ||
-      OperandType > AMDGPU::OPERAND_SRC_LAST)
+  if (!MO.isImm())
     return false;
 
+  if (OperandType < AMDGPU::OPERAND_SRC_FIRST ||
+      OperandType > AMDGPU::OPERAND_SRC_LAST)
+    return true;
+
   // MachineOperand provides no way to tell the true operand size, since it only
   // records a 64-bit value. We need to know the size to determine if a 32-bit
   // floating point immediate bit pattern is legal for an integer immediate. It
   // would be for any 32-bit integer operand, but would not be for a 64-bit one.
 
   int64_t Imm = MO.getImm();
   switch (OperandType) {
   case AMDGPU::OPERAND_REG_IMM_INT32:

llvm/test/CodeGen/AMDGPU/code-size-estimate.ll

+; RUN: llc -march=amdgcn -mcpu=gfx900 -show-mc-encoding < %s | FileCheck -check-prefixes=CHECK,GFX9 %s
+; RUN: llc -march=amdgcn -mcpu=gfx1030 -show-mc-encoding < %s | FileCheck -check-prefixes=CHECK,GFX10 %s
+
+declare float @llvm.fabs.f32(float)
+
+define float @v_mul_f32_vop2(float %x, float %y) {
+; GFX9-LABEL: v_mul_f32_vop2:
+; GFX9:       ; %bb.0:
+; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX9-NEXT:    v_mul_f32_e32 v0, v0, v1 ; encoding: [0x00,0x03,0x00,0x0a]
+; GFX9-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x1d,0x80,0xbe]
+;
+; GFX10-LABEL: v_mul_f32_vop2:
+; GFX10:       ; %bb.0:
+; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX10-NEXT:    v_mul_f32_e32 v0, v0, v1 ; encoding: [0x00,0x03,0x00,0x10]
+; GFX10-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x20,0x80,0xbe]
+  %mul = fmul float %x, %y
+  ret float %mul
+}
+; CHECK: codeLenInByte = 12
+
+define float @v_mul_f32_vop2_inline_imm(float %x) {
+; GFX9-LABEL: v_mul_f32_vop2_inline_imm:
+; GFX9:       ; %bb.0:
+; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX9-NEXT:    v_mul_f32_e32 v0, 4.0, v0 ; encoding: [0xf6,0x00,0x00,0x0a]
+; GFX9-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x1d,0x80,0xbe]
+;
+; GFX10-LABEL: v_mul_f32_vop2_inline_imm:
+; GFX10:       ; %bb.0:
+; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX10-NEXT:    v_mul_f32_e32 v0, 4.0, v0 ; encoding: [0xf6,0x00,0x00,0x10]
+; GFX10-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x20,0x80,0xbe]
+  %mul = fmul float %x, 4.0
+  ret float %mul
+}
+; CHECK: codeLenInByte = 12
+
+define float @v_mul_f32_vop2_literal(float %x) {
+; GFX9-LABEL: v_mul_f32_vop2_literal:
+; GFX9:       ; %bb.0:
+; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX9-NEXT:    v_mul_f32_e32 v0, 0x42f60000, v0 ; encoding: [0xff,0x00,0x00,0x0a,0x00,0x00,0xf6,0x42]
+; GFX9-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x1d,0x80,0xbe]
+;
+; GFX10-LABEL: v_mul_f32_vop2_literal:
+; GFX10:       ; %bb.0:
+; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX10-NEXT:    v_mul_f32_e32 v0, 0x42f60000, v0 ; encoding: [0xff,0x00,0x00,0x10,0x00,0x00,0xf6,0x42]
+; GFX10-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x20,0x80,0xbe]
+  %mul = fmul float %x, 123.0
+  ret float %mul
+}
+; CHECK: codeLenInByte = 16
+
+define float @v_mul_f32_vop3_src_mods(float %x, float %y) {
+; GFX9-LABEL: v_mul_f32_vop3_src_mods:
+; GFX9:       ; %bb.0:
+; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX9-NEXT:    v_mul_f32_e64 v0, |v0|, v1 ; encoding: [0x00,0x01,0x05,0xd1,0x00,0x03,0x02,0x00]
+; GFX9-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x1d,0x80,0xbe]
+;
+; GFX10-LABEL: v_mul_f32_vop3_src_mods:
+; GFX10:       ; %bb.0:
+; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX10-NEXT:    v_mul_f32_e64 v0, |v0|, v1 ; encoding: [0x00,0x01,0x08,0xd5,0x00,0x03,0x02,0x00]
+; GFX10-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x20,0x80,0xbe]
+  %fabs.x = call float @llvm.fabs.f32(float %x)
+  %mul = fmul float %fabs.x, %y
+  ret float %mul
+}
+; CHECK: codeLenInByte = 16
+
+define float @v_mul_f32_vop3_src_mods_inline_imm(float %x, float %y) {
+; GFX9-LABEL: v_mul_f32_vop3_src_mods_inline_imm:
+; GFX9:       ; %bb.0:
+; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX9-NEXT:    v_mul_f32_e64 v0, |v0|, 4.0 ; encoding: [0x00,0x01,0x05,0xd1,0x00,0xed,0x01,0x00]
+; GFX9-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x1d,0x80,0xbe]
+;
+; GFX10-LABEL: v_mul_f32_vop3_src_mods_inline_imm:
+; GFX10:       ; %bb.0:
+; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX10-NEXT:    v_mul_f32_e64 v0, |v0|, 4.0 ; encoding: [0x00,0x01,0x08,0xd5,0x00,0xed,0x01,0x00]
+; GFX10-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x20,0x80,0xbe]
+  %fabs.x = call float @llvm.fabs.f32(float %x)
+  %mul = fmul float %fabs.x, 4.0
+  ret float %mul
+}
+
+; CHECK: codeLenInByte = 16
+
+define float @v_mul_f32_vop3_src_mods_literal(float %x, float %y) {
+; GFX9-LABEL: v_mul_f32_vop3_src_mods_literal:
+; GFX9:       ; %bb.0:
+; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX9-NEXT:    s_mov_b32 s4, 0x42f60000 ; encoding: [0xff,0x00,0x84,0xbe,0x00,0x00,0xf6,0x42]
+; GFX9-NEXT:    v_mul_f32_e64 v0, |v0|, s4 ; encoding: [0x00,0x01,0x05,0xd1,0x00,0x09,0x00,0x00]
+; GFX9-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x1d,0x80,0xbe]
+;
+; GFX10-LABEL: v_mul_f32_vop3_src_mods_literal:
+; GFX10:       ; %bb.0:
+; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX10-NEXT:    v_mul_f32_e64 v0, 0x42f60000, |v0| ; encoding: [0x00,0x02,0x08,0xd5,0xff,0x00,0x02,0x00,0x00,0x00,0xf6,0x42]
+; GFX10-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x20,0x80,0xbe]
+  %fabs.x = call float @llvm.fabs.f32(float %x)
+  %mul = fmul float %fabs.x, 123.0
+  ret float %mul
+}
+
+; GFX9: codeLenInByte = 24
+; GFX10: codeLenInByte = 20
+
+define float @v_mul_f32_vop2_frame_index(float %x) {
+; GFX9-LABEL: v_mul_f32_vop2_frame_index:
+; GFX9:       ; %bb.0:
+; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX9-NEXT:    v_lshrrev_b32_e64 v1, 6, s32 ; encoding: [0x01,0x00,0x10,0xd1,0x86,0x40,0x00,0x00]
+; GFX9-NEXT:    v_mul_f32_e32 v0, v1, v0 ; encoding: [0x01,0x01,0x00,0x0a]
+; GFX9-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x1d,0x80,0xbe]
+;
+; GFX10-LABEL: v_mul_f32_vop2_frame_index:
+; GFX10:       ; %bb.0:
+; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0) ; encoding: [0x00,0x00,0x8c,0xbf]
+; GFX10-NEXT:    v_lshrrev_b32_e64 v1, 5, s32 ; encoding: [0x01,0x00,0x16,0xd5,0x85,0x40,0x00,0x00]
+; GFX10-NEXT:    v_mul_f32_e32 v0, v1, v0 ; encoding: [0x01,0x01,0x00,0x10]
+; GFX10-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x20,0x80,0xbe]
+  %alloca = alloca i32, addrspace(5)
+  %ptrtoint = ptrtoint ptr addrspace(5) %alloca to i32
+  %cast = bitcast i32 %ptrtoint to float
+  %mul = fmul float %x, %cast
+  ret float %mul
+}
+
+; CHECK: codeLenInByte = 20

llvm/test/CodeGen/AMDGPU/idiv-licm.ll

 ; GFX11-NEXT:    s_load_b32 s4, s[0:1], 0x2c
 ; GFX11-NEXT:    s_load_b64 s[2:3], s[0:1], 0x24
 ; GFX11-NEXT:    v_dual_mov_b32 v1, 0 :: v_dual_mov_b32 v4, 0
 ; GFX11-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX11-NEXT:    s_and_b32 s1, s4, 0xffff
 ; GFX11-NEXT:    s_delay_alu instid0(SALU_CYCLE_1) | instskip(NEXT) | instid1(VALU_DEP_1)
 ; GFX11-NEXT:    v_cvt_f32_u32_e32 v2, s1
 ; GFX11-NEXT:    v_rcp_iflag_f32_e32 v3, v2
-; GFX11-NEXT:    s_set_inst_prefetch_distance 0x1
 ; GFX11-NEXT:    .p2align 6
 ; GFX11-NEXT:  .LBB5_1: ; %bb3
 ; GFX11-NEXT:    ; =>This Inner Loop Header: Depth=1
 ; GFX11-NEXT:    v_and_b32_e32 v0, 0xffff, v4
 ; GFX11-NEXT:    v_add_nc_u16 v4, v4, 1
 ; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_2) | instskip(SKIP_4) | instid1(VALU_DEP_1)
 ; GFX11-NEXT:    v_cvt_f32_u32_e32 v7, v0
 ; GFX11-NEXT:    v_lshlrev_b64 v[5:6], 1, v[0:1]

 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v7, vcc_lo, 0, v8, vcc_lo
 ; GFX11-NEXT:    v_cmp_eq_u16_e32 vcc_lo, 0x400, v4
 ; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_2) | instskip(NEXT) | instid1(VALU_DEP_1)
 ; GFX11-NEXT:    v_mul_lo_u32 v7, v7, s1
 ; GFX11-NEXT:    v_sub_nc_u32_e32 v0, v0, v7
 ; GFX11-NEXT:    global_store_b16 v[5:6], v0, off
 ; GFX11-NEXT:    s_cbranch_vccz .LBB5_1
 ; GFX11-NEXT:  ; %bb.2: ; %bb2
-; GFX11-NEXT:    s_set_inst_prefetch_distance 0x2
 ; GFX11-NEXT:    s_nop 0
 ; GFX11-NEXT:    s_sendmsg sendmsg(MSG_DEALLOC_VGPRS)
 ; GFX11-NEXT:    s_endpgm
 bb:
   br label %bb3
 
 bb2:                                              ; preds = %bb3
   ret void

 ; GFX11-NEXT:    v_mov_b32_e32 v2, 0
 ; GFX11-NEXT:    s_mov_b32 s3, 0
 ; GFX11-NEXT:    s_mov_b32 s5, 0
 ; GFX11-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX11-NEXT:    s_sext_i32_i16 s4, s2
 ; GFX11-NEXT:    s_delay_alu instid0(SALU_CYCLE_1) | instskip(NEXT) | instid1(VALU_DEP_1)
 ; GFX11-NEXT:    v_cvt_f32_i32_e32 v0, s4
 ; GFX11-NEXT:    v_rcp_iflag_f32_e32 v1, v0
-; GFX11-NEXT:    s_set_inst_prefetch_distance 0x1
 ; GFX11-NEXT:    .p2align 6
 ; GFX11-NEXT:  .LBB7_1: ; %bb3
 ; GFX11-NEXT:    ; =>This Inner Loop Header: Depth=1
 ; GFX11-NEXT:    s_sext_i32_i16 s8, s5
 ; GFX11-NEXT:    v_add_nc_u16 v3, s5, 1
 ; GFX11-NEXT:    v_cvt_f32_i32_e32 v4, s8
 ; GFX11-NEXT:    s_xor_b32 s2, s8, s4
 ; GFX11-NEXT:    s_delay_alu instid0(SALU_CYCLE_1) | instskip(NEXT) | instid1(VALU_DEP_2)

 ; GFX11-NEXT:    s_add_u32 s6, s0, s6
 ; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_2) | instskip(SKIP_1) | instid1(VALU_DEP_1)
 ; GFX11-NEXT:    v_mul_lo_u32 v3, v4, s4
 ; GFX11-NEXT:    s_addc_u32 s7, s1, s7
 ; GFX11-NEXT:    v_sub_nc_u32_e32 v3, s8, v3
 ; GFX11-NEXT:    global_store_b16 v2, v3, s[6:7]
 ; GFX11-NEXT:    s_cbranch_vccz .LBB7_1
 ; GFX11-NEXT:  ; %bb.2: ; %bb2
-; GFX11-NEXT:    s_set_inst_prefetch_distance 0x2
 ; GFX11-NEXT:    s_nop 0
 ; GFX11-NEXT:    s_sendmsg sendmsg(MSG_DEALLOC_VGPRS)
 ; GFX11-NEXT:    s_endpgm
 bb:
   br label %bb3
 
 bb2:                                              ; preds = %bb3
   ret void