[AMDGPU][MC] Corrected handling of relocatable expressions

[dp]

Summary

Relocatable expressions may be used as operands; however they are not accounted for when computing the total number of literals used by an instruction.

See bug 43359: https://bugs.llvm.org//show_bug.cgi?id=43359

[rampitec]

LGTM

Does it need the same fix in the codegen?

[dp]

I believe codegen does not need any changes. Symbols like MO_ExternalSymbol, etc are handled by SIInstrInfo::usesConstantBus and SIInstrInfo::isLiteralConstantLike.

llvm/trunk/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

bool AMDGPUAsmParser::validateSOPLiteral(const MCInst &Inst) const {
   if (!(Desc.TSFlags & (SIInstrFlags::SOP2 | SIInstrFlags::SOPC)))
     return true;
 
   const int Src0Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src0);
   const int Src1Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src1);
 
   const int OpIndices[] = { Src0Idx, Src1Idx };
 
+  unsigned NumExprs = 0;
   unsigned NumLiterals = 0;
   uint32_t LiteralValue;
 
   for (int OpIdx : OpIndices) {
     if (OpIdx == -1) break;
 
     const MCOperand &MO = Inst.getOperand(OpIdx);
-    if (MO.isImm() &&
-        // Exclude special imm operands (like that used by s_set_gpr_idx_on)
-        AMDGPU::isSISrcOperand(Desc, OpIdx) &&
-        !isInlineConstant(Inst, OpIdx)) {
+    if (AMDGPU::isSISrcOperand(Desc, OpIdx)) {
+      if (MO.isImm() && !isInlineConstant(Inst, OpIdx)) {
         uint32_t Value = static_cast<uint32_t>(MO.getImm());
         if (NumLiterals == 0 || LiteralValue != Value) {
           LiteralValue = Value;
           ++NumLiterals;
         }
+      } else if (MO.isExpr()) {
+        ++NumExprs;
+      }
     }
   }
 
-  return NumLiterals <= 1;
+  return NumLiterals + NumExprs <= 1;
 }
 
 bool AMDGPUAsmParser::validateOpSel(const MCInst &Inst) {
   const unsigned Opc = Inst.getOpcode();
   if (Opc == AMDGPU::V_PERMLANE16_B32_gfx10 ||
       Opc == AMDGPU::V_PERMLANEX16_B32_gfx10) {
     int OpSelIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::op_sel);
     unsigned OpSel = Inst.getOperand(OpSelIdx).getImm();
if (!(Desc.TSFlags & (SIInstrFlags::VOP3 | SIInstrFlags::VOP3P)))
     return true;
 
   const int Src0Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src0);
   const int Src1Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src1);
   const int Src2Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src2);
 
   const int OpIndices[] = { Src0Idx, Src1Idx, Src2Idx };
 
+  unsigned NumExprs = 0;
   unsigned NumLiterals = 0;
   uint32_t LiteralValue;
 
   for (int OpIdx : OpIndices) {
     if (OpIdx == -1) break;
 
     const MCOperand &MO = Inst.getOperand(OpIdx);
-    if (!MO.isImm() || !AMDGPU::isSISrcOperand(Desc, OpIdx))
+    if (!MO.isImm() && !MO.isExpr())
+      continue;
+    if (!AMDGPU::isSISrcOperand(Desc, OpIdx))
       continue;
 
     if (OpIdx == Src2Idx && (Desc.TSFlags & SIInstrFlags::IsMAI) &&
         getFeatureBits()[AMDGPU::FeatureMFMAInlineLiteralBug])
       return false;
 
-    if (!isInlineConstant(Inst, OpIdx)) {
+    if (MO.isImm() && !isInlineConstant(Inst, OpIdx)) {
       uint32_t Value = static_cast<uint32_t>(MO.getImm());
       if (NumLiterals == 0 || LiteralValue != Value) {
         LiteralValue = Value;
         ++NumLiterals;
       }
+    } else if (MO.isExpr()) {
+      ++NumExprs;
     }
   }
+  NumLiterals += NumExprs;
 
   return !NumLiterals ||
          (NumLiterals == 1 && getFeatureBits()[AMDGPU::FeatureVOP3Literal]);
 }
 
 bool AMDGPUAsmParser::validateInstruction(const MCInst &Inst,
                                           const SMLoc &IDLoc,
                                           const OperandVector &Operands) {

llvm/trunk/test/MC/AMDGPU/expressions-gfx10.s

-// RUN: llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=-WavefrontSize32,+WavefrontSize64 -show-encoding %s | FileCheck %s --check-prefix=GFX10
+// RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=-WavefrontSize32,+WavefrontSize64 -show-encoding %s | FileCheck %s --check-prefix=GFX10
+// RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=-WavefrontSize32,+WavefrontSize64 %s 2>&1 | FileCheck -check-prefix=NOGFX10 %s
 
 i1=1
 
 //===----------------------------------------------------------------------===//
 // Constant expressions may be used where literals are accepted.
 //===----------------------------------------------------------------------===//
 
 v_bfe_u32 v0, i1+100, v1, v2

 
 v_cmp_f_i32 s[10:11], u+1, v2
 // GFX10: v_cmp_f_i32_e64 s[10:11], u+1, v2 ; encoding: [0x0a,0x00,0x80,0xd4,0xff,0x04,0x02,0x00,A,A,A,A]
 // GFX10-NEXT:                              ;   fixup A - offset: 8, value: u+1, kind: FK_PCRel_4
 
 v_lshlrev_b64 v[5:6], u-1, v[2:3]
 // GFX10: v_lshlrev_b64 v[5:6], u-1, v[2:3] ; encoding: [0x05,0x00,0xff,0xd6,0xff,0x04,0x02,0x00,A,A,A,A]
 // GFX10-NEXT:                              ;   fixup A - offset: 8, value: u-1, kind: FK_Data_4
+
+//===----------------------------------------------------------------------===//
+// Instructions can use only one literal.
+// Relocatable expressions are counted as literals.
+//===----------------------------------------------------------------------===//
+
+s_sub_u32 s0, 123, u
+// NOGFX10: error: only one literal operand is allowed
+
+s_sub_u32 s0, u, u
+// NOGFX10: error: only one literal operand is allowed
+
+s_sub_u32 s0, u, u1
+// NOGFX10: error: only one literal operand is allowed
+
+v_bfe_u32 v0, v2, 123, u
+// NOGFX10: error: invalid literal operand
+
+v_bfe_u32 v0, v2, u, u
+// NOGFX10: error: invalid literal operand
+
+v_bfe_u32 v0, v2, u, u1
+// NOGFX10: error: invalid literal operand

llvm/trunk/test/MC/AMDGPU/expressions-gfx9.s

 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx900 -show-encoding %s | FileCheck %s --check-prefix=GFX9
 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx900 -show-encoding %s 2>&1 | FileCheck %s --check-prefix=NOGFX9
 
 //===----------------------------------------------------------------------===//
 // Relocatable expressions cannot be used with SDWA modifiers.
 //===----------------------------------------------------------------------===//
 
 v_mov_b32_sdwa v1, sext(u)
 // NOGFX9: error: expected an absolute expression
 
 //===----------------------------------------------------------------------===//
+// Instructions can use only one literal.
+// Relocatable expressions are counted as literals.
+//===----------------------------------------------------------------------===//
+
+s_sub_u32 s0, 123, u
+// NOGFX9: error: only one literal operand is allowed
+
+s_sub_u32 s0, u, u
+// NOGFX9: error: only one literal operand is allowed
+
+s_sub_u32 s0, u, u1
+// NOGFX9: error: only one literal operand is allowed
+
+v_bfe_u32 v0, v2, v3, u
+// NOGFX9: error: invalid literal operand
+
+//===----------------------------------------------------------------------===//
 // Constant expressions may be used with 'sext' modifier
 //===----------------------------------------------------------------------===//
 
 i1=1
 
 v_mov_b32_sdwa v1, sext(i1-2)
 // GFX9: v_mov_b32_sdwa v1, sext(-1) dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:DWORD ; encoding: [0xf9,0x02,0x02,0x7e,0xc1,0x16,0x8e,0x00]