[AMDGPU] Treat KIMM32 and KIMM16 operand types as noninlinable

[mbrkusanin]

Summary

While they are represent 32/16 bit immediate values they are already
included in encoding of the instructions that use them and are not true
literals. FMAMK and FMAAK instructions that use them are marked with fixed
size so getInstSizeInBytes will not increase the size for these operands.

We also add tests whose logic relies on KIMM16 and KIMM32 being considered
not inlinable.

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

7822–7823

this part is redundant?

[mbrkusanin]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

7822–7823

No, because size of fmamk and fmaak (instructions that use kimm32 and kimm16 operands) is already 8 so we do not need to add extra 4 bytes for these literals.

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

7822–7823

Then that should be covered by !isInlineConstant.

The API made more sense before when it was only checking for explicitly encoded src operands. This shouldn't require special casing here, something is wrong with the operand definitions if it does

[Joe_Nash]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

7822–7823

The KIMM are not inline constants and they are not true Literals either. There needs to be a special case somewhere. I think it is better to have a special case here than spread over everywhere we have to handle literals.

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

7822–7823

Just a drive-by comment: would it help to set the size of fmaak to 4, so you *do* have to add the size of the literal to get the true size?

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

7822–7823

Is isFixedSize not correctly set on these?

[mbrkusanin]

• Set FixedSize for FMAMK and FMAAK instructions

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

3909

Wouldn't this still be true?

[mbrkusanin]

• Update tablegen comments

[Joe_Nash]

LGTM, but please wait for @arsenm

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

3909

I think this part is unnecessary but the rest looks fine

[mbrkusanin]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

3909

Not for the issue of inst size which is fixed with FixedSize, but for a downstream test (see email).

[Joe_Nash]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

3909

This is a fix for the test noted on here https://reviews.llvm.org/D157103, and setting it to false is necessary for that.

[arsenm]

Should also add the other test for the failing case

[mbrkusanin]

• add test case which needs kimm16 and kimm32 to be not inlinable
• remove some now redundant checks

[mbrkusanin]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

5268–5270

This check here was why test mentioned in https://reviews.llvm.org/D157103#4579608 failed, but only for fmaak_f16 not f32 version. Previously we only checked for KIMM32, but with this patch it's redundant to check either.

[RKSimon]

@mbrkusanin This is causing failures on EXPENSIVE_CHECKS buildbots: https://lab.llvm.org/buildbot/#/builders/16/builds/52934 - please can you take a look?

[mbrkusanin]

In D157624#4583164, @RKSimon wrote:

@mbrkusanin This is causing failures on EXPENSIVE_CHECKS buildbots: https://lab.llvm.org/buildbot/#/builders/16/builds/52934 - please can you take a look?

Here is a fix: https://reviews.llvm.org/D157857

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

bool SIInstrInfo::isInlineConstant(const MachineOperand &MO,
   case AMDGPU::OPERAND_REG_IMM_V2FP16:
   case AMDGPU::OPERAND_REG_INLINE_C_V2FP16:
   case AMDGPU::OPERAND_REG_INLINE_AC_V2FP16: {
     uint32_t Trunc = static_cast<uint32_t>(Imm);
     return AMDGPU::isInlinableLiteralV216(Trunc, ST.hasInv2PiInlineImm());
   }
   case AMDGPU::OPERAND_KIMM32:
   case AMDGPU::OPERAND_KIMM16:
+    return false;
   case AMDGPU::OPERAND_INPUT_MODS:
   case MCOI::OPERAND_IMMEDIATE:
     // Always embedded in the instruction for free.
     return true;
   case MCOI::OPERAND_UNKNOWN:
   case MCOI::OPERAND_REGISTER:
   case MCOI::OPERAND_PCREL:
   case MCOI::OPERAND_GENERIC_0:
for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
         RegSubRegPair SGPR(Op.getReg(), Op.getSubReg());
         if (!SGPRsUsed.count(SGPR) &&
             // FIXME: This can access off the end of the operands() array.
             usesConstantBus(MRI, Op, InstDesc.operands().begin()[i])) {
           if (--ConstantBusLimit <= 0)
             return false;
           SGPRsUsed.insert(SGPR);
         }
-      } else if (InstDesc.operands()[i].OperandType == AMDGPU::OPERAND_KIMM32 ||
-                 (AMDGPU::isSISrcOperand(InstDesc, i) &&
-                  !isInlineConstant(Op, InstDesc.operands()[i]))) {
+      } else if (AMDGPU::isSISrcOperand(InstDesc, i) &&
+                 !isInlineConstant(Op, InstDesc.operands()[i])) {
         if (!LiteralLimit--)
           return false;
         if (--ConstantBusLimit <= 0)
           return false;
       }
     }
   }
 
   if (MO->isReg()) {
unsigned SIInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
   if (isVALU(MI) || isSALU(MI)) {
     if (isDPP(MI))
       return DescSize;
     bool HasLiteral = false;
     for (int I = 0, E = MI.getNumExplicitOperands(); I != E; ++I) {
       const MachineOperand &Op = MI.getOperand(I);
       const MCOperandInfo &OpInfo = Desc.operands()[I];
       if (!Op.isReg() && !isInlineConstant(Op, OpInfo)) {
         HasLiteral = true;
         break;
       }
     }
     return HasLiteral ? DescSize + 4 : DescSize;
   }
 
   // Check whether we have extra NSA words.
   if (isMIMG(MI)) {
     int VAddr0Idx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vaddr0);

llvm/lib/Target/AMDGPU/VOP2Instructions.td

 let SubtargetPredicate = isGFX11Plus in {
   let isCommutable = 1 in {
     defm V_AND_B16_t16 : VOP2Inst_e64 <"v_and_b16_t16", VOPProfile_True16<VOP_I16_I16_I16>, and>;
     defm V_OR_B16_t16  : VOP2Inst_e64 <"v_or_b16_t16", VOPProfile_True16<VOP_I16_I16_I16>, or>;
     defm V_XOR_B16_t16 : VOP2Inst_e64 <"v_xor_b16_t16", VOPProfile_True16<VOP_I16_I16_I16>, xor>;
   } // End isCommutable = 1
 } // End SubtargetPredicate = isGFX11Plus
 
-let FPDPRounding = 1, isReMaterializable = 1 in {
+let FPDPRounding = 1, isReMaterializable = 1, FixedSize = 1 in {
 let SubtargetPredicate = isGFX10Plus, OtherPredicates = [NotHasTrue16BitInsts] in {
 def V_FMAMK_F16 : VOP2_Pseudo <"v_fmamk_f16", VOP_MADMK_F16, [], "">;
 }
 let SubtargetPredicate = HasTrue16BitInsts in {
 def V_FMAMK_F16_t16 : VOP2_Pseudo <"v_fmamk_f16_t16", VOP_MADMK_F16_t16, [], "">;
 }
 
 let isCommutable = 1 in {
 let SubtargetPredicate = isGFX10Plus, OtherPredicates = [NotHasTrue16BitInsts] in {
 def V_FMAAK_F16 : VOP2_Pseudo <"v_fmaak_f16", VOP_MADAK_F16, [], "">;
 }
 let SubtargetPredicate = HasTrue16BitInsts in {
 def V_FMAAK_F16_t16 : VOP2_Pseudo <"v_fmaak_f16_t16", VOP_MADAK_F16_t16, [], "">;
 }
 } // End isCommutable = 1
-} // End FPDPRounding  = 1, isReMaterializable = 1
+} // End FPDPRounding  = 1, isReMaterializable = 1, FixedSize = 1
 
 let Constraints = "$vdst = $src2",
     DisableEncoding="$src2",
     isConvertibleToThreeAddress = 1,
     isCommutable = 1 in {
 let SubtargetPredicate = isGFX10Plus, OtherPredicates = [NotHasTrue16BitInsts] in {
 defm V_FMAC_F16 : VOP2Inst <"v_fmac_f16", VOP_MAC_F16>;
 }
let AddedComplexity = 30 in {
   def : GCNPat<
     (i32 (int_amdgcn_sdot8 i32:$src0, i32:$src1, i32:$src2, (i1 DSTCLAMP.NONE))),
     (i32 (V_DOT8C_I32_I4_e32 $src0, $src1, $src2))
   > {
     let SubtargetPredicate = HasDot3Insts;
   }
 } // End AddedComplexity = 30
 
-let SubtargetPredicate = HasFmaakFmamkF32Insts, isReMaterializable = 1 in {
+let SubtargetPredicate = HasFmaakFmamkF32Insts, isReMaterializable = 1, FixedSize = 1 in {
 def V_FMAMK_F32 : VOP2_Pseudo<"v_fmamk_f32", VOP_MADMK_F32, [], "">, VOPD_Component<0x2, "v_fmamk_f32">;
 
 let isCommutable = 1 in
 def V_FMAAK_F32 : VOP2_Pseudo<"v_fmaak_f32", VOP_MADAK_F32, [], "">, VOPD_Component<0x1, "v_fmaak_f32">;
-}
+} // End SubtargetPredicate = HasFmaakFmamkF32Insts, isReMaterializable = 1, FixedSize = 1
 
 let SubtargetPredicate = HasPkFmacF16Inst in {
 defm V_PK_FMAC_F16 : VOP2Inst<"v_pk_fmac_f16", VOP_V2F16_V2F16_V2F16>;
 } // End SubtargetPredicate = HasPkFmacF16Inst
 
 // Note: 16-bit instructions produce a 0 result in the high 16-bits
 // on GFX8 and GFX9 and preserve high 16 bits on GFX10+
 multiclass Arithmetic_i16_0Hi_Pats <SDPatternOperator op, Instruction inst> {

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

 ; GFX11-NEXT:    s_setpc_b64 s[30:31] ; encoding: [0x1e,0x48,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
+; GFX11: 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]

llvm/test/CodeGen/AMDGPU/si-fold-kimm.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1010 -run-pass=si-fold-operands -verify-machineinstrs -o - %s | FileCheck --check-prefix=GCN %s
+
+# Do not fold noninlinable constants in instructions like fmaak and fmamk next
+# to kimm16 and kimm32 operands
+
+---
+name: fmaak_f16
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0
+
+    ; GCN-LABEL: name: fmaak_f16
+    ; GCN: liveins: $vgpr0
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+    ; GCN-NEXT: [[V_MOV_B32_e32_:%[0-9]+]]:vgpr_32 = V_MOV_B32_e32 14950, implicit $exec
+    ; GCN-NEXT: [[V_FMAAK_F16_:%[0-9]+]]:vgpr_32 = contract nofpexcept V_FMAAK_F16 [[COPY]], [[V_MOV_B32_e32_]], 11878, implicit $mode, implicit $exec
+    ; GCN-NEXT: $vgpr0 = COPY [[V_FMAAK_F16_]]
+    %0:vgpr_32 = COPY $vgpr0
+    %1:vgpr_32 = V_MOV_B32_e32 14950, implicit $exec
+    %2:vgpr_32 = contract nofpexcept V_FMAAK_F16 %0, %1, 11878, implicit $mode, implicit $exec
+    $vgpr0 = COPY %2
+...
+
+---
+name: fmamk_f16
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0
+
+    ; GCN-LABEL: name: fmamk_f16
+    ; GCN: liveins: $vgpr0
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+    ; GCN-NEXT: [[V_MOV_B32_e32_:%[0-9]+]]:vgpr_32 = V_MOV_B32_e32 14950, implicit $exec
+    ; GCN-NEXT: [[V_FMAMK_F16_:%[0-9]+]]:vgpr_32 = contract nofpexcept V_FMAMK_F16 [[COPY]], 11878, [[V_MOV_B32_e32_]], implicit $mode, implicit $exec
+    ; GCN-NEXT: $vgpr0 = COPY [[V_FMAMK_F16_]]
+    %0:vgpr_32 = COPY $vgpr0
+    %1:vgpr_32 = V_MOV_B32_e32 14950, implicit $exec
+    %2:vgpr_32 = contract nofpexcept V_FMAMK_F16 %0, 11878, %1, implicit $mode, implicit $exec
+    $vgpr0 = COPY %2
+...
+
+---
+name: fmaak_f32
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0
+
+    ; GCN-LABEL: name: fmaak_f32
+    ; GCN: liveins: $vgpr0
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+    ; GCN-NEXT: [[V_MOV_B32_e32_:%[0-9]+]]:vgpr_32 = V_MOV_B32_e32 979789416, implicit $exec
+    ; GCN-NEXT: [[V_FMAAK_F32_:%[0-9]+]]:vgpr_32 = contract nofpexcept V_FMAAK_F32 [[COPY]], [[V_MOV_B32_e32_]], 778462824, implicit $mode, implicit $exec
+    ; GCN-NEXT: $vgpr0 = COPY [[V_FMAAK_F32_]]
+    %0:vgpr_32 = COPY $vgpr0
+    %1:vgpr_32 = V_MOV_B32_e32 979789416, implicit $exec
+    %2:vgpr_32 = contract nofpexcept V_FMAAK_F32 %0, %1, 778462824, implicit $mode, implicit $exec
+    $vgpr0 = COPY %2
+...
+
+---
+name: fmamk_f32
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0
+
+    ; GCN-LABEL: name: fmamk_f32
+    ; GCN: liveins: $vgpr0
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+    ; GCN-NEXT: [[V_MOV_B32_e32_:%[0-9]+]]:vgpr_32 = V_MOV_B32_e32 979789416, implicit $exec
+    ; GCN-NEXT: [[V_FMAMK_F32_:%[0-9]+]]:vgpr_32 = contract nofpexcept V_FMAMK_F32 [[COPY]], 778462824, [[V_MOV_B32_e32_]], implicit $mode, implicit $exec
+    ; GCN-NEXT: $vgpr0 = COPY [[V_FMAMK_F32_]]
+    %0:vgpr_32 = COPY $vgpr0
+    %1:vgpr_32 = V_MOV_B32_e32 979789416, implicit $exec
+    %2:vgpr_32 = contract nofpexcept V_FMAMK_F32 %0, 778462824, %1, implicit $mode, implicit $exec
+    $vgpr0 = COPY %2
+...