[PATCH] MIR Serialization: Serialize MBB operands.

[Alex L]

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

>
> > 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`).
>

Thanks,
I like 'id' as well, I will commit this patch with the YAML field 'number'
renamed to 'id'.

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/9343bdd3/attachment.html>