From d0ef9f31157a9159cf6f2c62b7e4538cbdc81642 Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Tue, 7 Jul 2015 02:08:46 +0000 Subject: [PATCH] MIR Parser: Verify the implicit machine register operands. 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 --- lib/CodeGen/MIRParser/MIParser.cpp | 93 ++++++++++++++++++- .../expected-different-implicit-operand.mir | 38 ++++++++ ...ected-different-implicit-register-flag.mir | 38 ++++++++ .../MIR/X86/expected-number-after-bb.mir | 6 +- .../CodeGen/MIR/X86/global-value-operands.mir | 4 +- .../MIR/X86/large-index-number-error.mir | 6 +- .../MIR/X86/machine-basic-block-operands.mir | 12 +-- test/CodeGen/MIR/X86/machine-instructions.mir | 4 +- .../MIR/X86/missing-implicit-operand.mir | 40 ++++++++ test/CodeGen/MIR/X86/named-registers.mir | 2 +- .../MIR/X86/register-mask-operands.mir | 6 +- .../MIR/X86/unknown-machine-basic-block.mir | 6 +- .../X86/unknown-named-machine-basic-block.mir | 6 +- 13 files changed, 230 insertions(+), 31 deletions(-) create mode 100644 test/CodeGen/MIR/X86/expected-different-implicit-operand.mir create mode 100644 test/CodeGen/MIR/X86/expected-different-implicit-register-flag.mir create mode 100644 test/CodeGen/MIR/X86/missing-implicit-operand.mir diff --git a/lib/CodeGen/MIRParser/MIParser.cpp b/lib/CodeGen/MIRParser/MIParser.cpp index a2ddd48ab16..6b90d904efc 100644 --- a/lib/CodeGen/MIRParser/MIParser.cpp +++ b/lib/CodeGen/MIRParser/MIParser.cpp @@ -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 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 Operands; + SmallVector 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 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 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 index 00000000000..c5f5aaca34e --- /dev/null +++ b/test/CodeGen/MIR/X86/expected-different-implicit-operand.mir @@ -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 index 00000000000..ecf3a122bf6 --- /dev/null +++ b/test/CodeGen/MIR/X86/expected-different-implicit-register-flag.mir @@ -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' +... diff --git a/test/CodeGen/MIR/X86/expected-number-after-bb.mir b/test/CodeGen/MIR/X86/expected-number-after-bb.mir index f4248a76be4..5343a847fbb 100644 --- a/test/CodeGen/MIR/X86/expected-number-after-bb.mir +++ b/test/CodeGen/MIR/X86/expected-number-after-bb.mir @@ -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: diff --git a/test/CodeGen/MIR/X86/global-value-operands.mir b/test/CodeGen/MIR/X86/global-value-operands.mir index 4aa88fe96ce..3ea729b0055 100644 --- a/test/CodeGen/MIR/X86/global-value-operands.mir +++ b/test/CodeGen/MIR/X86/global-value-operands.mir @@ -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' ... diff --git a/test/CodeGen/MIR/X86/large-index-number-error.mir b/test/CodeGen/MIR/X86/large-index-number-error.mir index 61a5bdfe2ed..fdb25c907f5 100644 --- a/test/CodeGen/MIR/X86/large-index-number-error.mir +++ b/test/CodeGen/MIR/X86/large-index-number-error.mir @@ -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' diff --git a/test/CodeGen/MIR/X86/machine-basic-block-operands.mir b/test/CodeGen/MIR/X86/machine-basic-block-operands.mir index 9d1bd0bd58a..607acb5f273 100644 --- a/test/CodeGen/MIR/X86/machine-basic-block-operands.mir +++ b/test/CodeGen/MIR/X86/machine-basic-block-operands.mir @@ -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' diff --git a/test/CodeGen/MIR/X86/machine-instructions.mir b/test/CodeGen/MIR/X86/machine-instructions.mir index b743198cf27..08f3d76486b 100644 --- a/test/CodeGen/MIR/X86/machine-instructions.mir +++ b/test/CodeGen/MIR/X86/machine-instructions.mir @@ -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 index 00000000000..4d2cd03f4a3 --- /dev/null +++ b/test/CodeGen/MIR/X86/missing-implicit-operand.mir @@ -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' +... diff --git a/test/CodeGen/MIR/X86/named-registers.mir b/test/CodeGen/MIR/X86/named-registers.mir index 5defb8489e1..91ed4856867 100644 --- a/test/CodeGen/MIR/X86/named-registers.mir +++ b/test/CodeGen/MIR/X86/named-registers.mir @@ -18,6 +18,6 @@ body: instructions: # CHECK: - '%eax = MOV32r0 # CHECK-NEXT: - 'RETQ %eax - - '%eax = MOV32r0' + - '%eax = MOV32r0 implicit-def %eflags' - 'RETQ %eax' ... diff --git a/test/CodeGen/MIR/X86/register-mask-operands.mir b/test/CodeGen/MIR/X86/register-mask-operands.mir index 719bd9c905d..f4136598ff5 100644 --- a/test/CodeGen/MIR/X86/register-mask-operands.mir +++ b/test/CodeGen/MIR/X86/register-mask-operands.mir @@ -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' ... diff --git a/test/CodeGen/MIR/X86/unknown-machine-basic-block.mir b/test/CodeGen/MIR/X86/unknown-machine-basic-block.mir index 5bc979a83ea..a82e9a780f5 100644 --- a/test/CodeGen/MIR/X86/unknown-machine-basic-block.mir +++ b/test/CodeGen/MIR/X86/unknown-machine-basic-block.mir @@ -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' diff --git a/test/CodeGen/MIR/X86/unknown-named-machine-basic-block.mir b/test/CodeGen/MIR/X86/unknown-named-machine-basic-block.mir index cd8c5402256..f304113f40b 100644 --- a/test/CodeGen/MIR/X86/unknown-named-machine-basic-block.mir +++ b/test/CodeGen/MIR/X86/unknown-named-machine-basic-block.mir @@ -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: -- 2.34.1