[PATCH] MIR Serialization: Serialize machine instruction names.

Alex L arphaman at gmail.com
Wed Jun 17 17:33:03 PDT 2015


2015-06-17 16:49 GMT-07:00 Ahmed Bougacha <ahmed.bougacha at gmail.com>:

> On Wed, Jun 17, 2015 at 4:12 PM, Alex L <arphaman at gmail.com> wrote:
> >
> >
> > 2015-06-17 15:25 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com
> >:
> >>
> >>
> >> > 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.)
> >
> >
> > I'm not sure really, I just print and parse everything using lowercase
> name,
> > like LLVM IR.
> > I think this can be changed later if other people want case insensitive
> > format.
>
> Ah, here's the email thread ;)
>
> I think case sensitivity makes more sense, and might be necessary
> (matching tablegen, IIRC).  All else being equal, please do that
> instead!
>
> -Ahmed
>

I don't mind preserving the uppercase names just like they're defined in
TargetInstrInfo, I just think that
lowercase names are better for reading and writing the MIR files.


>
> >>
> >>
> >> > +  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();
> >> > +}
> >>
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150617/9f16d036/attachment.html>


More information about the llvm-commits mailing list