[llvm-exegesis] Fix serialization/deserialization of special NoRegister register (PR41448)

[lebedev.ri]

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 PR41448.

[courbet]

Thanks Roman.

[lebedev.ri]

In D60517#1462293, @courbet wrote:

Thanks Roman.

Thank you for the review!

llvm/test/tools/llvm-exegesis/X86/uops-CMOV16rm-noreg.s

+# 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,

llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp

 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 
 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 {
 
 namespace {
 
 // A mutable struct holding an LLVMState that can be passed through the
 // serialization process to encode/decode registers and instructions.
 struct YamlContext {
for (unsigned I = 0, E = InstrInfo.getNumOpcodes(); I < E; ++I)
       Map[InstrInfo.getName(I)] = I;
     assert(Map.size() == InstrInfo.getNumOpcodes() && "Size prediction failed");
     return Map;
   };
 
   llvm::StringMap<unsigned>
   generateRegNameToRegNoMapping(const llvm::MCRegisterInfo &RegInfo) {
     llvm::StringMap<unsigned> 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;
   };
 
   void serializeMCInst(const llvm::MCInst &MCInst, llvm::raw_ostream &OS) {
     OS << getInstrName(MCInst.getOpcode());
     for (const auto &Op : MCInst) {
void deserializeMCInst(llvm::StringRef String, llvm::MCInst &Value) {
     }
   }
 
   std::string &getLastError() { return ErrorStream.str(); }
 
   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<unsigned> 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:
   void serializeIntegerOperand(llvm::raw_ostream &OS, int64_t Value) {
     OS << kIntegerPrefix;
     OS.write_hex(llvm::bit_cast<uint64_t>(Value));
   }
 
private:
   llvm::MCOperand deserializeMCOperand(llvm::StringRef String) {
     assert(!String.empty());
     int64_t IntValue = 0;
     double DoubleValue = 0;
     if (tryDeserializeIntegerOperand(String, IntValue))
       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 {};
   }
 
   llvm::StringRef getInstrName(unsigned InstrNo) {
     const llvm::StringRef InstrName = State->getInstrInfo().getName(InstrNo);
     if (InstrName.empty())
template <> struct ScalarTraits<exegesis::RegisterValue> {
   }
 
   static StringRef input(StringRef String, void *Ctx,
                          exegesis::RegisterValue &RV) {
     llvm::SmallVector<llvm::StringRef, 2> Pieces;
     String.split(Pieces, "=0x", /* MaxSplit */ -1,
                  /* KeepEmpty */ false);
     YamlContext &Context = getTypedContext(Ctx);
-    if (Pieces.size() == 2) {
-      RV.Register = Context.getRegNo(Pieces[0]);
+    llvm::Optional<unsigned> 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 {
       Context.getErrorStream()
           << "Unknown initial register value: '" << String << "'";
     }
     return Context.getLastError();
   }