[PATCH] MIR Serialization: Serialize machine instruction names.

[Duncan P. N. Exon Smith]

> On 2015 Jun 16, at 10:42, Alex Lorenz <arphaman at gmail.com> wrote:
> 
> Hi dexonsmith, bob.wilson, bogner,
> 
> This patch is based on a patch that implements initial serialization of machine basic blocks (http://reviews.llvm.org/D10465).
> 
> This patch is the initial patch for machine instruction serialization. Only the machine instruction names are serialized by this patch. The instructions are represented using a YAML sequence of string literals and are a part of machine basic block YAML mapping. An example of such MBB mapping is shown below:
> 
>  name: entry
>  instructions:
>    - mov32r0
>    - retq
> 
> A machine instruction parser class 'MIParser' is introduced by this patch. This class will be used to parse the machine instructions. A supporting machine instruction lexing class will be added in the upcoming patch.
> 
> REPOSITORY
>  rL LLVM
> 
> http://reviews.llvm.org/D10481
> 
> Files:
>  include/llvm/CodeGen/MIRYamlMapping.h
>  lib/CodeGen/MIRParser/CMakeLists.txt
>  lib/CodeGen/MIRParser/MIParser.cpp
>  lib/CodeGen/MIRParser/MIParser.h
>  lib/CodeGen/MIRParser/MIRParser.cpp
>  lib/CodeGen/MIRPrinter.cpp
>  test/CodeGen/MIR/X86/lit.local.cfg
>  test/CodeGen/MIR/X86/machine-instructions.mir
>  test/CodeGen/MIR/X86/unknown-instruction.mir
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D10481.27769.patch>

LGTM with a few minor nitpicks, although I know you're waiting on the
MBB parsing.  

> Index: lib/CodeGen/MIRParser/MIParser.cpp
> ===================================================================
> --- /dev/null
> +++ lib/CodeGen/MIRParser/MIParser.cpp
> @@ -0,0 +1,105 @@
> +//===- MIParser.cpp - Machine instructions parser implementation ----------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// This file implements the parsing of machine instructions.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "MIParser.h"
> +#include "llvm/ADT/StringMap.h"
> +#include "llvm/CodeGen/MachineBasicBlock.h"
> +#include "llvm/CodeGen/MachineFunction.h"
> +#include "llvm/CodeGen/MachineInstr.h"
> +#include "llvm/Support/raw_ostream.h"
> +#include "llvm/Support/SourceMgr.h"
> +#include "llvm/Target/TargetSubtargetInfo.h"
> +#include "llvm/Target/TargetInstrInfo.h"
> +
> +using namespace llvm;
> +
> +namespace {
> +
> +class MIParser {
> +  SourceMgr &SM;
> +  MachineFunction &MF;
> +  SMDiagnostic &Error;
> +  StringRef Source;
> +  /// Maps from instruction names to op codes.
> +  StringMap<unsigned> Names2InstrOpCodes;
> +
> +public:
> +  MIParser(SourceMgr &SM, MachineFunction &MF, SMDiagnostic &Error,
> +           StringRef Source);
> +
> +  /// Report an error at the current location with the given message.
> +  ///
> +  /// This function always return true.
> +  bool error(const Twine &Msg);
> +
> +  MachineInstr *parse();
> +
> +private:
> +  void initNames2InstrOpCodes();
> +
> +  /// Try to convert an instruction name to an opcode. Return true if the
> +  /// instruction name is invalid.
> +  bool parseInstrName(StringRef InstrName, unsigned &OpCode);
> +};
> +
> +} // end anonymous namespace
> +
> +MIParser::MIParser(SourceMgr &SM, MachineFunction &MF, SMDiagnostic &Error,
> +                   StringRef Source)
> +    : SM(SM), MF(MF), Error(Error), Source(Source) {}
> +
> +bool MIParser::error(const Twine &Msg) {
> +  // TODO: Get the proper location in the MIR file, not just a location inside
> +  // the string.
> +  Error =
> +      SMDiagnostic(SM, SMLoc(), SM.getMemoryBuffer(SM.getMainFileID())
> +                                    ->getBufferIdentifier(),
> +                   1, 0, SourceMgr::DK_Error, Msg.str(), Source, None, None);
> +  return true;
> +}
> +
> +MachineInstr *MIParser::parse() {
> +  StringRef InstrName = Source;
> +  unsigned OpCode;
> +  if (parseInstrName(InstrName, OpCode)) {
> +    error(Twine("unknown machine instruction name '") + InstrName + "'");
> +    return nullptr;
> +  }

We need a TODO somewhere (maybe here?) for the rest of the instruction ;).

> +
> +  const auto &MCID = MF.getSubtarget().getInstrInfo()->get(OpCode);
> +  auto *MI = MF.CreateMachineInstr(MCID, DebugLoc());
> +  return MI;
> +}
> +
> +void MIParser::initNames2InstrOpCodes() {
> +  if (!Names2InstrOpCodes.empty())
> +    return;
> +  const auto *TII = MF.getSubtarget().getInstrInfo();

Something like:

    assert(TII && "Expected target instruction info");

might be nice.

> +  for (unsigned I = 0, E = TII->getNumOpcodes(); I < E; ++I)
> +    Names2InstrOpCodes.insert(
> +        std::make_pair(StringRef(TII->getName(I)).lower(), I));
> +}
> +
> +bool MIParser::parseInstrName(StringRef InstrName, unsigned &OpCode) {
> +  initNames2InstrOpCodes();
> +  auto InstrInfo = Names2InstrOpCodes.find(InstrName);

Should we support other cases, using `InstrName.lower()`, here?  (I'm
not sure we should; really just a question.  I'm happy to go ahead with
what you have, since it's easy to loosen later.)

> +  if (InstrInfo == Names2InstrOpCodes.end())
> +    return true;
> +  OpCode = InstrInfo->getValue();
> +  return false;
> +}
> +
> +MachineInstr *llvm::parseMachineInstr(SourceMgr &SM, MachineFunction &MF,
> +                                      StringRef Src, SMDiagnostic &Error) {
> +  return MIParser(SM, MF, Error, Src).parse();
> +}