<div dir="ltr">I committed this patch with 'number' renamed to 'id' and 'MBBMapping' renamed to 'MBBSlots' in r240792.<div>Cheers,</div><div>Alex</div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-06-23 16:09 GMT-07:00 Alex L <span dir="ltr"><<a href="mailto:arphaman@gmail.com" target="_blank">arphaman@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">2015-06-23 15:29 GMT-07:00 Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br>
> On 2015-Jun-23, at 15:08, Alex L <<a href="mailto:arphaman@gmail.com" target="_blank">arphaman@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> 2015-06-23 12:43 GMT-07:00 Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>>:<br>
><br>
> > On 2015-Jun-22, at 11:38, Alex Lorenz <<a href="mailto:arphaman@gmail.com" target="_blank">arphaman@gmail.com</a>> wrote:<br>
> ><br>
> > Hi dexonsmith, bob.wilson, bogner,<br>
> ><br>
> > This patch is based on a previous serialization patch that serializes null register operands (<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10580&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1e0Zc3D4Mtk0-xgYIqkzZYpyU38NdSvo_PWTgGH7lnY&s=2gtGozOZSu8W-oynb0eeshJpzZwtkFUNXVOFuw_PdZg&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10580</a>).<br>
> ><br>
> > This patch serializes machine basic block operands.<br>
> > The following syntax is used for basic block operands:<br>
> ><br>
> >    %bb.<name><br>
> >    %bb.<index><br>
> ><br>
> > 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.<br>
> > Example:<br>
> ><br>
> >    JG_1 %bb.if.cond<br>
> >    JG_1 %bb.2<br>
><br>
> This is ambiguous in general.  IIUC, it's legal for two different<br>
> `MachineBasicBlock`s to point at the same `BasicBlock` (see, e.g.,<br>
> `BranchFolder::SplitMBBAt()`), so the name isn't a sufficient identifier.<br>
><br>
> I'm not such a big fan of your numbered identifiers either, since they're<br>
> going to be hard to read, the numbering isn't explicit in the source, and<br>
> the numbering is fragile (removing a basic block in the middle changes<br>
> all the numbers following).<br>
><br>
> Here's a different straw man proposal (I don't love it, so I hope you can<br>
> refine it to something better).<br>
><br>
> First, change the `name:` field to include an ID number.  Suppose you<br>
> have two BBs, %then and %else.  %then has two MBBs.  You'd get something<br>
> like the following:<br>
><br>
>     - name: 0.then<br>
>     - name: 0.else<br>
>     - name: 1.then<br>
><br>
> The syntax is `<num> ['.' <bb-name>]`.  (Maybe the `<num>` should be<br>
> specified in a separate field?)<br>
><br>
> Second, change the references to use both these numbers and the names.<br>
> Maybe, for these:<br>
><br>
>     %bb.0.then<br>
>     %bb.0.else<br>
>     %bb.1.then<br>
><br>
> Unnamed basic blocks fall out naturally as:<br>
><br>
>     %bb.0 ; reference to the first unnamed basic block<br>
>     %bb.1 ; reference to the second unnamed basic block<br>
><br>
> These aren't alternate names, these are the only names.  And the `0` and<br>
> `1` are somehow defined explicitly in MIR.<br>
><br>
> Thoughts?  Other ideas?\<br>
><br>
><br>
> Thanks, yeah this makes sense.<br>
> 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.<br>
><br>
> Alex<br>
><br>
<br>
</div></div>Yup, that fixes the ambiguity.  Lots of options here, but we can always<br>
iterate later.  LGTM (although IMO `id` is a better name than `number`).<br></blockquote><div><br></div></div></div><div>Thanks,</div><div>I like 'id' as well, I will commit this patch with the YAML field 'number' renamed to 'id'.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Alex</div></font></span><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div><br>
><br>
> ><br>
> > REPOSITORY<br>
> >  rL LLVM<br>
> ><br>
> > <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10608&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1e0Zc3D4Mtk0-xgYIqkzZYpyU38NdSvo_PWTgGH7lnY&s=JTq_FUZyv8izps5QFxPws3XMoaOdBkQSbrnPJWVbLx4&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10608</a><br>
> ><br>
> > Files:<br>
> >  lib/CodeGen/MIRParser/MILexer.cpp<br>
> >  lib/CodeGen/MIRParser/MILexer.h<br>
> >  lib/CodeGen/MIRParser/MIParser.cpp<br>
> >  lib/CodeGen/MIRParser/MIParser.h<br>
> >  lib/CodeGen/MIRParser/MIRParser.cpp<br>
> >  lib/CodeGen/MIRPrinter.cpp<br>
> >  test/CodeGen/MIR/X86/large-index-number-error.mir<br>
> >  test/CodeGen/MIR/X86/machine-basic-block-operands.mir<br>
> >  test/CodeGen/MIR/X86/unknown-machine-basic-block.mir<br>
> >  test/CodeGen/MIR/X86/unknown-named-machine-basic-block.mir<br>
> ><br>
> > EMAIL PREFERENCES<br>
> >  <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1e0Zc3D4Mtk0-xgYIqkzZYpyU38NdSvo_PWTgGH7lnY&s=4aZgID7JUmXrnV8m9F8JYh0qM5nALC9lzb77yZo9jqk&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
> > <D10608.28137.patch><br>
<br>
</div></div></blockquote></span></div><br></div></div>
</blockquote></div><br></div>