[PATCH] MIR Serialization: Print and parse function's MBBs and simple MBB attributes.

[Alex L]

2015-06-17 15:07 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On 2015 Jun 15, at 17:05, Alex Lorenz <arphaman at gmail.com> wrote:
> >
> > Hi dexonsmith, bob.wilson, bogner,
> >
> > This patch implements the initial serialization of the machine basic
> blocks in a machine function. Only the simple, scalar MBB attributes are
> serialized. The link to LLVM IR's basic block is preserved when that basic
> block has a name.
> >
> > REPOSITORY
> >  rL LLVM
> >
> > http://reviews.llvm.org/D10465
> >
> > Files:
> >  include/llvm/CodeGen/MIRYamlMapping.h
> >  lib/CodeGen/MIRParser/MIRParser.cpp
> >  lib/CodeGen/MIRPrinter.cpp
> >  test/CodeGen/MIR/basic-blocks.mir
> >
> > EMAIL PREFERENCES
> >  http://reviews.llvm.org/settings/panel/emailpreferences/
> > <D10465.27730.patch>
>
> > Index: lib/CodeGen/MIRParser/MIRParser.cpp
> > ===================================================================
> > --- lib/CodeGen/MIRParser/MIRParser.cpp
> > +++ lib/CodeGen/MIRParser/MIRParser.cpp
> > @@ -184,6 +191,14 @@
> >    new UnreachableInst(Context, BB);
> >  }
> >
> > +static const BasicBlock *findBasicBlock(const Function *Fn, StringRef
> Name) {
>
> Can you take `Fn` by reference?
>
> > +  for (const BasicBlock &BB : Fn->getBasicBlockList()) {
> > +    if (Name == BB.getName())
> > +      return &BB;
>
> Seems expensive.  Aren't basic blocks stored in `Function`'s symbol
> table?  If so, the whole function should be:
>
>     return
> dyn_cast_or_null<BasicBlock>(Fn.getValueSymbolTable().lookup(Name)));
>
> and maybe you don't need to separate this out at all (although it's fine
> if you think it's cleaner).
>
> > +  }
> > +  return nullptr;
> > +}
> > +
> >  bool MIRParserImpl::initializeMachineFunction(MachineFunction &MF) {
> >    auto It = Functions.find(MF.getName());
> >    if (It == Functions.end())
> > Index: lib/CodeGen/MIRPrinter.cpp
> > ===================================================================
> > --- lib/CodeGen/MIRPrinter.cpp
> > +++ lib/CodeGen/MIRPrinter.cpp
> > @@ -61,10 +64,27 @@
> >    YamlMF.Alignment = MF.getAlignment();
> >    YamlMF.ExposesReturnsTwice = MF.exposesReturnsTwice();
> >    YamlMF.HasInlineAsm = MF.hasInlineAsm();
> > +  for (const auto &MBB : MF) {
> > +    yaml::MachineBasicBlock YamlMBB;
> > +    convert(YamlMBB, MBB);
> > +    YamlMF.BasicBlocks.push_back(YamlMBB);
> > +  }
> >    yaml::Output Out(OS);
> >    Out << YamlMF;
> >  }
> >
> > +void MIRPrinter::convert(yaml::MachineBasicBlock &YamlMBB,
> > +                         const MachineBasicBlock &MBB) {
> > +  const auto *BB = MBB.getBasicBlock();
> > +  if (BB && BB->hasName())
> > +    YamlMBB.Name = BB->getName();
> > +  else
> > +    YamlMBB.Name = "";
>
> What if BB doesn't have a name?  I guess it doesn't get serialized yet?
>

Yes, references to unnamed BB aren't preserved ATM.


>
> That's fine as a temporary missing feature, but I think you should
> phrase it this way:
>
>     // FIXME: Track local value slots in functions so we can serialize
>     // machine basic blocks derived from unnamed LLVM IR basic blocks.
>     assert((!BB || BB->hasName()) &&
>            "No serialization support yet for unnamed basic blocks");
>     YamlMBB.Name = BB ? BB->getName() : "";


> > +  YamlMBB.Alignment = MBB.getAlignment();
> > +  YamlMBB.AddressTaken = MBB.hasAddressTaken();
> > +  YamlMBB.IsLandingPad = MBB.isLandingPad();
> > +}
> > +
> >  void llvm::printMIR(raw_ostream &OS, const Module &M) {
> >    yaml::Output Out(OS);
> >    Out << const_cast<Module &>(M);
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150617/c1501e1f/attachment.html>