Copy Bitwriter to generate PNaClBitwriter.

Copy Bitwriter to generate PNaClBitwriter.

Copy classes for LLVM BitcodeWriter into a PNaCL subdirectory, to create a
PNaCL version. Renames classes/methods to include PNaCl prefix so that they
don't conflict with the LLVM BitcodeWriter. 

Also removed experimental support for use-list order
preservation.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3405

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

[Karl, 2013-04-23 21:37:05]

Fixed version of cl https://codereview.chromium.org/13968008

[Karl, 2013-04-23 21:38:21]

Here is a properly loaded version.

[Mark Seaborn, 2013-04-23 21:44:44]

On 23 April 2013 14:37, <kschimpf@google.com> wrote:

> Reviewers: ,
>
> Message:
> Fixed version of cl https://codereview.chromium.org/13968008
>

FWIW, don't ever delete Rietveld CLs.  You only needed to rerun "git cl
upload" on the existing one.

Comments rescued from previous CL:

https://codereview.chromium.org/13968008/diff/1/include/llvm/Bitcode/PNaCl/PN...
Line is >80 chars

https://codereview.chromium.org/13968008/diff/1/lib/Bitcode/PNaCl/Writer/PNaC...
Line >80 chars

https://codereview.chromium.org/13968008/diff/1/lib/Bitcode/PNaCl/Writer/PNaC...
> 1823 static void WriteModuleUseLists(const Module *M, ValueEnumerator &VE,
Can you explain the reason for the removal of this in the commit message?

https://codereview.chromium.org/13968008/diff/1/tools/pnacl-freeze/Makefile
Missing copyright notice

https://codereview.chromium.org/13968008/diff/1/tools/pnacl-freeze/pnacl-free...
Missing copyright notice

Mark

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

[Karl, 2013-04-24 18:25:59]

Updated file names to use NaCl instead of PNaCL, and responded to Mark's
comments.

https://codereview.chromium.org/14126011/diff/1/include/llvm/Bitcode/PNaCl/PN...
File include/llvm/Bitcode/PNaCl/PNaClBitstreamWriter.h (right):

https://codereview.chromium.org/14126011/diff/1/include/llvm/Bitcode/PNaCl/PN...
include/llvm/Bitcode/PNaCl/PNaClBitstreamWriter.h:1: //===- BitstreamWriter.h -
Low-level bitstream writer interface -*- C++ -*-===//
Fixed name of include file.

https://codereview.chromium.org/14126011/diff/1/include/llvm/Bitcode/PNaCl/PN...
File include/llvm/Bitcode/PNaCl/PNaClReaderWriter.h (right):

https://codereview.chromium.org/14126011/diff/1/include/llvm/Bitcode/PNaCl/PN...
include/llvm/Bitcode/PNaCl/PNaClReaderWriter.h:1: //===--
llvm/Bitcode/PNaCl/PNaClReaderWriter.h - Bitcode reader/writers ----*- C++
-*-===//
Shortened line length.

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/PNaC...
File lib/Bitcode/PNaCl/Writer/PNaClValueEnumerator.h (right):

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/PNaC...
lib/Bitcode/PNaCl/Writer/PNaClValueEnumerator.h:1: //===--
Bitcode/PNaCl/Writer/PNaClValueEnumerator.h - Number values --*- C++ -*-===//
Shortened line length.

https://codereview.chromium.org/14126011/diff/1/tools/pnacl-freeze/pnacl-free...
File tools/pnacl-freeze/pnacl-freeze.cpp (right):

https://codereview.chromium.org/14126011/diff/1/tools/pnacl-freeze/pnacl-free...
tools/pnacl-freeze/pnacl-freeze.cpp:10: //
Cleaned up comments describing command.

[Derek Schuff, 2013-04-25 17:05:18]

lgtm with nits

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/CMak...
File lib/Bitcode/PNaCl/Writer/CMakeLists.txt (right):

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/CMak...
lib/Bitcode/PNaCl/Writer/CMakeLists.txt:4: )
need add_dependencies(PNaClBitWrite intrinsics_gen)
(this is the include/llvm/IR/Intrinsics.gen header)

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/PNaC...
File lib/Bitcode/PNaCl/Writer/PNaClBitcodeWriter.cpp (right):

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/PNaC...
lib/Bitcode/PNaCl/Writer/PNaClBitcodeWriter.cpp:228: PNaClBitstreamWriter
&Stream) {
remove tabs

[Karl, 2013-04-25 17:39:31]

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/CMak...
File lib/Bitcode/PNaCl/Writer/CMakeLists.txt (right):

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/CMak...
lib/Bitcode/PNaCl/Writer/CMakeLists.txt:4: )
On 2013/04/25 17:05:18, Derek Schuff wrote:
> need add_dependencies(PNaClBitWrite intrinsics_gen)
> (this is the include/llvm/IR/Intrinsics.gen header)

Done.

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/PNaC...
File lib/Bitcode/PNaCl/Writer/PNaClBitcodeWriter.cpp (right):

https://codereview.chromium.org/14126011/diff/1/lib/Bitcode/PNaCl/Writer/PNaC...
lib/Bitcode/PNaCl/Writer/PNaClBitcodeWriter.cpp:228: PNaClBitstreamWriter
&Stream) {
On 2013/04/25 17:05:18, Derek Schuff wrote:
> remove tabs

Done.

[jvoung (off chromium), 2013-04-25 17:42:45]

https://codereview.chromium.org/14126011/diff/5001/include/llvm/Bitcode/NaCl/...
File include/llvm/Bitcode/NaCl/NaClReaderWriter.h (right):

https://codereview.chromium.org/14126011/diff/5001/include/llvm/Bitcode/NaCl/...
include/llvm/Bitcode/NaCl/NaClReaderWriter.h:1: //===--
llvm/Bitcode/PNaCl/PNaClReaderWriter.h - ----------------*- C++ -*-===//
Is the header meant to have reader functions too (it's called ReaderWriter)?

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Ma...
File lib/Bitcode/NaCl/Writer/Makefile (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Ma...
lib/Bitcode/NaCl/Writer/Makefile:11: LIBRARYNAME = LLVMPNaCLBitWriter
uncapitalize the L to be consistent? PNaCl

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/CMakeLi...
File tools/pnacl-freeze/CMakeLists.txt (right):

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/CMakeLi...
tools/pnacl-freeze/CMakeLists.txt:1: set(LLVM_LINK_COMPONENTS bitreader analysis
asmparser bitwriter)
Does this end up using the normal (non-pnacl) bitwriter?

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/Makefile
File tools/pnacl-freeze/Makefile (right):

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/Makefil...
tools/pnacl-freeze/Makefile:12: LINK_COMPONENTS := asmparser bitwriter bitreader
analysis
Does this end up using the non-pnacl bitwriter?

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
File tools/pnacl-freeze/pnacl-freeze.cpp (right):

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:16: //      -help         - Output
information about command line switches
Probably don't need to put the commandline usage here, since all the tools have
-help, etc...

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:24: #include "llvm/Bitcode/ReaderWriter.h"
Perhaps add a TODO that it should only use one of the ReaderWriter headers later
(not both the NaCl and non-NaCl?)?

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:45: OutputFilename("o", cl::desc("Specify
output filename"), cl::value_desc("filename"));
80 col

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:53: (OutputFilename.size() == 0 ?
(InputFilename + ".frozen") : OutputFilename);
Does this mean we can't do "pnacl-freeze input.pexe -o - | pnacl-thaw" and
compare the output of the thaw to input.pexe?

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:57: (new
tool_output_file(FrozenFilename.c_str(), ErrorInfo,
is the style supposed to indent the (new...)?

[jvoung (off chromium), 2013-04-25 17:49:06]

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/LLVMBuild...
File lib/Bitcode/NaCl/LLVMBuild.txt (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/LLVMBuild...
lib/Bitcode/NaCl/LLVMBuild.txt:1: ;===- ./lib/Bitcode/PNaCl/LLVMBuild.txt
-----------------------*- Conf -*--===;
PNaCl -> NaCl

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Makefile
File lib/Bitcode/NaCl/Makefile (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Makefile#...
lib/Bitcode/NaCl/Makefile:1: ##===- lib/Bitcode/PNaCl/Makefile
--------------------------*- Makefile -*-===##
PNaCl -> NaCl

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/LL...
File lib/Bitcode/NaCl/Writer/LLVMBuild.txt (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/LL...
lib/Bitcode/NaCl/Writer/LLVMBuild.txt:1: ;===-
./lib/Bitcode/PNaCl/Writer/LLVMBuild.txt ----------------*- Conf -*--===;
PNaCl -> NaCl

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Ma...
File lib/Bitcode/NaCl/Writer/Makefile (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Ma...
lib/Bitcode/NaCl/Writer/Makefile:1: ##===- lib/Bitcode/PNaCl/writer/Makefile
-------------------*- Makefile -*-===##
PNaCl -> NaCl
writer -> Writer

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
File lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp:1: //===---
Bitcode/PNaCl/Writer/PNaClBitcodeWriter.cpp - Bitcode Writer -----===//
PNaCl -> NaCl (from the directory move)

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
File lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h:1: //===--
Bitcode/PNaCl/Writer/PNaClValueEnumerator.h - ----------*- C++ -*-===//
same

[Karl, 2013-04-25 20:48:17]

Changed sources (and class names) to use NaCl (instead of PNaCl), except for
file defining pnacl-freeze.

https://codereview.chromium.org/14126011/diff/5001/include/llvm/Bitcode/NaCl/...
File include/llvm/Bitcode/NaCl/NaClBitstreamWriter.h (right):

https://codereview.chromium.org/14126011/diff/5001/include/llvm/Bitcode/NaCl/...
include/llvm/Bitcode/NaCl/NaClBitstreamWriter.h:1: //===- PNaClBitstreamWriter.h
- PNaCl bitstream writer ----------*- C++ -*-===//
Applied file rename: PNaCl -> NaCl.

https://codereview.chromium.org/14126011/diff/5001/include/llvm/Bitcode/NaCl/...
include/llvm/Bitcode/NaCl/NaClBitstreamWriter.h:25: class PNaClBitstreamWriter {
Renamed to NaClBitstreamWriter to be consistent.

https://codereview.chromium.org/14126011/diff/5001/include/llvm/Bitcode/NaCl/...
File include/llvm/Bitcode/NaCl/NaClReaderWriter.h (right):

https://codereview.chromium.org/14126011/diff/5001/include/llvm/Bitcode/NaCl/...
include/llvm/Bitcode/NaCl/NaClReaderWriter.h:1: //===--
llvm/Bitcode/PNaCl/PNaClReaderWriter.h - ----------------*- C++ -*-===//
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> Is the header meant to have reader functions too (it's called ReaderWriter)?

Yes. However, I haven't added that part yet. I did it this way to mimic the
structure used by LLVM.

The next CL will add the read function(s).

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/LLVMBuild...
File lib/Bitcode/NaCl/LLVMBuild.txt (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/LLVMBuild...
lib/Bitcode/NaCl/LLVMBuild.txt:1: ;===- ./lib/Bitcode/PNaCl/LLVMBuild.txt
-----------------------*- Conf -*--===;
On 2013/04/25 17:49:06, jvoung (cr) wrote:
> PNaCl -> NaCl

Done.

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/LLVMBuild...
lib/Bitcode/NaCl/LLVMBuild.txt:23: name = PNaClBitcode
Also renamed PNaClBitcode to NaClBitcode.

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Makefile
File lib/Bitcode/NaCl/Makefile (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Makefile#...
lib/Bitcode/NaCl/Makefile:1: ##===- lib/Bitcode/PNaCl/Makefile
--------------------------*- Makefile -*-===##
On 2013/04/25 17:49:06, jvoung (cr) wrote:
> PNaCl -> NaCl

Done.

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/LL...
File lib/Bitcode/NaCl/Writer/LLVMBuild.txt (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/LL...
lib/Bitcode/NaCl/Writer/LLVMBuild.txt:1: ;===-
./lib/Bitcode/PNaCl/Writer/LLVMBuild.txt ----------------*- Conf -*--===;
On 2013/04/25 17:49:06, jvoung (cr) wrote:
> PNaCl -> NaCl

Done.

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/LL...
lib/Bitcode/NaCl/Writer/LLVMBuild.txt:20: name = PNaClBitWriter
PNaCl -> NaCl

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/LL...
lib/Bitcode/NaCl/Writer/LLVMBuild.txt:21: parent = PNaClBitcode
PNaCl -> NaCl

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Ma...
File lib/Bitcode/NaCl/Writer/Makefile (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Ma...
lib/Bitcode/NaCl/Writer/Makefile:1: ##===- lib/Bitcode/PNaCl/writer/Makefile
-------------------*- Makefile -*-===##
On 2013/04/25 17:49:06, jvoung (cr) wrote:
> PNaCl -> NaCl
> writer -> Writer

Done.

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Ma...
lib/Bitcode/NaCl/Writer/Makefile:11: LIBRARYNAME = LLVMPNaCLBitWriter
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> uncapitalize the L to be consistent? PNaCl
Changed to LLVMNaClBitWriter to be consistent in naming.

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
File lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp:1: //===---
Bitcode/PNaCl/Writer/PNaClBitcodeWriter.cpp - Bitcode Writer -----===//
On 2013/04/25 17:49:06, jvoung (cr) wrote:
> PNaCl -> NaCl (from the directory move)

Done.

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp:144: unsigned AbbrevToUse,
PNaClBitstreamWriter &Stream) {
File replaced all occurrences: PNaCl -> NaCl

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
File lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp:1: //===-- ValueEnumerator.cpp -
Number values and types for bitcode writer --===//
Fixed this to be NaClValueEnumerator.cpp

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp:10: // This file implements the
PNaClValueEnumerator class.
applied to file: PNaCl -> NaCl

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
File lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h (right):

https://codereview.chromium.org/14126011/diff/5001/lib/Bitcode/NaCl/Writer/Na...
lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h:1: //===--
Bitcode/PNaCl/Writer/PNaClValueEnumerator.h - ----------*- C++ -*-===//
On 2013/04/25 17:49:06, jvoung (cr) wrote:
> same

Replaced all (file) references: PNaCl -> NaCl

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/CMakeLi...
File tools/pnacl-freeze/CMakeLists.txt (right):

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/CMakeLi...
tools/pnacl-freeze/CMakeLists.txt:1: set(LLVM_LINK_COMPONENTS bitreader analysis
asmparser bitwriter)
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> Does this end up using the normal (non-pnacl) bitwriter?

Cleaned this up by removing analysis, asmparser, and bitwriter, and then adding
naclbitwriter.

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/LLVMBui...
File tools/pnacl-freeze/LLVMBuild.txt (right):

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/LLVMBui...
tools/pnacl-freeze/LLVMBuild.txt:16: required_libraries = AsmParser
PNaClBitWriter Analysis BitReader
Updated libraries to only include NaClBitWriter and BitReader.

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/Makefile
File tools/pnacl-freeze/Makefile (right):

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/Makefil...
tools/pnacl-freeze/Makefile:12: LINK_COMPONENTS := asmparser bitwriter bitreader
analysis
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> Does this end up using the non-pnacl bitwriter?

Updated to only include naclbitwriter and bitreader.

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
File tools/pnacl-freeze/pnacl-freeze.cpp (right):

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:16: //      -help         - Output
information about command line switches
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> Probably don't need to put the commandline usage here, since all the tools
have
> -help, etc...

Done.

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:16: //      -help         - Output
information about command line switches
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> Probably don't need to put the commandline usage here, since all the tools
have
> -help, etc...

Done.

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:24: #include "llvm/Bitcode/ReaderWriter.h"
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> Perhaps add a TODO that it should only use one of the ReaderWriter headers
later
> (not both the NaCl and non-NaCl?)?

This can't be done. We need stuff in ReaderWriter for the bitreader, and stuff
in NaClReaderWriter for the naclbitwriter.

Added comments to clarify this.

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:45: OutputFilename("o", cl::desc("Specify
output filename"), cl::value_desc("filename"));
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> 80 col

Done.

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:53: (OutputFilename.size() == 0 ?
(InputFilename + ".frozen") : OutputFilename);
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> Does this mean we can't do "pnacl-freeze input.pexe -o - | pnacl-thaw" and
> compare the output of the thaw to input.pexe?

Note that pnacl-thaw is only for command line usage. I expect pnacl-translate to
be modified to use the NaCl Bitcode Reader so that it can directly thaw the wire
format file as it is read.

Part of the problem is to figure out how pnacl-freeze fits into the build
stream. If we are using it as a post-step, it would overwrite input.pexe.

It is true that at the moment, I only allow the API to update a file, and hence
"-o -" is not possible.

I would add your option, but I don't know how to convert stdout into a
'raw_fd_ostream::F_Binary' stream (see Out below). Hence, I don't know how to
allow this.

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:57: (new
tool_output_file(FrozenFilename.c_str(), ErrorInfo,
On 2013/04/25 17:42:45, jvoung (cr) wrote:
> is the style supposed to indent the (new...)?

I followed the style from llvm-as (i.e. I copied llvm-as.cpp and edited it).
However, I don't see any problem with indenting, so I'll indent it.

[jvoung (off chromium), 2013-04-25 22:25:36]

tabs =)
otherwise lgtm

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/CMakeLi...
File tools/pnacl-freeze/CMakeLists.txt (right):

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/CMakeLi...
tools/pnacl-freeze/CMakeLists.txt:1: set(LLVM_LINK_COMPONENTS bitreader analysis
asmparser bitwriter)
On 2013/04/25 20:48:17, Karl wrote:
> On 2013/04/25 17:42:45, jvoung (cr) wrote:
> > Does this end up using the normal (non-pnacl) bitwriter?
> 
> Cleaned this up by removing analysis, asmparser, and bitwriter, and then
adding
> naclbitwriter.

Thanks!

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
File tools/pnacl-freeze/pnacl-freeze.cpp (right):

https://codereview.chromium.org/14126011/diff/5001/tools/pnacl-freeze/pnacl-f...
tools/pnacl-freeze/pnacl-freeze.cpp:53: (OutputFilename.size() == 0 ?
(InputFilename + ".frozen") : OutputFilename);
On 2013/04/25 20:48:17, Karl wrote:
> On 2013/04/25 17:42:45, jvoung (cr) wrote:
> > Does this mean we can't do "pnacl-freeze input.pexe -o - | pnacl-thaw" and
> > compare the output of the thaw to input.pexe?
> 
> Note that pnacl-thaw is only for command line usage. I expect pnacl-translate
to
> be modified to use the NaCl Bitcode Reader so that it can directly thaw the
wire
> format file as it is read.
> 
> Part of the problem is to figure out how pnacl-freeze fits into the build
> stream. If we are using it as a post-step, it would overwrite input.pexe.
> 
> It is true that at the moment, I only allow the API to update a file, and
hence
> "-o -" is not possible.
> 
> I would add your option, but I don't know how to convert stdout into a
> 'raw_fd_ostream::F_Binary' stream (see Out below). Hence, I don't know how to
> allow this.

Ok, just wondering if handling that like the other tools would help with
testing.

https://codereview.chromium.org/14126011/diff/23001/tools/pnacl-freeze/pnacl-...
File tools/pnacl-freeze/pnacl-freeze.cpp (right):

https://codereview.chromium.org/14126011/diff/23001/tools/pnacl-freeze/pnacl-...
tools/pnacl-freeze/pnacl-freeze.cpp:43: cl::value_desc("filename"));
tabs -> spaces?

https://codereview.chromium.org/14126011/diff/23001/tools/pnacl-freeze/pnacl-...
tools/pnacl-freeze/pnacl-freeze.cpp:56: raw_fd_ostream::F_Binary));
tabs -> spaces?

[Karl, 2013-04-26 19:54:27]

Committed patchset #4 manually as r5712db9 (presubmit successful).