MIR Parser: Verify the implicit machine register operands.
authorAlex Lorenz <arphaman@gmail.com>
Tue, 7 Jul 2015 02:08:46 +0000 (02:08 +0000)
committerAlex Lorenz <arphaman@gmail.com>
Tue, 7 Jul 2015 02:08:46 +0000 (02:08 +0000)
This commit verifies that the parsed machine instructions contain the implicit
register operands as specified by the MCInstrDesc. Variadic and call
instructions aren't verified.

Reviewers: Duncan P. N. Exon Smith

Differential Revision: http://reviews.llvm.org/D10781

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@241537 91177308-0d34-0410-b5e6-96231b3b80d8

13 files changed:
lib/CodeGen/MIRParser/MIParser.cpp
test/CodeGen/MIR/X86/expected-different-implicit-operand.mir [new file with mode: 0644]
test/CodeGen/MIR/X86/expected-different-implicit-register-flag.mir [new file with mode: 0644]
test/CodeGen/MIR/X86/expected-number-after-bb.mir
test/CodeGen/MIR/X86/global-value-operands.mir
test/CodeGen/MIR/X86/large-index-number-error.mir
test/CodeGen/MIR/X86/machine-basic-block-operands.mir
test/CodeGen/MIR/X86/machine-instructions.mir
test/CodeGen/MIR/X86/missing-implicit-operand.mir [new file with mode: 0644]
test/CodeGen/MIR/X86/named-registers.mir
test/CodeGen/MIR/X86/register-mask-operands.mir
test/CodeGen/MIR/X86/unknown-machine-basic-block.mir
test/CodeGen/MIR/X86/unknown-named-machine-basic-block.mir

index a2ddd48ab16ee984de97ea28df05f348d1cfe694..6b90d904efcf27cdf37d23aec992088842203f5d 100644 (file)
@@ -29,6 +29,18 @@ using namespace llvm;
 
 namespace {
 
+/// A wrapper struct around the 'MachineOperand' struct that includes a source
+/// range.
+struct MachineOperandWithLocation {
+  MachineOperand Operand;
+  StringRef::iterator Begin;
+  StringRef::iterator End;
+
+  MachineOperandWithLocation(const MachineOperand &Operand,
+                             StringRef::iterator Begin, StringRef::iterator End)
+      : Operand(Operand), Begin(Begin), End(End) {}
+};
+
 class MIParser {
   SourceMgr &SM;
   MachineFunction &MF;
@@ -90,6 +102,9 @@ private:
 
   bool parseInstruction(unsigned &OpCode);
 
+  bool verifyImplicitOperands(ArrayRef<MachineOperandWithLocation> Operands,
+                              const MCInstrDesc &MCID);
+
   void initNames2Regs();
 
   /// Try to convert a register name to a register number. Return true if the
@@ -139,11 +154,12 @@ bool MIParser::parse(MachineInstr *&MI) {
   // Parse any register operands before '='
   // TODO: Allow parsing of multiple operands before '='
   MachineOperand MO = MachineOperand::CreateImm(0);
-  SmallVector<MachineOperand, 8> Operands;
+  SmallVector<MachineOperandWithLocation, 8> Operands;
   if (Token.isRegister() || Token.isRegisterFlag()) {
+    auto Loc = Token.location();
     if (parseRegisterOperand(MO, /*IsDef=*/true))
       return true;
-    Operands.push_back(MO);
+    Operands.push_back(MachineOperandWithLocation(MO, Loc, Token.location()));
     if (Token.isNot(MIToken::equal))
       return error("expected '='");
     lex();
@@ -157,9 +173,10 @@ bool MIParser::parse(MachineInstr *&MI) {
 
   // Parse the remaining machine operands.
   while (Token.isNot(MIToken::Eof)) {
+    auto Loc = Token.location();
     if (parseMachineOperand(MO))
       return true;
-    Operands.push_back(MO);
+    Operands.push_back(MachineOperandWithLocation(MO, Loc, Token.location()));
     if (Token.is(MIToken::Eof))
       break;
     if (Token.isNot(MIToken::comma))
@@ -168,12 +185,16 @@ bool MIParser::parse(MachineInstr *&MI) {
   }
 
   const auto &MCID = MF.getSubtarget().getInstrInfo()->get(OpCode);
+  if (!MCID.isVariadic()) {
+    // FIXME: Move the implicit operand verification to the machine verifier.
+    if (verifyImplicitOperands(Operands, MCID))
+      return true;
+  }
 
   // TODO: Check for extraneous machine operands.
-  // TODO: Check that this instruction has the implicit register operands.
   MI = MF.CreateMachineInstr(MCID, DebugLoc(), /*NoImplicit=*/true);
   for (const auto &Operand : Operands)
-    MI->addOperand(MF, Operand);
+    MI->addOperand(MF, Operand.Operand);
   return false;
 }
 
@@ -190,6 +211,68 @@ bool MIParser::parseMBB(MachineBasicBlock *&MBB) {
   return false;
 }
 
+static const char *printImplicitRegisterFlag(const MachineOperand &MO) {
+  assert(MO.isImplicit());
+  return MO.isDef() ? "implicit-def" : "implicit";
+}
+
+static std::string getRegisterName(const TargetRegisterInfo *TRI,
+                                   unsigned Reg) {
+  assert(TargetRegisterInfo::isPhysicalRegister(Reg) && "expected phys reg");
+  return StringRef(TRI->getName(Reg)).lower();
+}
+
+bool MIParser::verifyImplicitOperands(
+    ArrayRef<MachineOperandWithLocation> Operands, const MCInstrDesc &MCID) {
+  if (MCID.isCall())
+    // We can't verify call instructions as they can contain arbitrary implicit
+    // register and register mask operands.
+    return false;
+
+  // Gather all the expected implicit operands.
+  SmallVector<MachineOperand, 4> ImplicitOperands;
+  if (MCID.ImplicitDefs)
+    for (const uint16_t *ImpDefs = MCID.getImplicitDefs(); *ImpDefs; ++ImpDefs)
+      ImplicitOperands.push_back(
+          MachineOperand::CreateReg(*ImpDefs, true, true));
+  if (MCID.ImplicitUses)
+    for (const uint16_t *ImpUses = MCID.getImplicitUses(); *ImpUses; ++ImpUses)
+      ImplicitOperands.push_back(
+          MachineOperand::CreateReg(*ImpUses, false, true));
+
+  const auto *TRI = MF.getSubtarget().getRegisterInfo();
+  assert(TRI && "Expected target register info");
+  size_t I = ImplicitOperands.size(), J = Operands.size();
+  while (I) {
+    --I;
+    if (J) {
+      --J;
+      const auto &ImplicitOperand = ImplicitOperands[I];
+      const auto &Operand = Operands[J].Operand;
+      if (ImplicitOperand.isIdenticalTo(Operand))
+        continue;
+      if (Operand.isReg() && Operand.isImplicit()) {
+        return error(Operands[J].Begin,
+                     Twine("expected an implicit register operand '") +
+                         printImplicitRegisterFlag(ImplicitOperand) + " %" +
+                         getRegisterName(TRI, ImplicitOperand.getReg()) + "'");
+      }
+    }
+    // TODO: Fix source location when Operands[J].end is right before '=', i.e:
+    // insead of reporting an error at this location:
+    //            %eax = MOV32r0
+    //                 ^
+    // report the error at the following location:
+    //            %eax = MOV32r0
+    //                          ^
+    return error(J < Operands.size() ? Operands[J].End : Token.location(),
+                 Twine("missing implicit register operand '") +
+                     printImplicitRegisterFlag(ImplicitOperands[I]) + " %" +
+                     getRegisterName(TRI, ImplicitOperands[I].getReg()) + "'");
+  }
+  return false;
+}
+
 bool MIParser::parseInstruction(unsigned &OpCode) {
   if (Token.isNot(MIToken::Identifier))
     return error("expected a machine instruction");
diff --git a/test/CodeGen/MIR/X86/expected-different-implicit-operand.mir b/test/CodeGen/MIR/X86/expected-different-implicit-operand.mir
new file mode 100644 (file)
index 0000000..c5f5aac
--- /dev/null
@@ -0,0 +1,38 @@
+# RUN: not llc -march=x86-64 -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
+
+--- |
+
+  define i32 @foo(i32* %p) {
+  entry:
+    %a = load i32, i32* %p
+    %0 = icmp sle i32 %a, 10
+    br i1 %0, label %less, label %exit
+
+  less:
+    ret i32 0
+
+  exit:
+    ret i32 %a
+  }
+
+
+...
+---
+name:            foo
+body:
+ - id:              0
+   name:            entry
+   instructions:
+     - '%eax = MOV32rm %rdi, 1, _, 0, _'
+     - 'CMP32ri8 %eax, 10, implicit-def %eflags'
+# CHECK: [[@LINE+1]]:26: expected an implicit register operand 'implicit %eflags'
+     - 'JG_1 %bb.2.exit, implicit %eax'
+ - id:              1
+   name:            less
+   instructions:
+     - '%eax = MOV32r0 implicit-def %eflags'
+ - id:              2
+   name:            exit
+   instructions:
+     - 'RETQ %eax'
+...
diff --git a/test/CodeGen/MIR/X86/expected-different-implicit-register-flag.mir b/test/CodeGen/MIR/X86/expected-different-implicit-register-flag.mir
new file mode 100644 (file)
index 0000000..ecf3a12
--- /dev/null
@@ -0,0 +1,38 @@
+# RUN: not llc -march=x86-64 -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
+
+--- |
+
+  define i32 @foo(i32* %p) {
+  entry:
+    %a = load i32, i32* %p
+    %0 = icmp sle i32 %a, 10
+    br i1 %0, label %less, label %exit
+
+  less:
+    ret i32 0
+
+  exit:
+    ret i32 %a
+  }
+
+
+...
+---
+name:            foo
+body:
+ - id:              0
+   name:            entry
+   instructions:
+     - '%eax = MOV32rm %rdi, 1, _, 0, _'
+     - 'CMP32ri8 %eax, 10, implicit-def %eflags'
+# CHECK: [[@LINE+1]]:26: expected an implicit register operand 'implicit %eflags'
+     - 'JG_1 %bb.2.exit, implicit-def %eflags'
+ - id:              1
+   name:            less
+   instructions:
+     - '%eax = MOV32r0 implicit-def %eflags'
+ - id:              2
+   name:            exit
+   instructions:
+     - 'RETQ %eax'
+...
index f4248a76be461bf1586c03a2144b244fca59d5f9..5343a847fbb90a0d49fd5857dc3f6fb4d94563be 100644 (file)
@@ -23,13 +23,13 @@ body:
    name:   entry
    instructions:
      - '%eax = MOV32rm %rdi, 1, _, 0, _'
-     - 'CMP32ri8 %eax, 10'
+     - 'CMP32ri8 %eax, 10, implicit-def %eflags'
      # CHECK: [[@LINE+1]]:18: expected a number after '%bb.'
-     - 'JG_1 %bb.nah'
+     - 'JG_1 %bb.nah, implicit %eflags'
  - id: 1
    name: yes
    instructions:
-     - '%eax = MOV32r0'
+     - '%eax = MOV32r0 implicit-def %eflags'
  - id: 2
    name: nah
    instructions:
index 4aa88fe96cebee318caf3a1a9037d61f1622162e..3ea729b00554dd16514a991fe8870b109617cc8d 100644 (file)
@@ -31,7 +31,7 @@ body:
       # CHECK: - '%rax = MOV64rm %rip, 1, _, @G, _'
       - '%rax = MOV64rm %rip, 1, _, @G, _'
       - '%eax = MOV32rm %rax, 1, _, 0, _'
-      - '%eax = INC32r %eax'
+      - '%eax = INC32r %eax, implicit-def %eflags'
       - 'RETQ %eax'
 ...
 ---
@@ -44,6 +44,6 @@ body:
       # CHECK: - '%rax = MOV64rm %rip, 1, _, @0, _'
       - '%rax = MOV64rm %rip, 1, _, @0, _'
       - '%eax = MOV32rm %rax, 1, _, 0, _'
-      - '%eax = INC32r %eax'
+      - '%eax = INC32r %eax, implicit-def %eflags'
       - 'RETQ %eax'
 ...
index 61a5bdfe2edb68e4adc9de188e59961289ae0fab..fdb25c907f527b97691008939037dbc0bffdb127 100644 (file)
@@ -23,12 +23,12 @@ body:
    name: entry
    instructions:
      - '%eax = MOV32rm %rdi, 1, _, 0, _'
-     - 'CMP32ri8 %eax, 10'
+     - 'CMP32ri8 %eax, 10, implicit-def %eflags'
      # CHECK: [[@LINE+1]]:14: expected 32-bit integer (too large)
-     - 'JG_1 %bb.123456789123456'
+     - 'JG_1 %bb.123456789123456, implicit %eflags'
  - id: 1
    instructions:
-     - '%eax = MOV32r0'
+     - '%eax = MOV32r0 implicit-def %eflags'
  - id: 2
    instructions:
      - 'RETQ %eax'
index 9d1bd0bd58adcfa9a887a4da0f0e1f59b2ba914a..607acb5f273ebe3390f36c2a3b6888272060fb2b 100644 (file)
@@ -41,13 +41,13 @@ body:
      - '%eax = MOV32rm %rdi, 1, _, 0, _'
      # CHECK:      - 'CMP32ri8 %eax, 10
      # CHECK-NEXT: - 'JG_1 %bb.2.exit
-     - 'CMP32ri8 %eax, 10'
-     - 'JG_1 %bb.2.exit'
+     - 'CMP32ri8 %eax, 10, implicit-def %eflags'
+     - 'JG_1 %bb.2.exit, implicit %eflags'
  # CHECK: name: less
  - id:              1
    name:            less
    instructions:
-     - '%eax = MOV32r0'
+     - '%eax = MOV32r0 implicit-def %eflags'
  - id:              2
    name:            exit
    instructions:
@@ -64,11 +64,11 @@ body:
      - '%eax = MOV32rm %rdi, 1, _, 0, _'
      # CHECK:      - 'CMP32ri8 %eax, 10
      # CHECK-NEXT: - 'JG_1 %bb.2
-     - 'CMP32ri8 %eax, 10'
-     - 'JG_1 %bb.3'
+     - 'CMP32ri8 %eax, 10, implicit-def %eflags'
+     - 'JG_1 %bb.3, implicit %eflags'
  - id: 1
    instructions:
-     - '%eax = MOV32r0'
+     - '%eax = MOV32r0 implicit-def %eflags'
  - id: 3
    instructions:
      - 'RETQ %eax'
index b743198cf27073d2d3bbec6a269546ced370c673..08f3d76486b15d32097be0c2d2af231dd6029d70 100644 (file)
@@ -18,8 +18,8 @@ body:
  - id:           0
    name:         entry
    instructions:
-     # CHECK:      - IMUL32rri8
+     # CHECK:      - MOV32rr
      # CHECK-NEXT: - RETQ
-     - IMUL32rri8
+     - MOV32rr
      - ' RETQ '
 ...
diff --git a/test/CodeGen/MIR/X86/missing-implicit-operand.mir b/test/CodeGen/MIR/X86/missing-implicit-operand.mir
new file mode 100644 (file)
index 0000000..4d2cd03
--- /dev/null
@@ -0,0 +1,40 @@
+# RUN: not llc -march=x86-64 -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
+# This test ensures that the MIR parser reports an error when an instruction
+# is missing one of its implicit register operands.
+
+--- |
+
+  define i32 @foo(i32* %p) {
+  entry:
+    %a = load i32, i32* %p
+    %0 = icmp sle i32 %a, 10
+    br i1 %0, label %less, label %exit
+
+  less:
+    ret i32 0
+
+  exit:
+    ret i32 %a
+  }
+
+
+...
+---
+name:            foo
+body:
+ - id:              0
+   name:            entry
+   instructions:
+     - '%eax = MOV32rm %rdi, 1, _, 0, _'
+     - 'CMP32ri8 %eax, 10, implicit-def %eflags'
+# CHECK: [[@LINE+1]]:24: missing implicit register operand 'implicit %eflags'
+     - 'JG_1 %bb.2.exit'
+ - id:              1
+   name:            less
+   instructions:
+     - '%eax = MOV32r0 implicit-def %eflags'
+ - id:              2
+   name:            exit
+   instructions:
+     - 'RETQ %eax'
+...
index 5defb8489e1e77c1382e8639539c59b7bb6af29d..91ed4856867833ad3bf44786249bd9ccfaf2bd7f 100644 (file)
@@ -18,6 +18,6 @@ body:
    instructions:
      # CHECK:      - '%eax = MOV32r0
      # CHECK-NEXT: - 'RETQ %eax
-     - '%eax = MOV32r0'
+     - '%eax = MOV32r0 implicit-def %eflags'
      - 'RETQ %eax'
 ...
index 719bd9c905d74f8089c48764775fd3aa9405581e..f4136598ff5c0b9cf9b81e63f39e61ee0fc1ec39 100644 (file)
@@ -24,7 +24,7 @@ body:
   - id:          0
     name:        body
     instructions:
-      - '%eax = IMUL32rri8 %edi, 11'
+      - '%eax = IMUL32rri8 %edi, 11, implicit-def %eflags'
       - 'RETQ %eax'
 ...
 ---
@@ -36,8 +36,8 @@ body:
     instructions:
       # CHECK:      - 'PUSH64r %rax
       # CHECK-NEXT: - 'CALL64pcrel32 @compute, csr_64, implicit %rsp, implicit %edi, implicit-def %rsp, implicit-def %eax'
-      - 'PUSH64r %rax'
+      - 'PUSH64r %rax, implicit-def %rsp, implicit %rsp'
       - 'CALL64pcrel32 @compute, csr_64, implicit %rsp, implicit %edi, implicit-def %rsp, implicit-def %eax'
-      - '%rdx = POP64r'
+      - '%rdx = POP64r implicit-def %rsp, implicit %rsp'
       - 'RETQ %eax'
 ...
index 5bc979a83eafd5738db46eb5189ada571db2f6ec..a82e9a780f542ecce304057c1e9f4abfa8b0257c 100644 (file)
@@ -26,12 +26,12 @@ body:
    name:         entry
    instructions:
      - '%eax = MOV32rm %rdi, 1, _, 0, _'
-     - 'CMP32ri8 %eax, 10'
+     - 'CMP32ri8 %eax, 10, implicit-def %eflags'
      # CHECK: [[@LINE+1]]:14: use of undefined machine basic block #4
-     - 'JG_1 %bb.4'
+     - 'JG_1 %bb.4, implicit %eflags'
  - id: 1
    instructions:
-     - '%eax = MOV32r0'
+     - '%eax = MOV32r0 implicit-def %eflags'
  - id: 2
    instructions:
      - 'RETQ %eax'
index cd8c5402256fbe11894fe9e80a8373bcddf6fc3f..f304113f40b9cb0c4cc44c7045481e3f5e329d38 100644 (file)
@@ -25,13 +25,13 @@ body:
    name:            entry
    instructions:
      - '%eax = MOV32rm %rdi, 1, _, 0, _'
-     - 'CMP32ri8 %eax, 10'
+     - 'CMP32ri8 %eax, 10, implicit-def %eflags'
      # CHECK: [[@LINE+1]]:14: the name of machine basic block #2 isn't 'hit'
-     - 'JG_1 %bb.2.hit'
+     - 'JG_1 %bb.2.hit, implicit %eflags'
  - id:              1
    name:            less
    instructions:
-     - '%eax = MOV32r0'
+     - '%eax = MOV32r0 implicit-def %eflags'
  - id:              2
    name:            exit
    instructions: