From b3382d253f7e8a381463b7ee54d0d232d04502da Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Thu, 11 Apr 2019 07:20:50 +0000 Subject: [PATCH] [llvm-exegesis] Fix serialization/deserialization of special NoRegister register (PR41448) Summary: A *lot* of instructions have this special register. It seems this never really worked, but i finally noticed it only because it happened to break for `CMOV16rm` instruction. We serialized that register as "" (empty string), which is naturally 'ignored' during deserialization, so we re-create a `MCInst` with too few operands. And when we then happened to try to resolve variant sched class for this mis-serialized instruction, and the variant predicate tried to read an operand that was out of bounds since we got less operands, we crashed. Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=41448 | PR41448 ]]. Reviewers: craig.topper, courbet Reviewed By: courbet Subscribers: tschuett, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D60517 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@358153 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../llvm-exegesis/X86/uops-CMOV16rm-noreg.s | 17 +++++++++++++++ tools/llvm-exegesis/lib/BenchmarkResult.cpp | 21 ++++++++++++------- 2 files changed, 31 insertions(+), 7 deletions(-) create mode 100644 test/tools/llvm-exegesis/X86/uops-CMOV16rm-noreg.s diff --git a/test/tools/llvm-exegesis/X86/uops-CMOV16rm-noreg.s b/test/tools/llvm-exegesis/X86/uops-CMOV16rm-noreg.s new file mode 100644 index 00000000000..3fc1f31d54d --- /dev/null +++ b/test/tools/llvm-exegesis/X86/uops-CMOV16rm-noreg.s @@ -0,0 +1,17 @@ +# RUN: llvm-exegesis -mode=uops -opcode-name=CMOV16rm -benchmarks-file=%t.CMOV16rm-uops.yaml +# RUN: FileCheck -check-prefixes=CHECK-YAML -input-file=%t.CMOV16rm-uops.yaml %s +# RUN: llvm-exegesis -mcpu=bdver2 -mode=analysis -benchmarks-file=%t.CMOV16rm-uops.yaml -analysis-clusters-output-file=- -analysis-clustering-epsilon=0.1 -analysis-inconsistency-epsilon=0.1 -analysis-numpoints=1 -analysis-clustering=naive | FileCheck -check-prefixes=CHECK-CLUSTERS %s + +# https://bugs.llvm.org/show_bug.cgi?id=41448 +# 1. Verify that we correctly serialize RegNo 0 as %noreg, not as an empty string! +# 2. Verify that deserialization works. Since CMOV16rm has a variant sched class, just printing clusters is sufficient + +CHECK-YAML: --- +CHECK-YAML-NEXT: mode: uops +CHECK-YAML-NEXT: key: +CHECK-YAML-NEXT: instructions: +CHECK-YAML-NEXT: - 'CMOV16rm {{[A-Z0-9]+}} {{[A-Z0-9]+}} {{[A-Z0-9]+}} i_0x1 %noreg i_0x0 %noreg i_0x{{[0-9a-f]}}' +CHECK-YAML-LAST: ... + +# CHECK-CLUSTERS: {{^}}cluster_id,opcode_name,config,sched_class, +# CHECK-CLUSTERS-NEXT: {{^}}0, diff --git a/tools/llvm-exegesis/lib/BenchmarkResult.cpp b/tools/llvm-exegesis/lib/BenchmarkResult.cpp index 4a696e2e7d3..84ef7ac0f27 100644 --- a/tools/llvm-exegesis/lib/BenchmarkResult.cpp +++ b/tools/llvm-exegesis/lib/BenchmarkResult.cpp @@ -21,6 +21,7 @@ static constexpr const char kIntegerPrefix[] = "i_0x"; static constexpr const char kDoublePrefix[] = "f_"; static constexpr const char kInvalidOperand[] = "INVALID"; +static constexpr llvm::StringLiteral kNoRegister("%noreg"); namespace llvm { @@ -47,7 +48,9 @@ struct YamlContext { llvm::StringMap generateRegNameToRegNoMapping(const llvm::MCRegisterInfo &RegInfo) { llvm::StringMap Map(RegInfo.getNumRegs()); - for (unsigned I = 0, E = RegInfo.getNumRegs(); I < E; ++I) + // Special-case RegNo 0, which would otherwise be spelled as ''. + Map[kNoRegister] = 0; + for (unsigned I = 1, E = RegInfo.getNumRegs(); I < E; ++I) Map[RegInfo.getName(I)] = I; assert(Map.size() == RegInfo.getNumRegs() && "Size prediction failed"); return Map; @@ -83,18 +86,21 @@ struct YamlContext { llvm::raw_string_ostream &getErrorStream() { return ErrorStream; } llvm::StringRef getRegName(unsigned RegNo) { + // Special case: RegNo 0 is NoRegister. We have to deal with it explicitly. + if (RegNo == 0) + return kNoRegister; const llvm::StringRef RegName = State->getRegInfo().getName(RegNo); if (RegName.empty()) ErrorStream << "No register with enum value '" << RegNo << "'\n"; return RegName; } - unsigned getRegNo(llvm::StringRef RegName) { + llvm::Optional getRegNo(llvm::StringRef RegName) { auto Iter = RegNameToRegNo.find(RegName); if (Iter != RegNameToRegNo.end()) return Iter->second; ErrorStream << "No register with name '" << RegName << "'\n"; - return 0; + return llvm::None; } private: @@ -142,8 +148,8 @@ private: return llvm::MCOperand::createImm(IntValue); if (tryDeserializeFPOperand(String, DoubleValue)) return llvm::MCOperand::createFPImm(DoubleValue); - if (unsigned RegNo = getRegNo(String)) - return llvm::MCOperand::createReg(RegNo); + if (auto RegNo = getRegNo(String)) + return llvm::MCOperand::createReg(*RegNo); if (String != kInvalidOperand) ErrorStream << "Unknown Operand: '" << String << "'\n"; return {}; @@ -258,8 +264,9 @@ template <> struct ScalarTraits { String.split(Pieces, "=0x", /* MaxSplit */ -1, /* KeepEmpty */ false); YamlContext &Context = getTypedContext(Ctx); - if (Pieces.size() == 2) { - RV.Register = Context.getRegNo(Pieces[0]); + llvm::Optional RegNo; + if (Pieces.size() == 2 && (RegNo = Context.getRegNo(Pieces[0]))) { + RV.Register = *RegNo; const unsigned BitsNeeded = llvm::APInt::getBitsNeeded(Pieces[1], kRadix); RV.Value = llvm::APInt(BitsNeeded, Pieces[1], kRadix); } else { -- 2.50.1