[llvm] 5c7ee89 - AMDGPU: Stop validating earlyclobber operands in assembler

Jay Foad via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 21 13:47:12 PDT 2022


Author: Jay Foad
Date: 2022-09-21T21:46:59+01:00
New Revision: 5c7ee894f8039902b9cd6da3db39b43819ca3996

URL: https://github.com/llvm/llvm-project/commit/5c7ee894f8039902b9cd6da3db39b43819ca3996
DIFF: https://github.com/llvm/llvm-project/commit/5c7ee894f8039902b9cd6da3db39b43819ca3996.diff

LOG: AMDGPU: Stop validating earlyclobber operands in assembler

This validation was introduced in D34003 for v_qsad/v_mqsad instructions
but it applies to all instructions with earlyclobber operands, which now
includes v_mad_i64/v_mad_u64.

In all these cases I do not think there is documentation saying that the
destination must not overlap the sources. Rather there are *some* cases
where the instruction may not function correctly if there is an overlap,
and we are using earlyclobber as a conservative way of preventing
codegen from generating those cases.

I think it is unhelpful for the assembler to enforce the earlyclobber
restriction because it prevents assembling cases where the programmer
knows that in fact the overlap is safe.

See also: https://github.com/llvm/llvm-project/issues/57610

Differential Revision: https://reviews.llvm.org/D134272

Added: 
    

Modified: 
    llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
    llvm/test/MC/AMDGPU/gfx10_err_pos.s
    llvm/test/MC/AMDGPU/vop3-errs.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index dd96eb583247d..7e4b639d84629 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -1664,7 +1664,6 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
   bool validateSMEMOffset(const MCInst &Inst, const OperandVector &Operands);
   bool validateSOPLiteral(const MCInst &Inst) const;
   bool validateConstantBusLimitations(const MCInst &Inst, const OperandVector &Operands);
-  bool validateEarlyClobberLimitations(const MCInst &Inst, const OperandVector &Operands);
   bool validateIntClampSupported(const MCInst &Inst);
   bool validateMIMGAtomicDMask(const MCInst &Inst);
   bool validateMIMGGatherDMask(const MCInst &Inst);
@@ -3575,46 +3574,6 @@ bool AMDGPUAsmParser::validateConstantBusLimitations(
   return false;
 }
 
-bool
-AMDGPUAsmParser::validateEarlyClobberLimitations(const MCInst &Inst,
-                                                 const OperandVector &Operands) {
-  const unsigned Opcode = Inst.getOpcode();
-  const MCInstrDesc &Desc = MII.get(Opcode);
-
-  const int DstIdx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::vdst);
-  if (DstIdx == -1 ||
-      Desc.getOperandConstraint(DstIdx, MCOI::EARLY_CLOBBER) == -1) {
-    return true;
-  }
-
-  const MCRegisterInfo *TRI = getContext().getRegisterInfo();
-
-  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);
-
-  assert(DstIdx != -1);
-  const MCOperand &Dst = Inst.getOperand(DstIdx);
-  assert(Dst.isReg());
-
-  const int SrcIndices[] = { Src0Idx, Src1Idx, Src2Idx };
-
-  for (int SrcIdx : SrcIndices) {
-    if (SrcIdx == -1) break;
-    const MCOperand &Src = Inst.getOperand(SrcIdx);
-    if (Src.isReg()) {
-      if (TRI->regsOverlap(Dst.getReg(), Src.getReg())) {
-        const unsigned SrcReg = mc2PseudoReg(Src.getReg());
-        Error(getRegLoc(SrcReg, Operands),
-          "destination must be 
diff erent than all sources");
-        return false;
-      }
-    }
-  }
-
-  return true;
-}
-
 bool AMDGPUAsmParser::validateIntClampSupported(const MCInst &Inst) {
 
   const unsigned Opc = Inst.getOpcode();
@@ -4643,9 +4602,6 @@ bool AMDGPUAsmParser::validateInstruction(const MCInst &Inst,
   if (!validateConstantBusLimitations(Inst, Operands)) {
     return false;
   }
-  if (!validateEarlyClobberLimitations(Inst, Operands)) {
-    return false;
-  }
   if (!validateIntClampSupported(Inst)) {
     Error(getImmLoc(AMDGPUOperand::ImmTyClampSI, Operands),
       "integer clamping is not supported on this GPU");

diff  --git a/llvm/test/MC/AMDGPU/gfx10_err_pos.s b/llvm/test/MC/AMDGPU/gfx10_err_pos.s
index 0307a7598f9e3..cc5fd58f8f4f8 100644
--- a/llvm/test/MC/AMDGPU/gfx10_err_pos.s
+++ b/llvm/test/MC/AMDGPU/gfx10_err_pos.s
@@ -1,28 +1,5 @@
 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=+WavefrontSize32,-WavefrontSize64 %s 2>&1 | FileCheck %s --implicit-check-not=error: --strict-whitespace
 
-//==============================================================================
-// destination must be 
diff erent than all sources
-
-v_mqsad_pk_u16_u8 v[0:1], v[1:2], v9, v[4:5]
-// CHECK: error: destination must be 
diff erent than all sources
-// CHECK-NEXT:{{^}}v_mqsad_pk_u16_u8 v[0:1], v[1:2], v9, v[4:5]
-// CHECK-NEXT:{{^}}                          ^
-
-v_mqsad_pk_u16_u8 v[0:1], v[2:3], v0, v[4:5]
-// CHECK: error: destination must be 
diff erent than all sources
-// CHECK-NEXT:{{^}}v_mqsad_pk_u16_u8 v[0:1], v[2:3], v0, v[4:5]
-// CHECK-NEXT:{{^}}                                  ^
-
-v_mqsad_pk_u16_u8 v[0:1], v[2:3], v1, v[4:5]
-// CHECK: error: destination must be 
diff erent than all sources
-// CHECK-NEXT:{{^}}v_mqsad_pk_u16_u8 v[0:1], v[2:3], v1, v[4:5]
-// CHECK-NEXT:{{^}}                                  ^
-
-v_mqsad_pk_u16_u8 v[0:1], v[2:3], v9, v[0:1]
-// CHECK: error: destination must be 
diff erent than all sources
-// CHECK-NEXT:{{^}}v_mqsad_pk_u16_u8 v[0:1], v[2:3], v9, v[0:1]
-// CHECK-NEXT:{{^}}                                      ^
-
 //==============================================================================
 // dim modifier is required on this GPU
 

diff  --git a/llvm/test/MC/AMDGPU/vop3-errs.s b/llvm/test/MC/AMDGPU/vop3-errs.s
index e3e12d05f220c..dcf9b26d5badc 100644
--- a/llvm/test/MC/AMDGPU/vop3-errs.s
+++ b/llvm/test/MC/AMDGPU/vop3-errs.s
@@ -11,31 +11,6 @@ v_div_scale_f32  v24, vcc, v22, 1.1, v22
 
 v_mqsad_u32_u8 v[0:3], s[2:3], v4, v[0:3]
 // GFX67: error: instruction not supported on this GPU
-// GFX89: error: destination must be 
diff erent than all sources
-
-v_mqsad_pk_u16_u8 v[0:1], v[1:2], v9, v[4:5]
-// GCN: error: destination must be 
diff erent than all sources
-
-v_mqsad_pk_u16_u8 v[1:2], v[1:2], v9, v[4:5]
-// GCN: error: destination must be 
diff erent than all sources
-
-v_mqsad_pk_u16_u8 v[2:3], v[1:2], v9, v[4:5]
-// GCN: error: destination must be 
diff erent than all sources
-
-v_mqsad_pk_u16_u8 v[3:4], v[0:1], v9, v[4:5]
-// GCN: error: destination must be 
diff erent than all sources
-
-v_mqsad_pk_u16_u8 v[4:5], v[1:2], v9, v[4:5]
-// GCN: error: destination must be 
diff erent than all sources
-
-v_mqsad_pk_u16_u8 v[5:6], v[1:2], v9, v[4:5]
-// GCN: error: destination must be 
diff erent than all sources
-
-v_mqsad_pk_u16_u8 v[8:9], v[1:2], v9, v[4:5]
-// GCN: error: destination must be 
diff erent than all sources
-
-v_mqsad_pk_u16_u8 v[9:10], v[1:2], v9, v[4:5]
-// GCN: error: destination must be 
diff erent than all sources
 
 v_cmp_eq_f32_e64 vcc, v0, v1 mul:2
 // GCN: error: invalid operand for instruction


        


More information about the llvm-commits mailing list