]> granicus.if.org Git - llvm/commit
[RISCV] Add MachineInstr immediate verification
authorLuis Marques <luismarques@lowrisc.org>
Wed, 16 Oct 2019 15:06:02 +0000 (15:06 +0000)
committerLuis Marques <luismarques@lowrisc.org>
Wed, 16 Oct 2019 15:06:02 +0000 (15:06 +0000)
commit42a92bf21c8adbff205c0009ada1689a30b42024
tree4b1bf38933af68fd72a4a0e1a2ed55b81ce14784
parent9cb56c603f5e64105aa75e32f1d8400ebfcb0d46
[RISCV] Add MachineInstr immediate verification

Summary:
This patch implements the `TargetInstrInfo::verifyInstruction` hook for RISC-V. Currently the hook verifies the machine instruction's immediate operands, to check if the immediates are within the expected bounds. Without the hook invalid immediates are not detected except when doing assembly parsing, so they are silently emitted (including being truncated when emitting object code).

The bounds information is specified in tablegen by using the `OperandType` definition, which sets the `MCOperandInfo`'s `OperandType` field. Several RISC-V-specific immediate operand types were created, which extend the `MCInstrDesc`'s `OperandType` `enum`.

To have the hook called with `llc` pass it the `-verify-machineinstrs` option. For Clang add the cmake build config `-DLLVM_ENABLE_EXPENSIVE_CHECKS=True`, or temporarily patch `TargetPassConfig::addVerifyPass`.

Review concerns:

- The patch adds immediate operand type checks that cover at least the base ISA. There are several other operand types for the C extension and one type for the F/D extensions that were left out of this initial patch because they introduced further design concerns that I felt were best evaluated separately.

- Invalid register classes (e.g. passing a GPR register where a GPRC is expected) are already caught, so were not included.

- This design makes the more abstract `MachineInstr` verification depend on MC layer definitions, which arguably is not the cleanest design, but is in line with how things are done in other parts of the target and LLVM in general.

- There is some duplication of logic already present in the `MCOperandPredicate`s. Since the `MachineInstr` and `MCInstr` notions of immediates are fundamentally different, this is currently necessary.

Reviewers: asb, lenary

Reviewed By: lenary

Subscribers: hiraditya, rbar, johnrusso, simoncook, apazos, sabuasal, niosHD, kito-cheng, shiva0217, jrtc27, MaskRay, zzheng, edward-jones, rogfer01, MartinMosbeck, brucehoult, the_o, rkruppe, PkmX, jocewei, psnobl, benna, Jim, s.egerton, pzheng, llvm-commits

Tags: #llvm

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@375006 91177308-0d34-0410-b5e6-96231b3b80d8
lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
lib/Target/RISCV/RISCVInstrInfo.cpp
lib/Target/RISCV/RISCVInstrInfo.h
lib/Target/RISCV/RISCVInstrInfo.td
lib/Target/RISCV/RISCVSubtarget.cpp
lib/Target/RISCV/Utils/RISCVBaseInfo.h
test/CodeGen/RISCV/verify-instr.mir [new file with mode: 0644]