[PATCH] MIR Serialization: Serialize MBB operands.
Duncan P. N. Exon Smith
dexonsmith at apple.com
Tue Jun 23 15:29:02 PDT 2015
> 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:
> 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.
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>
More information about the llvm-commits