[PATCH] MIR Serialization: Serialize MBB operands.

[Alex L]

2015-06-23 12:43 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On 2015-Jun-22, at 11:38, Alex Lorenz <arphaman at gmail.com> wrote:
> >
> > Hi dexonsmith, bob.wilson, bogner,
> >
> > This patch is based on a previous serialization patch that serializes
> null register operands (http://reviews.llvm.org/D10580).
> >
> > This patch serializes machine basic block operands.
> > The following syntax is used for basic block operands:
> >
> >    %bb.<name>
> >    %bb.<index>
> >
> > The name is printed when the machine basic block has a name, i.e. when
> its basic block has a name. Otherwise, a number is printed, which is just
> an index into a list of machine basic blocks for that function.
> > Example:
> >
> >    JG_1 %bb.if.cond
> >    JG_1 %bb.2
>
> This is ambiguous in general.  IIUC, it's legal for two different
> `MachineBasicBlock`s to point at the same `BasicBlock` (see, e.g.,
> `BranchFolder::SplitMBBAt()`), so the name isn't a sufficient identifier.
>
> I'm not such a big fan of your numbered identifiers either, since they're
> going to be hard to read, the numbering isn't explicit in the source, and
> the numbering is fragile (removing a basic block in the middle changes
> all the numbers following).
>
> Here's a different straw man proposal (I don't love it, so I hope you can
> refine it to something better).
>
> First, change the `name:` field to include an ID number.  Suppose you
> have two BBs, %then and %else.  %then has two MBBs.  You'd get something
> like the following:
>
>     - name: 0.then
>     - name: 0.else
>     - name: 1.then
>
> The syntax is `<num> ['.' <bb-name>]`.  (Maybe the `<num>` should be
> specified in a separate field?)
>
> Second, change the references to use both these numbers and the names.
> Maybe, for these:
>
>     %bb.0.then
>     %bb.0.else
>     %bb.1.then
>
> Unnamed basic blocks fall out naturally as:
>
>     %bb.0 ; reference to the first unnamed basic block
>     %bb.1 ; reference to the second unnamed basic block
>
> These aren't alternate names, these are the only names.  And the `0` and
> `1` are somehow defined explicitly in MIR.
>
> Thoughts?  Other ideas?\
>


Thanks, yeah this makes sense.
I decided to add a required number field to the MBB YAML mapping, so now
the numbers are explicitly defined in MIR. The references adopt the syntax
proposed by you.

Alex


>
> >
> > REPOSITORY
> >  rL LLVM
> >
> > http://reviews.llvm.org/D10608
> >
> > Files:
> >  lib/CodeGen/MIRParser/MILexer.cpp
> >  lib/CodeGen/MIRParser/MILexer.h
> >  lib/CodeGen/MIRParser/MIParser.cpp
> >  lib/CodeGen/MIRParser/MIParser.h
> >  lib/CodeGen/MIRParser/MIRParser.cpp
> >  lib/CodeGen/MIRPrinter.cpp
> >  test/CodeGen/MIR/X86/large-index-number-error.mir
> >  test/CodeGen/MIR/X86/machine-basic-block-operands.mir
> >  test/CodeGen/MIR/X86/unknown-machine-basic-block.mir
> >  test/CodeGen/MIR/X86/unknown-named-machine-basic-block.mir
> >
> > EMAIL PREFERENCES
> >  http://reviews.llvm.org/settings/panel/emailpreferences/
> > <D10608.28137.patch>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150623/a287d721/attachment.html>