[AMDGPU] Fix for folding v2.16 literals.

[dfukalov]

Summary

It was found some packed immediate operands (e.g. <half 1.0, half 2.0>) are
incorrectly processed so one of two packed values were lost.

Introduced new function to check immediate 32-bit operand can be folded.
Converted condition about current op_sel flags value to fall-through.

Fixes: SWDEV-247595

[rampitec]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

195–196

Seems like you still need to check if it is an inline literal, not just foldable. If it is foldable but not inline you can do it only with VOP3 literals available, in which case you do not need to play these games with op_sel at all.

Then I also do not see where the check for vop3 literals is done here. Imagine you would get one and produce op_sel version of something foldable but not inline on GFX9.

[dfukalov]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

195–196

This case is about op_sel folding optimisation only (condition in line 193 checks the instruction has SIInstrFlags::IsPacked flag so it is VOP3). If a literal is not foldable with op_sel, it is processed by code below.

[rampitec]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

195–196

It is not sufficient to check for packed. Assume that literal is 0x200. It is foldable according to your check because high half is zero. Then the code as written will fold it. Same for 0x02000000 because low half is zero. But this is only legal to do on GFX10 which hasVOP3Literal(). As far as I understand it will also fold it on GFX9 where it is not legal. I do not see this legality check here.

[dfukalov]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

195–196

Actually we are here in function that updates operands, since they are in FoldList. Your examples are not placed in candidates list for gfx900. They fails in last tryAddToFoldList() from foldOperand(). It checks isOperandLegal() and fails to use other options.

There are a lot of tests (with different resulting ISA for GFX9 and GFX10) for symmetrical literals (hi16bit == lo16bit) in the test file named "immv216.ll". These literals pass the checks in discussed lines too.

Additionally, "pk_max_f16_literal.ll" test contains check that <half 0xH0000, half 0xH41C8> and <half 0xH41C8, half 0xH0> literals folded for GFX10, but not for GFX9.

Should I duplicate second tests into immv216.ll with, probably, other packed instructions?

[rampitec]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

195–196

It seems to be working, but I want to understand how. Could you please find out at which point this is rejected on GFX9?

[dfukalov]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

195–196

For the case you described, on GFX9:
Firstly foldOperand() fall through its body just to last tryAddToFoldList() at line 914. Then in line 335 we go into if(!TII->isOperandLegal()) and try to check other options (MAC operations, SETREG instruction). They are all not possible in our case so then we try to commute operands and check this variant (lines 373-424). This approach doesn't make folding possible too.

So tryAddToFoldList() doesn't add the instruction to FoldList and returns with false at line 427. And then we do not reach tryAddToFoldList() call at line 1286 within foreach(FoldList).

For the difference, on GFX10 tryAddToFoldList() doesn't got into if(!TII->isOperandLegal()) at line 335 but just arrives to appendFoldCandidate() at line 457 and add the instruction to FoldList.

[rampitec]

OK, thanks!

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

static bool updateOperand(FoldCandidate &Fold,
                           const GCNSubtarget &ST) {
   MachineInstr *MI = Fold.UseMI;
   MachineOperand &Old = MI->getOperand(Fold.UseOpNo);
   assert(Old.isReg());
 
   if (Fold.isImm()) {
     if (MI->getDesc().TSFlags & SIInstrFlags::IsPacked &&
         !(MI->getDesc().TSFlags & SIInstrFlags::IsMAI) &&
-        AMDGPU::isInlinableLiteralV216(static_cast<uint16_t>(Fold.ImmToFold),
-                                       ST.hasInv2PiInlineImm())) {
+        AMDGPU::isFoldableLiteralV216(Fold.ImmToFold,
       // Set op_sel/op_sel_hi on this operand or bail out if op_sel is
       // already set.
       unsigned Opcode = MI->getOpcode();
       int OpNo = MI->getOperandNo(&Old);
       int ModIdx = -1;
       if (OpNo == AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src0))
         ModIdx = AMDGPU::OpName::src0_modifiers;
       else if (OpNo == AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src1))
         ModIdx = AMDGPU::OpName::src1_modifiers;
       else if (OpNo == AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src2))
         ModIdx = AMDGPU::OpName::src2_modifiers;
       assert(ModIdx != -1);
       ModIdx = AMDGPU::getNamedOperandIdx(Opcode, ModIdx);
       MachineOperand &Mod = MI->getOperand(ModIdx);
       unsigned Val = Mod.getImm();
-      if ((Val & SISrcMods::OP_SEL_0) || !(Val & SISrcMods::OP_SEL_1))
-        return false;
+      if (!(Val & SISrcMods::OP_SEL_0) && (Val & SISrcMods::OP_SEL_1)) {
         // Only apply the following transformation if that operand requries
         // a packed immediate.
         switch (TII.get(Opcode).OpInfo[OpNo].OperandType) {
         case AMDGPU::OPERAND_REG_IMM_V2FP16:
         case AMDGPU::OPERAND_REG_IMM_V2INT16:
         case AMDGPU::OPERAND_REG_INLINE_C_V2FP16:
         case AMDGPU::OPERAND_REG_INLINE_C_V2INT16:
           // If upper part is all zero we do not need op_sel_hi.
           if (!isUInt<16>(Fold.ImmToFold)) {
             if (!(Fold.ImmToFold & 0xffff)) {
               Mod.setImm(Mod.getImm() | SISrcMods::OP_SEL_0);
               Mod.setImm(Mod.getImm() & ~SISrcMods::OP_SEL_1);
               Old.ChangeToImmediate((Fold.ImmToFold >> 16) & 0xffff);
               return true;
             }
             Mod.setImm(Mod.getImm() & ~SISrcMods::OP_SEL_1);
             Old.ChangeToImmediate(Fold.ImmToFold & 0xffff);
             return true;
           }
           break;
         default:
           break;
         }
       }
     }
+  }
 
   if ((Fold.isImm() || Fold.isFI() || Fold.isGlobal()) && Fold.needsShrink()) {
     MachineBasicBlock *MBB = MI->getParent();
     auto Liveness = MBB->computeRegisterLiveness(&TRI, AMDGPU::VCC, MI, 16);
     if (Liveness != MachineBasicBlock::LQR_Dead) {
       LLVM_DEBUG(dbgs() << "Not shrinking " << MI << " due to vcc liveness\n");
       return false;
     }

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h

 bool isInlinableLiteral16(int16_t Literal, bool HasInv2Pi);
 
 LLVM_READNONE
 bool isInlinableLiteralV216(int32_t Literal, bool HasInv2Pi);
 
 LLVM_READNONE
 bool isInlinableIntLiteralV216(int32_t Literal);
 
+LLVM_READNONE
+bool isFoldableLiteralV216(int32_t Literal, bool HasInv2Pi);
+
 bool isArgPassedInSGPR(const Argument *Arg);
 
 LLVM_READONLY
 bool isLegalSMRDEncodedUnsignedOffset(const MCSubtargetInfo &ST,
                                       int64_t EncodedOffset);
 
 LLVM_READONLY
 bool isLegalSMRDEncodedSignedOffset(const MCSubtargetInfo &ST,

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

if (isInt<16>(Literal) || isUInt<16>(Literal))
     return isInlinableIntLiteral(Lo16);
 
   int16_t Hi16 = static_cast<int16_t>(Literal >> 16);
   if (!(Literal & 0xffff))
     return isInlinableIntLiteral(Hi16);
   return Lo16 == Hi16 && isInlinableIntLiteral(Lo16);
 }
 
+bool isFoldableLiteralV216(int32_t Literal, bool HasInv2Pi) {
+  assert(HasInv2Pi);
+
+  int16_t Lo16 = static_cast<int16_t>(Literal);
+  if (isInt<16>(Literal) || isUInt<16>(Literal))
+    return true;
+
+  int16_t Hi16 = static_cast<int16_t>(Literal >> 16);
+  if (!(Literal & 0xffff))
+    return true;
+  return Lo16 == Hi16;
+}
+
 bool isArgPassedInSGPR(const Argument *A) {
   const Function *F = A->getParent();
 
   // Arguments to compute shaders are never a source of divergence.
   CallingConv::ID CC = F->getCallingConv();
   switch (CC) {
   case CallingConv::AMDGPU_KERNEL:
   case CallingConv::SPIR_KERNEL:

llvm/test/CodeGen/AMDGPU/shrink-add-sub-constant.ll

 ; GFX10-LABEL: v_test_v2i16_x_sub_7_64:
 ; GFX10:       ; %bb.0:
 ; GFX10-NEXT:    s_load_dwordx4 s[0:3], s[0:1], 0x24
 ; GFX10-NEXT:    v_lshlrev_b32_e32 v0, 2, v0
 ; GFX10-NEXT:    ; implicit-def: $vcc_hi
 ; GFX10-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-NEXT:    global_load_dword v1, v0, s[2:3]
 ; GFX10-NEXT:    s_waitcnt vmcnt(0)
-; GFX10-NEXT:    v_pk_sub_i16 v1, v1, 7 op_sel_hi:[1,0]
+; GFX10-NEXT:    v_pk_sub_i16 v1, v1, 0x400007
 ; GFX10-NEXT:    global_store_dword v0, v1, s[0:1]
 ; GFX10-NEXT:    s_endpgm
   %tid = call i32 @llvm.amdgcn.workitem.id.x()
   %tid.ext = sext i32 %tid to i64
   %gep = getelementptr inbounds <2 x i16>, <2 x i16> addrspace(1)* %in, i64 %tid.ext
   %gep.out = getelementptr inbounds <2 x i16>, <2 x i16> addrspace(1)* %out, i64 %tid.ext
   %x = load <2 x i16>, <2 x i16> addrspace(1)* %gep
   %result = sub <2 x i16> %x, <i16 7, i16 64>

 ; GFX10-LABEL: v_test_v2i16_x_sub_64_123:
 ; GFX10:       ; %bb.0:
 ; GFX10-NEXT:    s_load_dwordx4 s[0:3], s[0:1], 0x24
 ; GFX10-NEXT:    v_lshlrev_b32_e32 v0, 2, v0
 ; GFX10-NEXT:    ; implicit-def: $vcc_hi
 ; GFX10-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-NEXT:    global_load_dword v1, v0, s[2:3]
 ; GFX10-NEXT:    s_waitcnt vmcnt(0)
-; GFX10-NEXT:    v_pk_sub_i16 v1, v1, 64 op_sel_hi:[1,0]
+; GFX10-NEXT:    v_pk_sub_i16 v1, v1, 0x7b0040
 ; GFX10-NEXT:    global_store_dword v0, v1, s[0:1]
 ; GFX10-NEXT:    s_endpgm
   %tid = call i32 @llvm.amdgcn.workitem.id.x()
   %tid.ext = sext i32 %tid to i64
   %gep = getelementptr inbounds <2 x i16>, <2 x i16> addrspace(1)* %in, i64 %tid.ext
   %gep.out = getelementptr inbounds <2 x i16>, <2 x i16> addrspace(1)* %out, i64 %tid.ext
   %x = load <2 x i16>, <2 x i16> addrspace(1)* %gep
   %result = sub <2 x i16> %x, <i16 64, i16 123>