Modify how textual bitcode is injected into pnacl-sz.

Modify how textual bitcode is injected into pnacl-sz.

Added command line flag "--bitcode-as-text", and triggered
the acceptance of textual bitcode on this flag. To make sure
this isn't added to the sandboxed translator, the reading of
bitcode text is also dependent on the translator not being a
minimal build.

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

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

[Karl, 2015-07-07 19:11:27]

kschimpf@google.com changed reviewers:
+ dschuff@chromium.org, jvoung@chromium.org, stichnot@chromium.org

[jvoung (off chromium), 2015-07-07 21:40:17]

lgtm

https://codereview.chromium.org/1215463014/diff/1/src/IceCompileServer.cpp
File src/IceCompileServer.cpp (right):

https://codereview.chromium.org/1215463014/diff/1/src/IceCompileServer.cpp#ne...
src/IceCompileServer.cpp:44: "browswer translator");
browswer -> browser

https://codereview.chromium.org/1215463014/diff/1/src/IceCompileServer.cpp#ne...
src/IceCompileServer.cpp:67: ErrStrm << EC.message(); // << "\n";
why comment out the "// << "\n";"?, and the // ErrStrm.flush(); below

[Jim Stichnoth, 2015-07-10 18:04:24]

lgtm

https://codereview.chromium.org/1215463014/diff/1/src/IceBuildDefs.h
File src/IceBuildDefs.h (right):

https://codereview.chromium.org/1215463014/diff/1/src/IceBuildDefs.h#newcode28
src/IceBuildDefs.h:28: constexpr bool textualBitcode() { return
ACCEPT_TEXTUAL_PNACL_BITCODE; }
Consider prefixing with ALLOW_ instead of ACCEPT_ for consistency?

[Karl, 2015-07-21 21:07:02]

https://codereview.chromium.org/1215463014/diff/1/CMakeLists.txt
File CMakeLists.txt (right):

https://codereview.chromium.org/1215463014/diff/1/CMakeLists.txt#newcode22
CMakeLists.txt:22: if(PNACL_BROWSER_TRANSLATOR)
Fixed this to match what is written in Makefile.

https://codereview.chromium.org/1215463014/diff/1/Makefile.standalone
File Makefile.standalone (right):

https://codereview.chromium.org/1215463014/diff/1/Makefile.standalone#newcode89
Makefile.standalone:89: ifdef TEXTUAL_BITCODE
Removed this. No longer a build flag (its now a CL flag).

https://codereview.chromium.org/1215463014/diff/1/Makefile.standalone#newcode238
Makefile.standalone:238: ifdef TEXTUAL_BITCODE
Simplified this. TEXTUAL_BITCODE is no longer a build option, only a CL flag.

https://codereview.chromium.org/1215463014/diff/1/src/IceBuildDefs.h
File src/IceBuildDefs.h (right):

https://codereview.chromium.org/1215463014/diff/1/src/IceBuildDefs.h#newcode28
src/IceBuildDefs.h:28: constexpr bool textualBitcode() { return
ACCEPT_TEXTUAL_PNACL_BITCODE; }
On 2015/07/10 18:04:23, stichnot wrote:
> Consider prefixing with ALLOW_ instead of ACCEPT_ for consistency?

Removed. Using command line argument to control whether we allow textual
bitcode.

https://codereview.chromium.org/1215463014/diff/1/src/IceCompileServer.cpp
File src/IceCompileServer.cpp (right):

https://codereview.chromium.org/1215463014/diff/1/src/IceCompileServer.cpp#ne...
src/IceCompileServer.cpp:24: // Include code to handle converting textual
bitcode records to binary (for
Removed comment, no longer dependent on ACCEPT_TEXTUAL_PNACL_BITCODE.

https://codereview.chromium.org/1215463014/diff/1/src/IceCompileServer.cpp#ne...
src/IceCompileServer.cpp:44: "browswer translator");
On 2015/07/07 21:40:17, jvoung wrote:
> browswer -> browser

Removed static_assert since it is no longer dependent on a build flag (just a CL
flag).

https://codereview.chromium.org/1215463014/diff/1/src/IceCompileServer.cpp#ne...
src/IceCompileServer.cpp:67: ErrStrm << EC.message(); // << "\n";
On 2015/07/07 21:40:17, jvoung wrote:
> why comment out the "// << "\n";"?, and the // ErrStrm.flush(); below

I removed the '<< "\n"' because the caller expects it to be an error string
passed to llvm::SMDiagnostic, which assumes the string isn't terminated with a
newline (it adds it when printing the string message).

I'm not quite sure why the flush is commented out. Fixing.

[jvoung (off chromium), 2015-07-22 21:20:45]

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlags.cpp
File src/IceClFlags.cpp (left):

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlags.cpp#oldc...
src/IceClFlags.cpp:18: 
keep the newline? I think the new style is Foo.cpp has Foo.h first, on its own
line, then other .h after.

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlags.cpp
File src/IceClFlags.cpp (right):

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlags.cpp#newc...
src/IceClFlags.cpp:453: OutFlagsExtra.setBitcodeAsText(BitcodeAsText);
Can the bools portion be sorted?

I think it's separated by bool flags and string flags.

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlagsExtra.h
File src/IceClFlagsExtra.h (right):

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlagsExtra.h#n...
src/IceClFlagsExtra.h:73: bool BitcodeAsText = false;
sort?

https://codereview.chromium.org/1215463014/diff/40001/src/IceCompileServer.cpp
File src/IceCompileServer.cpp (right):

https://codereview.chromium.org/1215463014/diff/40001/src/IceCompileServer.cp...
src/IceCompileServer.cpp:149: llvm::report_fatal_error("Can't specify
'bitcode-as-text' flag in "
Hopefully the optimizer is sufficiently smart to coordinate this if and the " ?
: "

[Karl, 2015-07-22 21:58:17]

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlags.cpp
File src/IceClFlags.cpp (left):

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlags.cpp#oldc...
src/IceClFlags.cpp:18: 
On 2015/07/22 21:20:45, jvoung wrote:
> keep the newline? I think the new style is Foo.cpp has Foo.h first, on its own
> line, then other .h after.

Done.

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlags.cpp
File src/IceClFlags.cpp (right):

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlags.cpp#newc...
src/IceClFlags.cpp:453: OutFlagsExtra.setBitcodeAsText(BitcodeAsText);
On 2015/07/22 21:20:44, jvoung wrote:
> Can the bools portion be sorted?
> 
> I think it's separated by bool flags and string flags.

Done.

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlagsExtra.h
File src/IceClFlagsExtra.h (right):

https://codereview.chromium.org/1215463014/diff/40001/src/IceClFlagsExtra.h#n...
src/IceClFlagsExtra.h:73: bool BitcodeAsText = false;
On 2015/07/22 21:20:45, jvoung wrote:
> sort?

Done.

https://codereview.chromium.org/1215463014/diff/40001/src/IceCompileServer.cpp
File src/IceCompileServer.cpp (right):

https://codereview.chromium.org/1215463014/diff/40001/src/IceCompileServer.cp...
src/IceCompileServer.cpp:149: llvm::report_fatal_error("Can't specify
'bitcode-as-text' flag in "
On 2015/07/22 21:20:45, jvoung wrote:
> Hopefully the optimizer is sufficiently smart to coordinate this if and the "
?
> : "

I early had !BuildDefs::minimal() in the "?:", but then removed it. I decided to
put it back. This should make the job of the compiler to omit it easier.

[jvoung (off chromium), 2015-07-22 22:30:06]

lgtm

[Karl, 2015-07-23 16:10:09]

Committed patchset #4 (id:60001) manually as
cb6e95aac252a35c66705fb5e30d3c5e9ba4c1dc (presubmit successful).