[llvm] r358183 - [RISCV] Diagnose invalid second input register operand when using %tprel_add

[Roger Ferrer Ibanez via llvm-commits]

Author: rogfer01
Date: Thu Apr 11 08:13:12 2019
New Revision: 358183

URL: http://llvm.org/viewvc/llvm-project?rev=358183&view=rev
Log:
[RISCV] Diagnose invalid second input register operand when using %tprel_add

RISCVMCCodeEmitter::expandAddTPRel asserts that the second operand must be
x4/tp. As we are not currently checking this in the RISCVAsmParser, the assert
is easy to trigger due to wrong assembly input.

This patch does a late check of this constraint.

An alternative could be using a singleton register class for x4/tp similar to
the current one for sp. Unfortunately it does not result in a good diagnostic.
Because add is an overloaded mnemonic, if no matching is possible, the
diagnostic of the first failing alternative seems to be used as the diagnostic
itself. This means that this case the %tprel_add is diagnosed as an invalid
operand (because the real add instruction only has 3 operands).

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


Modified:
    llvm/trunk/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
    llvm/trunk/test/MC/RISCV/rv32i-invalid.s

Modified: llvm/trunk/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp?rev=358183&r1=358182&r2=358183&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp Thu Apr 11 08:13:12 2019
@@ -96,11 +96,18 @@ class RISCVAsmParser : public MCTargetAs
   void emitLoadStoreSymbol(MCInst &Inst, unsigned Opcode, SMLoc IDLoc,
                            MCStreamer &Out, bool HasTmpReg);
 
+  // Checks that a PseudoAddTPRel is using x4/tp in its second input operand.
+  // Enforcing this using a restricted register class for the second input
+  // operand of PseudoAddTPRel results in a poor diagnostic due to the fact
+  // 'add' is an overloaded mnemonic.
+  bool checkPseudoAddTPRel(MCInst &Inst, OperandVector &Operands);
+
   /// Helper for processing MC instructions that have been successfully matched
   /// by MatchAndEmitInstruction. Modifications to the emitted instructions,
   /// like the expansion of pseudo instructions (e.g., "li"), can be performed
   /// in this method.
-  bool processInstruction(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out);
+  bool processInstruction(MCInst &Inst, SMLoc IDLoc, OperandVector &Operands,
+                          MCStreamer &Out);
 
 // Auto-generated instruction matching functions
 #define GET_ASSEMBLER_HEADER
@@ -794,7 +801,7 @@ bool RISCVAsmParser::MatchAndEmitInstruc
   default:
     break;
   case Match_Success:
-    return processInstruction(Inst, IDLoc, Out);
+    return processInstruction(Inst, IDLoc, Operands, Out);
   case Match_MissingFeature:
     return Error(IDLoc, "instruction use requires an option to be enabled");
   case Match_MnemonicFail:
@@ -1596,7 +1603,21 @@ void RISCVAsmParser::emitLoadStoreSymbol
                     Opcode, IDLoc, Out);
 }
 
+bool RISCVAsmParser::checkPseudoAddTPRel(MCInst &Inst,
+                                         OperandVector &Operands) {
+  assert(Inst.getOpcode() == RISCV::PseudoAddTPRel && "Invalid instruction");
+  assert(Inst.getOperand(2).isReg() && "Unexpected second operand kind");
+  if (Inst.getOperand(2).getReg() != RISCV::X4) {
+    SMLoc ErrorLoc = ((RISCVOperand &)*Operands[3]).getStartLoc();
+    return Error(ErrorLoc, "the second input operand must be tp/x4 when using "
+                           "%tprel_add modifier");
+  }
+
+  return false;
+}
+
 bool RISCVAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
+                                        OperandVector &Operands,
                                         MCStreamer &Out) {
   Inst.setLoc(IDLoc);
 
@@ -1675,6 +1696,9 @@ bool RISCVAsmParser::processInstruction(
   case RISCV::PseudoFSD:
     emitLoadStoreSymbol(Inst, RISCV::FSD, IDLoc, Out, /*HasTmpReg=*/true);
     return false;
+  case RISCV::PseudoAddTPRel:
+    if (checkPseudoAddTPRel(Inst, Operands))
+      return true;
   }
 
   emitToStreamer(Out, Inst);

Modified: llvm/trunk/test/MC/RISCV/rv32i-invalid.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/RISCV/rv32i-invalid.s?rev=358183&r1=358182&r2=358183&view=diff
==============================================================================
--- llvm/trunk/test/MC/RISCV/rv32i-invalid.s (original)
+++ llvm/trunk/test/MC/RISCV/rv32i-invalid.s Thu Apr 11 08:13:12 2019
@@ -133,6 +133,7 @@ auipc a0, %pcrel_lo(foo) # CHECK: :[[@LI
 # TP-relative symbol names require a %tprel_add modifier.
 add a0, a0, tp, zero # CHECK: :[[@LINE]]:17: error: expected '%' for operand modifier
 add a0, a0, tp, %hi(foo) # CHECK: :[[@LINE]]:17: error: operand must be a symbol with %tprel_add modifier
+add a0, tp, a0, %tprel_add(foo) # CHECK: :[[@LINE]]:13: error: the second input operand must be tp/x4 when using %tprel_add modifier
 
 # Unrecognized operand modifier
 addi t0, sp, %modifer(255) # CHECK: :[[@LINE]]:15: error: unrecognized operand modifier