[PATCH] [opaque pointer type] Bitcode support for explicit type parameter on GEP.

David Blaikie dblaikie at gmail.com
Tue Feb 24 16:25:56 PST 2015


On Tue, Feb 24, 2015 at 4:18 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2015-Feb-24, at 15:55, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Use an explicit abbreviation with an Array type for operands.
> >
> >
> > http://reviews.llvm.org/D7736
> >
> > Files:
> >  include/llvm/Bitcode/LLVMBitCodes.h
> >  include/llvm/IR/Instructions.h
> >  lib/Bitcode/Reader/BitcodeReader.cpp
> >  lib/Bitcode/Writer/BitcodeWriter.cpp
> >  test/Bitcode/function-encoding-rel-operands.ll
> >  tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp
> >
> > EMAIL PREFERENCES
> >  http://reviews.llvm.org/settings/panel/emailpreferences/
> > <D7736.20634.patch>
>
> (A little confused about how your response came in two separate
> emails -- does phabricator always do that? -- but I'm responding to
> the one with the patch and copying in relevant comments.)
>

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.

So invariably you end up with two emails, one which is the "posting a new
patch" and another which is "replying to feedback" :/


> > 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))
>
> VBR makes sense here.  I don't think a similar trick for values would
> be particularly helpful -- since there are so many -- and values are
> stored relative to the instruction id anyway (see implementation of
> `PushValueAndType()`).
>

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.


>
> > Index: lib/Bitcode/Writer/BitcodeWriter.cpp
> > ===================================================================
> > --- lib/Bitcode/Writer/BitcodeWriter.cpp
> > +++ lib/Bitcode/Writer/BitcodeWriter.cpp
> > @@ -2284,6 +2288,19 @@
> >                                     Abbv) !=
> FUNCTION_INST_UNREACHABLE_ABBREV)
> >        llvm_unreachable("Unexpected abbrev ordering!");
> >    }
> > +  {
> > +    BitCodeAbbrev *Abbv = new BitCodeAbbrev();
> > +    Abbv->Add(BitCodeAbbrevOp(bitc::FUNC_CODE_INST_GEP));
> > +    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1));
> > +    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed,       // dest ty
> > +                              Log2_32_Ceil(VE.getTypes().size()+1)));
> > +    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
> > +    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
> > +    if (Stream.EmitBlockInfoAbbrev(bitc::FUNCTION_BLOCK_ID,
> > +                                   Abbv) != FUNCTION_INST_GEP_ABBREV)
> > +      llvm_unreachable("Unexpected abbrev ordering!");
> > +  }
> > +
> >
> >    Stream.ExitBlock();
> >  }
> >
>
> I also don't know what the best VBR is, but since the "default"
> encoding seems to be VBR-6, I think that's a better choice in the
> absence of data.
>

OK, I'll switch to 6 rather than 8.


> Otherwise this LGTM.
>

Thanks!


> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150224/300e9fb6/attachment.html>


More information about the llvm-commits mailing list