[llvm-commits] Proposal/patch: Enable bitcode streaming
nlewycky at google.com
Wed Nov 30 14:39:53 PST 2011
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
> 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
> 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
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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits