[PATCH] MIR Serialization: Serialize MBB operands.

[Alex L]

I committed this patch with 'number' renamed to 'id' and 'MBBMapping'
renamed to 'MBBSlots' in r240792.
Cheers,
Alex

2015-06-23 16:09 GMT-07:00 Alex L <arphaman at gmail.com>:

>
>
> 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/20150626/93e261f2/attachment.html>