<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 24, 2015 at 4:18 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On 2015-Feb-24, at 15:55, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
> Use an explicit abbreviation with an Array type for operands.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D7736" target="_blank">http://reviews.llvm.org/D7736</a><br>
><br>
> Files:<br>
> include/llvm/Bitcode/LLVMBitCodes.h<br>
> include/llvm/IR/Instructions.h<br>
> lib/Bitcode/Reader/BitcodeReader.cpp<br>
> lib/Bitcode/Writer/BitcodeWriter.cpp<br>
> test/Bitcode/function-encoding-rel-operands.ll<br>
> tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp<br>
><br>
> EMAIL PREFERENCES<br>
> <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
</div></div>> <D7736.20634.patch><br>
<br>
(A little confused about how your response came in two separate<br>
emails -- does phabricator always do that? -- but I'm responding to<br>
the one with the patch and copying in relevant comments.)<br></blockquote><div><br>Yeah, unfortunately when you push an update to a patch you get to write a message about it - but it's not in reply to existing feedback, so you can't make an infix reply to feedback, etc, which is unfortunate.<br><br>So invariably you end up with two emails, one which is the "posting a new patch" and another which is "replying to feedback" :/<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> Ah, thanks - updated the patch to use this. I'm not sure what array type is ideal (since it'll be a combination of types and value references - is there a way to figure out the maximum size I need for those? I know there's the trick for computing the bit size for type references (& use that for the pointee type of the gep) but maybe there's a way to do that for value numbers too, and take the max of the two? Assuming that produces optimal bit encoding (maybe it's better to use a smaller VBR encoding so we only pay extra bits for some of the higher indicies? I've no idea))<br>
<br>
</span>VBR makes sense here. I don't think a similar trick for values would<br>
be particularly helpful -- since there are so many -- and values are<br>
stored relative to the instruction id anyway (see implementation of<br>
`PushValueAndType()`).<br></blockquote><div><br>Ah, right - yeah, that makes it impossible to know the max bit size of the values before emitting those records, then. Thanks for the pointers/explanation.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> Index: lib/Bitcode/Writer/BitcodeWriter.cpp<br>
> ===================================================================<br>
> --- lib/Bitcode/Writer/BitcodeWriter.cpp<br>
> +++ lib/Bitcode/Writer/BitcodeWriter.cpp<br>
> @@ -2284,6 +2288,19 @@<br>
> Abbv) != FUNCTION_INST_UNREACHABLE_ABBREV)<br>
> llvm_unreachable("Unexpected abbrev ordering!");<br>
> }<br>
> + {<br>
> + BitCodeAbbrev *Abbv = new BitCodeAbbrev();<br>
> + Abbv->Add(BitCodeAbbrevOp(bitc::FUNC_CODE_INST_GEP));<br>
> + Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1));<br>
> + Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, // dest ty<br>
> + Log2_32_Ceil(VE.getTypes().size()+1)));<br>
> + Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));<br>
> + Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));<br>
> + if (Stream.EmitBlockInfoAbbrev(bitc::FUNCTION_BLOCK_ID,<br>
> + Abbv) != FUNCTION_INST_GEP_ABBREV)<br>
> + llvm_unreachable("Unexpected abbrev ordering!");<br>
> + }<br>
> +<br>
><br>
> Stream.ExitBlock();<br>
> }<br>
><br>
<br>
I also don't know what the best VBR is, but since the "default"<br>
encoding seems to be VBR-6, I think that's a better choice in the<br>
absence of data.<br></blockquote><div><br>OK, I'll switch to 6 rather than 8.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Otherwise this LGTM.<br></blockquote><div><br>Thanks!<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>