[llvm-commits] Proposal/patch: Enable bitcode streaming

[Nick Lewycky]

On 28 November 2011 12:42, Derek Schuff <dschuff at google.com> wrote:

> thanks for the feedback, I'm fixing all the suggestions, but I do have one
> question:
>
> On Mon, Nov 21, 2011 at 3:38 PM, Nick Lewycky <nlewycky at google.com> wrote:
>
>> This is in a header file. Please move the implementation of the virtual
>> destructor to BitcodeReader.cpp, or else copies will end up in every .o
>> file that includes this header.
>>
>> Same with MemoryBitstreamBytes and LazyBitstreamBytes. More general
>> question, any reason to put MemoryBitstreamBytes and LazyBitstreamBytes
>> implementations inside the header? You could create
>> lib/Bitcode/Reader/BitstreamReader.cpp.
>>
>>
> The BitstreamBytes implementations are in the header file because
> BitstreamReader/BitstreamCursor is implemented entirely in the header
> file. Presumably BitstreamReader is in the header file so it can be
> inlined. Presumably this improves performance (especially in llc where
> there the usage is only in one file), although there seem to be several
> uses of BitstreamCursor in clang, so it may contribute to more code size
> bloat there. If you want I can move everything to BitstreamReader.cpp
> (perhaps LTO could still buy us back any performance loss from inlining?)
>

That's a great point. Given the other code in the bitcode reader, I think
it's fine to leave the implementations in the header -- except for the
virtual destructors.

It would also be possible to move just the BitstreamBytes implementations
> into a cpp file using the common idiom of having static factory functions
> with an anonymous namespace rather than just calling "new" directly like we
> do right now.
>

I don't have a sense that either way would be particularly better.

 >One other thing. The behaviour when LazyBitcode is true could be really
> different from when it off on the same .bc file on >disk. It'd be good to
> add comments pointing out why (module-level assembly comes to mind,
> anything else?).
>
> Sure. where would be a good place to put it?
>

I was thinking of comments above the option in llc.

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111130/318ff5f1/attachment.html>