[llvm-branch-commits] [llvm] e97dd11 - [AMDGPU][MC] Corrected error position for invalid MOVREL src

[Dmitry Preobrazhensky via llvm-branch-commits]

Author: Dmitry Preobrazhensky
Date: 2020-12-05T13:23:14+03:00
New Revision: e97dd119776129e888ab3c02e1394b58b709fd1f

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

LOG: [AMDGPU][MC] Corrected error position for invalid MOVREL src

See bug 47518 (https://bugs.llvm.org/show_bug.cgi?id=47518)

Reviewers: rampitec

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index d1e5fe59e910..8128bbcaa65d 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -1401,7 +1401,7 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
   bool validateIntClampSupported(const MCInst &Inst);
   bool validateMIMGAtomicDMask(const MCInst &Inst);
   bool validateMIMGGatherDMask(const MCInst &Inst);
-  bool validateMovrels(const MCInst &Inst);
+  bool validateMovrels(const MCInst &Inst, const OperandVector &Operands);
   bool validateMIMGDataSize(const MCInst &Inst);
   bool validateMIMGAddrSize(const MCInst &Inst);
   bool validateMIMGD16(const MCInst &Inst);
@@ -3337,7 +3337,8 @@ static bool IsMovrelsSDWAOpcode(const unsigned Opcode)
 // movrels* opcodes should only allow VGPRS as src0.
 // This is specified in .td description for vop1/vop3,
 // but sdwa is handled 
diff erently. See isSDWAOperand.
-bool AMDGPUAsmParser::validateMovrels(const MCInst &Inst) {
+bool AMDGPUAsmParser::validateMovrels(const MCInst &Inst,
+                                      const OperandVector &Operands) {
 
   const unsigned Opc = Inst.getOpcode();
   const MCInstrDesc &Desc = MII.get(Opc);
@@ -3348,13 +3349,20 @@ bool AMDGPUAsmParser::validateMovrels(const MCInst &Inst) {
   const int Src0Idx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::src0);
   assert(Src0Idx != -1);
 
+  SMLoc ErrLoc;
   const MCOperand &Src0 = Inst.getOperand(Src0Idx);
-  if (!Src0.isReg())
-    return false;
+  if (Src0.isReg()) {
+    auto Reg = mc2PseudoReg(Src0.getReg());
+    const MCRegisterInfo *TRI = getContext().getRegisterInfo();
+    if (!isSGPR(Reg, TRI))
+      return true;
+    ErrLoc = getRegLoc(Reg, Operands);
+  } else {
+    ErrLoc = getConstLoc(Operands);
+  }
 
-  auto Reg = Src0.getReg();
-  const MCRegisterInfo *TRI = getContext().getRegisterInfo();
-  return !isSGPR(mc2PseudoReg(Reg), TRI);
+  Error(ErrLoc, "source operand must be a VGPR");
+  return false;
 }
 
 bool AMDGPUAsmParser::validateMAIAccWrite(const MCInst &Inst,
@@ -3899,8 +3907,7 @@ bool AMDGPUAsmParser::validateInstruction(const MCInst &Inst,
       "invalid image_gather dmask: only one bit must be set");
     return false;
   }
-  if (!validateMovrels(Inst)) {
-    Error(IDLoc, "source operand must be a VGPR");
+  if (!validateMovrels(Inst, Operands)) {
     return false;
   }
   if (!validateFlatOffset(Inst, Operands)) {
@@ -4033,7 +4040,7 @@ bool AMDGPUAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
     SMLoc ErrorLoc = IDLoc;
     if (ErrorInfo != ~0ULL) {
       if (ErrorInfo >= Operands.size()) {
-        return Error(IDLoc, "too few operands for instruction");
+        return Error(getLoc(), "too few operands for instruction");
       }
       ErrorLoc = ((AMDGPUOperand &)*Operands[ErrorInfo]).getStartLoc();
       if (ErrorLoc == SMLoc())

diff  --git a/llvm/test/MC/AMDGPU/exp-err.s b/llvm/test/MC/AMDGPU/exp-err.s
index ee83bef0c50b..b650a78627db 100644
--- a/llvm/test/MC/AMDGPU/exp-err.s
+++ b/llvm/test/MC/AMDGPU/exp-err.s
@@ -53,7 +53,7 @@ exp , v3, v2, v1, v0
 // GCN: :5: error: unknown token in expression
 
 exp
-// GCN: :1: error: too few operands for instruction
+// GCN: :4: error: too few operands for instruction
 
 exp mrt0 s0, v0, v0, v0
 // GCN: 10: error: invalid operand for instruction

diff  --git a/llvm/test/MC/AMDGPU/gfx10_err_pos.s b/llvm/test/MC/AMDGPU/gfx10_err_pos.s
index c1aa9f860b5c..f7c9bd914f46 100644
--- a/llvm/test/MC/AMDGPU/gfx10_err_pos.s
+++ b/llvm/test/MC/AMDGPU/gfx10_err_pos.s
@@ -938,7 +938,17 @@ s_mov_b64 s[10:11], [s2,s1]
 v_movrels_b32_sdwa v0, 1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD
 // CHECK: error: source operand must be a VGPR
 // CHECK-NEXT:{{^}}v_movrels_b32_sdwa v0, 1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD
-// CHECK-NEXT:{{^}}^
+// CHECK-NEXT:{{^}}                       ^
+
+v_movrels_b32_sdwa v0, s0
+// CHECK: error: source operand must be a VGPR
+// CHECK-NEXT:{{^}}v_movrels_b32_sdwa v0, s0
+// CHECK-NEXT:{{^}}                       ^
+
+v_movrels_b32_sdwa v0, shared_base
+// CHECK: error: source operand must be a VGPR
+// CHECK-NEXT:{{^}}v_movrels_b32_sdwa v0, shared_base
+// CHECK-NEXT:{{^}}                       ^
 
 //==============================================================================
 // specified hardware register is not supported on this GPU
@@ -954,12 +964,12 @@ s_getreg_b32 s2, hwreg(HW_REG_SHADER_CYCLES)
 tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7]
 // CHECK: error: too few operands for instruction
 // CHECK-NEXT:{{^}}tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7]
-// CHECK-NEXT:{{^}}^
+// CHECK-NEXT:{{^}}                                                ^
 
 v_add_f32_e64 v0, v1
 // CHECK: error: too few operands for instruction
 // CHECK-NEXT:{{^}}v_add_f32_e64 v0, v1
-// CHECK-NEXT:{{^}}^
+// CHECK-NEXT:{{^}}                    ^
 
 //==============================================================================
 // too large value for expcnt