[llvm] r375006 - [RISCV] Add MachineInstr immediate verification
Luis Marques via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 16 08:06:02 PDT 2019
Author: luismarques
Date: Wed Oct 16 08:06:02 2019
New Revision: 375006
URL: http://llvm.org/viewvc/llvm-project?rev=375006&view=rev
Log:
[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
Added:
llvm/trunk/test/CodeGen/RISCV/verify-instr.mir
Modified:
llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.cpp
llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.h
llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td
llvm/trunk/lib/Target/RISCV/RISCVSubtarget.cpp
llvm/trunk/lib/Target/RISCV/Utils/RISCVBaseInfo.h
Modified: llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp?rev=375006&r1=375005&r2=375006&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp (original)
+++ llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp Wed Oct 16 08:06:02 2019
@@ -16,6 +16,7 @@
#include "RISCVMCAsmInfo.h"
#include "RISCVTargetStreamer.h"
#include "TargetInfo/RISCVTargetInfo.h"
+#include "Utils/RISCVBaseInfo.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/CodeGen/Register.h"
#include "llvm/MC/MCAsmInfo.h"
Modified: llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.cpp?rev=375006&r1=375005&r2=375006&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.cpp Wed Oct 16 08:06:02 2019
@@ -29,8 +29,9 @@
using namespace llvm;
-RISCVInstrInfo::RISCVInstrInfo()
- : RISCVGenInstrInfo(RISCV::ADJCALLSTACKDOWN, RISCV::ADJCALLSTACKUP) {}
+RISCVInstrInfo::RISCVInstrInfo(RISCVSubtarget &STI)
+ : RISCVGenInstrInfo(RISCV::ADJCALLSTACKDOWN, RISCV::ADJCALLSTACKUP),
+ STI(STI) {}
unsigned RISCVInstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex) const {
@@ -486,3 +487,58 @@ bool RISCVInstrInfo::isAsCheapAsAMove(co
}
return MI.isAsCheapAsAMove();
}
+
+bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
+ StringRef &ErrInfo) const {
+ const MCInstrInfo *MCII = STI.getInstrInfo();
+ MCInstrDesc const &Desc = MCII->get(MI.getOpcode());
+
+ for (auto &OI : enumerate(Desc.operands())) {
+ unsigned OpType = OI.value().OperandType;
+ if (OpType >= RISCVOp::OPERAND_FIRST_RISCV_IMM &&
+ OpType <= RISCVOp::OPERAND_LAST_RISCV_IMM) {
+ const MachineOperand &MO = MI.getOperand(OI.index());
+ if (MO.isImm()) {
+ int64_t Imm = MO.getImm();
+ bool Ok;
+ switch (OpType) {
+ default:
+ llvm_unreachable("Unexpected operand type");
+ case RISCVOp::OPERAND_UIMM4:
+ Ok = isUInt<4>(Imm);
+ break;
+ case RISCVOp::OPERAND_UIMM5:
+ Ok = isUInt<5>(Imm);
+ break;
+ case RISCVOp::OPERAND_UIMM12:
+ Ok = isUInt<12>(Imm);
+ break;
+ case RISCVOp::OPERAND_SIMM12:
+ Ok = isInt<12>(Imm);
+ break;
+ case RISCVOp::OPERAND_SIMM13_LSB0:
+ Ok = isShiftedInt<12, 1>(Imm);
+ break;
+ case RISCVOp::OPERAND_UIMM20:
+ Ok = isUInt<20>(Imm);
+ break;
+ case RISCVOp::OPERAND_SIMM21_LSB0:
+ Ok = isShiftedInt<20, 1>(Imm);
+ break;
+ case RISCVOp::OPERAND_UIMMLOG2XLEN:
+ if (STI.getTargetTriple().isArch64Bit())
+ Ok = isUInt<6>(Imm);
+ else
+ Ok = isUInt<5>(Imm);
+ break;
+ }
+ if (!Ok) {
+ ErrInfo = "Invalid immediate";
+ return false;
+ }
+ }
+ }
+ }
+
+ return true;
+}
Modified: llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.h?rev=375006&r1=375005&r2=375006&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.h (original)
+++ llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.h Wed Oct 16 08:06:02 2019
@@ -21,10 +21,12 @@
namespace llvm {
+class RISCVSubtarget;
+
class RISCVInstrInfo : public RISCVGenInstrInfo {
public:
- RISCVInstrInfo();
+ explicit RISCVInstrInfo(RISCVSubtarget &STI);
unsigned isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex) const override;
@@ -80,6 +82,12 @@ public:
int64_t BrOffset) const override;
bool isAsCheapAsAMove(const MachineInstr &MI) const override;
+
+ bool verifyInstruction(const MachineInstr &MI,
+ StringRef &ErrInfo) const override;
+
+protected:
+ const RISCVSubtarget &STI;
};
}
#endif
Modified: llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td?rev=375006&r1=375005&r2=375006&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td (original)
+++ llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td Wed Oct 16 08:06:02 2019
@@ -93,6 +93,8 @@ def fencearg : Operand<XLenVT> {
let ParserMatchClass = FenceArg;
let PrintMethod = "printFenceArg";
let DecoderMethod = "decodeUImmOperand<4>";
+ let OperandType = "OPERAND_UIMM4";
+ let OperandNamespace = "RISCVOp";
}
def UImmLog2XLenAsmOperand : AsmOperandClass {
@@ -117,11 +119,15 @@ def uimmlog2xlen : Operand<XLenVT>, ImmL
return isUInt<6>(Imm);
return isUInt<5>(Imm);
}];
+ let OperandType = "OPERAND_UIMMLOG2XLEN";
+ let OperandNamespace = "RISCVOp";
}
def uimm5 : Operand<XLenVT>, ImmLeaf<XLenVT, [{return isUInt<5>(Imm);}]> {
let ParserMatchClass = UImmAsmOperand<5>;
let DecoderMethod = "decodeUImmOperand<5>";
+ let OperandType = "OPERAND_UIMM5";
+ let OperandNamespace = "RISCVOp";
}
def simm12 : Operand<XLenVT>, ImmLeaf<XLenVT, [{return isInt<12>(Imm);}]> {
@@ -134,6 +140,8 @@ def simm12 : Operand<XLenVT>, ImmLeaf<XL
return isInt<12>(Imm);
return MCOp.isBareSymbolRef();
}];
+ let OperandType = "OPERAND_SIMM12";
+ let OperandNamespace = "RISCVOp";
}
// A 13-bit signed immediate where the least significant bit is zero.
@@ -147,6 +155,8 @@ def simm13_lsb0 : Operand<OtherVT> {
return isShiftedInt<12, 1>(Imm);
return MCOp.isBareSymbolRef();
}];
+ let OperandType = "OPERAND_SIMM13_LSB0";
+ let OperandNamespace = "RISCVOp";
}
class UImm20Operand : Operand<XLenVT> {
@@ -158,6 +168,8 @@ class UImm20Operand : Operand<XLenVT> {
return isUInt<20>(Imm);
return MCOp.isBareSymbolRef();
}];
+ let OperandType = "OPERAND_UIMM20";
+ let OperandNamespace = "RISCVOp";
}
def uimm20_lui : UImm20Operand {
@@ -182,6 +194,8 @@ def simm21_lsb0_jal : Operand<OtherVT> {
return isShiftedInt<20, 1>(Imm);
return MCOp.isBareSymbolRef();
}];
+ let OperandType = "OPERAND_SIMM21_LSB0";
+ let OperandNamespace = "RISCVOp";
}
def BareSymbol : AsmOperandClass {
@@ -230,6 +244,8 @@ def csr_sysreg : Operand<XLenVT> {
let ParserMatchClass = CSRSystemRegister;
let PrintMethod = "printCSRSystemRegister";
let DecoderMethod = "decodeUImmOperand<12>";
+ let OperandType = "OPERAND_UIMM12";
+ let OperandNamespace = "RISCVOp";
}
// A parameterized register class alternative to i32imm/i64imm from Target.td.
Modified: llvm/trunk/lib/Target/RISCV/RISCVSubtarget.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/RISCVSubtarget.cpp?rev=375006&r1=375005&r2=375006&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/RISCVSubtarget.cpp (original)
+++ llvm/trunk/lib/Target/RISCV/RISCVSubtarget.cpp Wed Oct 16 08:06:02 2019
@@ -51,7 +51,7 @@ RISCVSubtarget::RISCVSubtarget(const Tri
StringRef ABIName, const TargetMachine &TM)
: RISCVGenSubtargetInfo(TT, CPU, FS),
FrameLowering(initializeSubtargetDependencies(TT, CPU, FS, ABIName)),
- InstrInfo(), RegInfo(getHwMode()), TLInfo(TM, *this) {
+ InstrInfo(*this), RegInfo(getHwMode()), TLInfo(TM, *this) {
CallLoweringInfo.reset(new RISCVCallLowering(*getTargetLowering()));
Legalizer.reset(new RISCVLegalizerInfo(*this));
Modified: llvm/trunk/lib/Target/RISCV/Utils/RISCVBaseInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/Utils/RISCVBaseInfo.h?rev=375006&r1=375005&r2=375006&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/Utils/RISCVBaseInfo.h (original)
+++ llvm/trunk/lib/Target/RISCV/Utils/RISCVBaseInfo.h Wed Oct 16 08:06:02 2019
@@ -16,6 +16,7 @@
#include "MCTargetDesc/RISCVMCTargetDesc.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
+#include "llvm/MC/MCInstrDesc.h"
#include "llvm/MC/SubtargetFeature.h"
namespace llvm {
@@ -63,6 +64,21 @@ enum {
};
} // namespace RISCVII
+namespace RISCVOp {
+enum OperandType : unsigned {
+ OPERAND_FIRST_RISCV_IMM = MCOI::OPERAND_FIRST_TARGET,
+ OPERAND_UIMM4 = OPERAND_FIRST_RISCV_IMM,
+ OPERAND_UIMM5,
+ OPERAND_UIMM12,
+ OPERAND_SIMM12,
+ OPERAND_SIMM13_LSB0,
+ OPERAND_UIMM20,
+ OPERAND_SIMM21_LSB0,
+ OPERAND_UIMMLOG2XLEN,
+ OPERAND_LAST_RISCV_IMM = OPERAND_UIMMLOG2XLEN
+};
+} // namespace RISCVOp
+
// Describes the predecessor/successor bits used in the FENCE instruction.
namespace RISCVFenceField {
enum FenceField {
Added: llvm/trunk/test/CodeGen/RISCV/verify-instr.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/RISCV/verify-instr.mir?rev=375006&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/RISCV/verify-instr.mir (added)
+++ llvm/trunk/test/CodeGen/RISCV/verify-instr.mir Wed Oct 16 08:06:02 2019
@@ -0,0 +1,11 @@
+# RUN: not llc -march=riscv32 -run-pass machineverifier %s -o - 2>&1 | FileCheck %s
+
+# CHECK: *** Bad machine code: Invalid immediate ***
+# CHECK: - instruction: $x2 = ADDI $x1, 10000
+
+---
+name: verify_instr
+body: |
+ bb.0:
+ $x2 = ADDI $x1, 10000
+...
More information about the llvm-commits
mailing list