Remove IceConverter when LLVM IR is not allowed.

Remove IceConverter when LLVM IR is not allowed.

BUG=
R=stichnot@chromium.org

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

[Karl, 2014-10-28 20:23:09]

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

[Jim Stichnoth, 2014-10-28 20:45:35]

https://codereview.chromium.org/689433002/diff/20001/pydir/run-llvm2ice.py
File pydir/run-llvm2ice.py (right):

https://codereview.chromium.org/689433002/diff/20001/pydir/run-llvm2ice.py#ne...
pydir/run-llvm2ice.py:80: cmd += ['--build-on-read']
Maybe --build-on-read=1 for consistency/clarity?

https://codereview.chromium.org/689433002/diff/20001/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/689433002/diff/20001/src/llvm2ice.cpp#newcode275
src/llvm2ice.cpp:275: if (BuildOnRead) {
Could this #if construct be more like this?

if (BuildOnRead) {
  ... Translator ...
} else {
#if ALLOW_LLVM_IR == 1
  ... Converter ...
#else // ALLOW_LLVM_IR != 1
  ... "Error: " ...
#endif // ALLOW_LLVM_IR != 1
}

That way, there is a single #if construct that fits entirely within a nesting
level.  (Also, when there are more than a couple of lines between the #
directives, I prefer to put comments on the #else and #endif.)

[Karl, 2014-10-28 21:43:51]

Also fixed title for issue.

https://codereview.chromium.org/689433002/diff/20001/pydir/run-llvm2ice.py
File pydir/run-llvm2ice.py (right):

https://codereview.chromium.org/689433002/diff/20001/pydir/run-llvm2ice.py#ne...
pydir/run-llvm2ice.py:80: cmd += ['--build-on-read']
On 2014/10/28 20:45:35, stichnot wrote:
> Maybe --build-on-read=1 for consistency/clarity?

Done.

https://codereview.chromium.org/689433002/diff/20001/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/689433002/diff/20001/src/llvm2ice.cpp#newcode275
src/llvm2ice.cpp:275: if (BuildOnRead) {
On 2014/10/28 20:45:35, stichnot wrote:
> Could this #if construct be more like this?
> 
> if (BuildOnRead) {
>   ... Translator ...
> } else {
> #if ALLOW_LLVM_IR == 1
>   ... Converter ...
> #else // ALLOW_LLVM_IR != 1
>   ... "Error: " ...
> #endif // ALLOW_LLVM_IR != 1
> }
> 
> That way, there is a single #if construct that fits entirely within a nesting
> level.  (Also, when there are more than a couple of lines between the #
> directives, I prefer to put comments on the #else and #endif.)

Done.

[Karl, 2014-10-29 19:58:38]

https://codereview.chromium.org/689433002/diff/60001/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/689433002/diff/60001/src/llvm2ice.cpp#newcode278
src/llvm2ice.cpp:278: } else if (ALLOW_LLVM_IR) {
Cleaned this up by not using preprocessor directives.

[Jim Stichnoth, 2014-10-29 21:34:11]

lgtm

https://codereview.chromium.org/689433002/diff/60001/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/689433002/diff/60001/src/llvm2ice.cpp#newcode278
src/llvm2ice.cpp:278: } else if (ALLOW_LLVM_IR) {
On 2014/10/29 19:58:38, Karl wrote:
> Cleaned this up by not using preprocessor directives.

Nice.

[Karl, 2014-10-29 21:58:32]

Committed patchset #5 (id:80001) manually as
ab06df3163683b90ab3cbdba6de9342833892638 (presubmit successful).