[PATCH] MIR Serialization: Serialize MBB operands.

[Duncan P. N. Exon Smith]

> On 2015-Jun-23, at 15:08, Alex L <arphaman at gmail.com> wrote:
> 
> 
> 
> 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
>  

Yup, that fixes the ambiguity.  Lots of options here, but we can always
iterate later.  LGTM (although IMO `id` is a better name than `number`).

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