Copy LLVM bitcode reader to generate a PNaCl wire format reader.

Copy LLVM bitcode reader to generate a PNaCl wire format reader.

Copy classes for LLVM BitcodeReader into a NaCl subdirectory, to create a wire
format version. Renames classes/functions to include NaCl prefix, so that
they don't conflict with the LLVM Bitcode reader.

Also implements pnacl-thaw, showing that we can read the PNaCl wire format
files.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3405
R=jvoung@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-llvm.git;a=commit;h=f42b26d

[Karl, 2013-04-26 22:06:02]

Please review this CL. Thanks.

[jvoung (off chromium), 2013-04-29 18:02:08]

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
File include/llvm/Bitcode/NaCl/NaClBitstreamReader.h (right):

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
include/llvm/Bitcode/NaCl/NaClBitstreamReader.h:131: /// BitstreamEntry - When
advancing through a bitstream cursor, each advance can
Should this top-level struct be made NaClBitstreamEntry?

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
File include/llvm/Bitcode/NaCl/NaClReaderWriter.h (right):

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
include/llvm/Bitcode/NaCl/NaClReaderWriter.h:105: ///   ... potentially other
gunk ...
Did we put "other gunk" in the wrapper header, to *really* tell the difference
between LLVM bitcode and NaCl files that are frozen?

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/CMake...
File lib/Bitcode/NaCl/Reader/CMakeLists.txt (right):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/CMake...
lib/Bitcode/NaCl/Reader/CMakeLists.txt:1: add_llvm_library(NaClBitReader
prefix with LLVM ?

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp (left):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:3130: if (R->ParseTriple(Triple))
Any reason this is removed?

The NaClBitcodeReader's ParseTriple/ParseModuleTriple methods still exist.  Are
they used by anything else?  Looks like no, only getBitcodeTargetTriple is used
by libLTO.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp (right):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:465: BitstreamEntry Entry =
Stream.advanceSkippingSubblocks();
See question in header file about making it NaClBitstreamEntry or not.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:3056: LLVMContext& Context,
tabs

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:3080: DataStreamer *streamer,
tabs!

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:3104: std::string *ErrMsg){
tabs

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h (right):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h:20: #include
"llvm/Bitcode/LLVMBitCodes.h"
Are we keeping the original LLVMBitCodes.h enum?

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
File lib/Bitcode/NaCl/Reader/NaClBitstreamReader.cpp (right):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitstreamReader.cpp:8: //
Should the merge script in native_client/pnacl/scripts/merge-tool-git.sh have
some notice about guidelines for merging changes in the upstream
BitstreamReader.cpp?

Or, will this file change quite a bit in following CLs?  Sounded like you wanted
to replace Abbrevs, Records, etc.?

I think the changes since 3.0 to this low-level interface hasn't been about the
format, but more performance related (e.g., enabling streaming compilation). 
After 4.0, there might be more drastic changes that we wouldn't want to merge
though.

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.cpp
File tools/pnacl-thaw/pnacl-thaw.cpp (left):

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.c...
tools/pnacl-thaw/pnacl-thaw.cpp:22: #include "llvm/DebugInfo.h"
can you apply similar cleanup to pnacl-freeze?

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.cpp
File tools/pnacl-thaw/pnacl-thaw.cpp (right):

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.c...
tools/pnacl-thaw/pnacl-thaw.cpp:10: // Converts NaCl pexe wire format back to
LLVM pexe.
nit: "pexe" seems like a NaCl thing, so perhaps just call it "LLVM bitcode"
instead of "LLVM pexe" as you do in the help message below.

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.c...
tools/pnacl-thaw/pnacl-thaw.cpp:15: #include "llvm/IR/LLVMContext.h"
include the context header only once?

[Derek Schuff, 2013-04-29 18:25:38]

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
File include/llvm/Bitcode/NaCl/NaClReaderWriter.h (right):

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
include/llvm/Bitcode/NaCl/NaClReaderWriter.h:105: ///   ... potentially other
gunk ...
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> Did we put "other gunk" in the wrapper header, to *really* tell the difference
> between LLVM bitcode and NaCl files that are frozen?
we probably should. althoughwe might want to just not use the LLVM magic numbers
and wrapper format at all.

[Karl, 2013-04-29 20:44:37]

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
File include/llvm/Bitcode/NaCl/NaClBitstreamReader.h (right):

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
include/llvm/Bitcode/NaCl/NaClBitstreamReader.h:131: /// BitstreamEntry - When
advancing through a bitstream cursor, each advance can
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> Should this top-level struct be made NaClBitstreamEntry?

Good point. Renaming.

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
File include/llvm/Bitcode/NaCl/NaClReaderWriter.h (right):

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
include/llvm/Bitcode/NaCl/NaClReaderWriter.h:105: ///   ... potentially other
gunk ...
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> Did we put "other gunk" in the wrapper header, to *really* tell the difference
> between LLVM bitcode and NaCl files that are frozen?

Not yet. Added TODO to clarify we need to do this.

https://codereview.chromium.org/14314016/diff/1/include/llvm/Bitcode/NaCl/NaC...
include/llvm/Bitcode/NaCl/NaClReaderWriter.h:105: ///   ... potentially other
gunk ...
On 2013/04/29 18:25:38, Derek Schuff wrote:
> On 2013/04/29 18:02:08, jvoung (cr) wrote:
> > Did we put "other gunk" in the wrapper header, to *really* tell the
difference
> > between LLVM bitcode and NaCl files that are frozen?
> we probably should. althoughwe might want to just not use the LLVM magic
numbers
> and wrapper format at all.

Added TODO for this as well.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/CMake...
File lib/Bitcode/NaCl/Reader/CMakeLists.txt (right):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/CMake...
lib/Bitcode/NaCl/Reader/CMakeLists.txt:1: add_llvm_library(NaClBitReader
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> prefix with LLVM ?

Done.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp (left):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:3130: if (R->ParseTriple(Triple))
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> Any reason this is removed?
> 
> The NaClBitcodeReader's ParseTriple/ParseModuleTriple methods still exist. 
Are
> they used by anything else?  Looks like no, only getBitcodeTargetTriple is
used
> by libLTO.

As far as I can tell, only lto uses it, so thought it wasn't necessary. Hence, I
removed it.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp (right):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:465: BitstreamEntry Entry =
Stream.advanceSkippingSubblocks();
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> See question in header file about making it NaClBitstreamEntry or not.

Done.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:3056: LLVMContext& Context,
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> tabs

Done.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:3080: DataStreamer *streamer,
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> tabs!

Done.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:3104: std::string *ErrMsg){
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> tabs

Done.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h (right):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h:20: #include
"llvm/Bitcode/LLVMBitCodes.h"
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> Are we keeping the original LLVMBitCodes.h enum?

Good point. Adding separate copy and renaming appropriately so that we can
modify as needed.

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
File lib/Bitcode/NaCl/Reader/NaClBitstreamReader.cpp (right):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitstreamReader.cpp:8: //
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> Should the merge script in native_client/pnacl/scripts/merge-tool-git.sh have
> some notice about guidelines for merging changes in the upstream
> BitstreamReader.cpp?
> 
> Or, will this file change quite a bit in following CLs?  Sounded like you
wanted
> to replace Abbrevs, Records, etc.?
> 
> I think the changes since 3.0 to this low-level interface hasn't been about
the
> format, but more performance related (e.g., enabling streaming compilation). 
> After 4.0, there might be more drastic changes that we wouldn't want to merge
> though.
> 

The problem (as I see it) is that the way I want to approach abbreviations and
records will break bc-analyzer. As such, my guess is that a lot of the changes
will not be upstreamable.

I agree that some performance related changes can be upstreamed (See CL on
types).

Also, the goal of forking is to make sure that our ABI doesn't change under
upstream changes.

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.cpp
File tools/pnacl-thaw/pnacl-thaw.cpp (left):

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.c...
tools/pnacl-thaw/pnacl-thaw.cpp:22: #include "llvm/DebugInfo.h"
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> can you apply similar cleanup to pnacl-freeze?

Done.

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.cpp
File tools/pnacl-thaw/pnacl-thaw.cpp (right):

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.c...
tools/pnacl-thaw/pnacl-thaw.cpp:10: // Converts NaCl pexe wire format back to
LLVM pexe.
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> nit: "pexe" seems like a NaCl thing, so perhaps just call it "LLVM bitcode"
> instead of "LLVM pexe" as you do in the help message below.

Done.

https://codereview.chromium.org/14314016/diff/1/tools/pnacl-thaw/pnacl-thaw.c...
tools/pnacl-thaw/pnacl-thaw.cpp:15: #include "llvm/IR/LLVMContext.h"
On 2013/04/29 18:02:08, jvoung (cr) wrote:
> include the context header only once?

Done.

[jvoung (off chromium), 2013-04-29 21:49:34]

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp (left):

https://codereview.chromium.org/14314016/diff/1/lib/Bitcode/NaCl/Reader/NaClB...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:3130: if (R->ParseTriple(Triple))
On 2013/04/29 20:44:37, Karl wrote:
> On 2013/04/29 18:02:08, jvoung (cr) wrote:
> > Any reason this is removed?
> > 
> > The NaClBitcodeReader's ParseTriple/ParseModuleTriple methods still exist. 
> Are
> > they used by anything else?  Looks like no, only getBitcodeTargetTriple is
> used
> > by libLTO.
> 
> As far as I can tell, only lto uses it, so thought it wasn't necessary. Hence,
I
> removed it.

Would have been a bit cleaner to do that separately, since you could also remove
ParseTriple and ParseModuleTriple?  Then again, as long as the writer writes out
a triple, it seems good to be able to read it back...

https://codereview.chromium.org/14314016/diff/10001/include/llvm/Bitcode/NaCl...
File include/llvm/Bitcode/NaCl/NaClBitCodes.h (right):

https://codereview.chromium.org/14314016/diff/10001/include/llvm/Bitcode/NaCl...
include/llvm/Bitcode/NaCl/NaClBitCodes.h:79: enum NaCLAttributeCodes {
NaCL -> NaCl

https://codereview.chromium.org/14314016/diff/10001/include/llvm/Bitcode/NaCl...
File include/llvm/Bitcode/NaCl/NaClBitstreamReader.h (right):

https://codereview.chromium.org/14314016/diff/10001/include/llvm/Bitcode/NaCl...
include/llvm/Bitcode/NaCl/NaClBitstreamReader.h:31: /// NaClBitstreamReader -
This class is used to read from an NaCl
an NaCl -> a NaCl

https://codereview.chromium.org/14314016/diff/10001/lib/Bitcode/NaCl/Reader/N...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp (right):

https://codereview.chromium.org/14314016/diff/10001/lib/Bitcode/NaCl/Reader/N...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:1528: case
bitc::BLOCKINFO_BLOCK_ID:
bitc:: ?

Did some header still end up including the original bitc:: enums?

Oh n/m this is part of the "core" enum, which hasn't been duplicated?

https://codereview.chromium.org/14314016/diff/10001/lib/Bitcode/NaCl/Reader/N...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:1828: case
bitc::BLOCKINFO_BLOCK_ID:
bitc:: same question about why bitc:: here.

[Karl, 2013-05-01 16:23:23]

https://chromiumcodereview.appspot.com/14314016/diff/10001/include/llvm/Bitco...
File include/llvm/Bitcode/NaCl/NaClBitCodes.h (right):

https://chromiumcodereview.appspot.com/14314016/diff/10001/include/llvm/Bitco...
include/llvm/Bitcode/NaCl/NaClBitCodes.h:79: enum NaCLAttributeCodes {
On 2013/04/29 21:49:34, jvoung (cr) wrote:
> NaCL -> NaCl

Done.

https://chromiumcodereview.appspot.com/14314016/diff/10001/include/llvm/Bitco...
File include/llvm/Bitcode/NaCl/NaClBitstreamReader.h (right):

https://chromiumcodereview.appspot.com/14314016/diff/10001/include/llvm/Bitco...
include/llvm/Bitcode/NaCl/NaClBitstreamReader.h:31: /// NaClBitstreamReader -
This class is used to read from an NaCl
On 2013/04/29 21:49:34, jvoung (cr) wrote:
> an NaCl -> a NaCl

Done.

https://chromiumcodereview.appspot.com/14314016/diff/10001/lib/Bitcode/NaCl/R...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp (right):

https://chromiumcodereview.appspot.com/14314016/diff/10001/lib/Bitcode/NaCl/R...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:1528: case
bitc::BLOCKINFO_BLOCK_ID:
On 2013/04/29 21:49:34, jvoung (cr) wrote:
> bitc:: ?
> 
> Did some header still end up including the original bitc:: enums?
> 
> Oh n/m this is part of the "core" enum, which hasn't been duplicated?

Added TODO (in NaClLLVMBitCodes.h) to move into NaCl, so that all enums are in
naclbitc (so they can't be confused with enum names in bitc), and abbreviations
can be generalized for the PNaCl wire format.

https://chromiumcodereview.appspot.com/14314016/diff/10001/lib/Bitcode/NaCl/R...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:1828: case
bitc::BLOCKINFO_BLOCK_ID:
On 2013/04/29 21:49:34, jvoung (cr) wrote:
> bitc:: same question about why bitc:: here.

See previous comment.

https://chromiumcodereview.appspot.com/14314016/diff/25001/include/llvm/Bitco...
File include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h (right):

https://chromiumcodereview.appspot.com/14314016/diff/25001/include/llvm/Bitco...
include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h:24: #include
"llvm/Bitcode/BitCodes.h"
Added TODO here to move BitCodes.h into NaCl subdirectory.

[jvoung (off chromium), 2013-05-01 17:33:36]

lgtm

https://codereview.chromium.org/14314016/diff/25001/include/llvm/Bitcode/NaCl...
File include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h (right):

https://codereview.chromium.org/14314016/diff/25001/include/llvm/Bitcode/NaCl...
include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h:1: //===- NaClBitCodes.h
-------------------------------------------*- C++ -*-===//
NaClLLVMBitCodes.h

https://codereview.chromium.org/14314016/diff/25001/include/llvm/Bitcode/NaCl...
include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h:19: #ifndef
LLVM_BITCODE_NACL_NACLBITCODES_H
NACLLLVMBITCODES_H

[Karl, 2013-05-01 17:42:42]

Committed patchset #5 manually as rf42b26d (presubmit successful).

[Karl, 2013-05-01 17:43:06]

https://chromiumcodereview.appspot.com/14314016/diff/25001/include/llvm/Bitco...
File include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h (right):

https://chromiumcodereview.appspot.com/14314016/diff/25001/include/llvm/Bitco...
include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h:1: //===- NaClBitCodes.h
-------------------------------------------*- C++ -*-===//
On 2013/05/01 17:33:36, jvoung (cr) wrote:
> NaClLLVMBitCodes.h

Done.

https://chromiumcodereview.appspot.com/14314016/diff/25001/include/llvm/Bitco...
include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h:19: #ifndef
LLVM_BITCODE_NACL_NACLBITCODES_H
On 2013/05/01 17:33:36, jvoung (cr) wrote:
> NACLLLVMBITCODES_H

Done.

[jvoung (off chromium), 2013-05-01 17:48:09]

https://chromiumcodereview.appspot.com/14314016/diff/25001/include/llvm/Bitco...
File include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h (right):

https://chromiumcodereview.appspot.com/14314016/diff/25001/include/llvm/Bitco...
include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h:19: #ifndef
LLVM_BITCODE_NACL_NACLBITCODES_H
On 2013/05/01 17:43:06, Karl wrote:
> On 2013/05/01 17:33:36, jvoung (cr) wrote:
> > NACLLLVMBITCODES_H
> 
> Done.

I don't see this part of the change in patchset 5?

[Karl, 2013-05-01 17:57:35]

Don't know what I did wrong. I thought it was added. Will create a CL to fix.