Modify pnacl subzero to be able to read pnacl bitcode files.

Modify pnacl subzero to be able to read pnacl bitcode files.

BUG=None

[Karl, 2014-05-09 20:28:17]

PTAL. Thanks.

[Jim Stichnoth, 2014-05-09 23:02:42]

https://codereview.chromium.org/277033003/diff/1/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/277033003/diff/1/src/llvm2ice.cpp#newcode597
src/llvm2ice.cpp:597: cl::opt<NaClFileFormat>
Make this static for consistency with other cl::opt definitions.  (Though
actually, all the statics should probably be moved into an anonymous namespace.)

https://codereview.chromium.org/277033003/diff/1/src/llvm2ice.cpp#newcode616
src/llvm2ice.cpp:616: Mod = NaClParseIRFile(IRFilename, InputFileFormat, Err,
getGlobalContext());
Would it be possible to add a -bitcode-format=pnacl lit test?

[Karl, 2014-05-14 19:31:26]

https://codereview.chromium.org/277033003/diff/1/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/277033003/diff/1/src/llvm2ice.cpp#newcode597
src/llvm2ice.cpp:597: cl::opt<NaClFileFormat>
On 2014/05/09 23:02:42, stichnot wrote:
> Make this static for consistency with other cl::opt definitions.  (Though
> actually, all the statics should probably be moved into an anonymous
namespace.)

Done.

https://codereview.chromium.org/277033003/diff/1/src/llvm2ice.cpp#newcode616
src/llvm2ice.cpp:616: Mod = NaClParseIRFile(IRFilename, InputFileFormat, Err,
getGlobalContext());
On 2014/05/09 23:02:42, stichnot wrote:
> Would it be possible to add a -bitcode-format=pnacl lit test?

Done.

[Jim Stichnoth, 2014-05-14 21:45:18]

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py
File llvm2iceinsts.py (right):

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode7
llvm2iceinsts.py:7: import re
Sort imports alphabetically.  (I know some or all files already had this
mistake...)

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode17
llvm2iceinsts.py:17: """Runs commands"""
More descriptive description.

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode18
llvm2iceinsts.py:18: desc = 'Run llvm2ice on llvm file to produce ice
instructions.'
I've been trying to use all-capitalized ICE in textual descriptions.  (Never
mind that ICE doesn't actually stand for anything, yet...)

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode26
llvm2iceinsts.py:26: '(for to building PNaCl files)')
for to --> for

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode33
llvm2iceinsts.py:33: help='Trace command that generates ice instructions')
ice --> ICE

https://codereview.chromium.org/277033003/diff/20001/szdiff.py
File szdiff.py (right):

https://codereview.chromium.org/277033003/diff/20001/szdiff.py#newcode66
szdiff.py:66: ' bitcast .*\* .* to .*\*' # bitcast pointer types
Align comments

[Karl, 2014-05-14 21:56:56]

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py
File llvm2iceinsts.py (right):

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode7
llvm2iceinsts.py:7: import re
On 2014/05/14 21:45:18, stichnot wrote:
> Sort imports alphabetically.  (I know some or all files already had this
> mistake...)

Done.

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode17
llvm2iceinsts.py:17: """Runs commands"""
On 2014/05/14 21:45:18, stichnot wrote:
> More descriptive description.

Done.

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode18
llvm2iceinsts.py:18: desc = 'Run llvm2ice on llvm file to produce ice
instructions.'
On 2014/05/14 21:45:18, stichnot wrote:
> I've been trying to use all-capitalized ICE in textual descriptions.  (Never
> mind that ICE doesn't actually stand for anything, yet...)

Done.

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode26
llvm2iceinsts.py:26: '(for to building PNaCl files)')
On 2014/05/14 21:45:18, stichnot wrote:
> for to --> for

Done.

https://codereview.chromium.org/277033003/diff/20001/llvm2iceinsts.py#newcode33
llvm2iceinsts.py:33: help='Trace command that generates ice instructions')
On 2014/05/14 21:45:18, stichnot wrote:
> ice --> ICE

Done.

https://codereview.chromium.org/277033003/diff/20001/szdiff.py
File szdiff.py (right):

https://codereview.chromium.org/277033003/diff/20001/szdiff.py#newcode66
szdiff.py:66: ' bitcast .*\* .* to .*\*' # bitcast pointer types
On 2014/05/14 21:45:18, stichnot wrote:
> Align comments

Done.

[Jim Stichnoth, 2014-05-14 22:51:22]

LGTM (though I could be missing pythonic comments...)

[JF, 2014-05-19 18:10:25]

lgtm besides a few style things.

https://codereview.chromium.org/277033003/diff/40001/llvm2iceinsts.py
File llvm2iceinsts.py (right):

https://codereview.chromium.org/277033003/diff/40001/llvm2iceinsts.py#newcode13
llvm2iceinsts.py:13: break
Shouldn't that be in main?

https://codereview.chromium.org/277033003/diff/40001/llvm2iceinsts.py#newcode28
llvm2iceinsts.py:28: generate the corresponding ICE instructions.
I think this docstring would be better as --help content for argparse (I'd drop
the docstring entirely).

https://codereview.chromium.org/277033003/diff/40001/llvm2iceinsts.py#newcode34
llvm2iceinsts.py:34: help='Path to llvm2ice driver program [default
./llvm2ice]')
I think you should use this for default instead of specifying it manually:
https://docs.python.org/dev/library/argparse.html#argparse.ArgumentDefaultsHe...

https://codereview.chromium.org/277033003/diff/40001/szdiff.py
File szdiff.py (right):

https://codereview.chromium.org/277033003/diff/40001/szdiff.py#newcode10
szdiff.py:10: Before comparing, the llvm file is massaged to remove comments,
You should capitalize "LLVM" here and below.

[Karl, 2014-05-19 21:34:39]

https://codereview.chromium.org/277033003/diff/40001/llvm2iceinsts.py
File llvm2iceinsts.py (right):

https://codereview.chromium.org/277033003/diff/40001/llvm2iceinsts.py#newcode13
llvm2iceinsts.py:13: break
On 2014/05/19 18:10:25, JF wrote:
> Shouldn't that be in main?

Can't be. It is used to find module utils.

https://codereview.chromium.org/277033003/diff/40001/llvm2iceinsts.py#newcode28
llvm2iceinsts.py:28: generate the corresponding ICE instructions.
On 2014/05/19 18:10:25, JF wrote:
> I think this docstring would be better as --help content for argparse (I'd
drop
> the docstring entirely).

Added as epilog.

https://codereview.chromium.org/277033003/diff/40001/llvm2iceinsts.py#newcode34
llvm2iceinsts.py:34: help='Path to llvm2ice driver program [default
./llvm2ice]')
On 2014/05/19 18:10:25, JF wrote:
> I think you should use this for default instead of specifying it manually:
>
https://docs.python.org/dev/library/argparse.html#argparse.ArgumentDefaultsHe...

Done.

[Karl, 2014-05-19 21:59:29]

Committed.