Add unreachable instruction to Subzero.

Add unreachable instruction to Subzero.

Also fixes error messages on instruction operands, to print
out the operand (rather than pointer to it), since we can
now print out operands.

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

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

[Karl, 2014-09-16 18:20:05]

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

[Jim Stichnoth, 2014-09-16 20:16:31]

LGTM with a few nits.

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:1427: << "not allowed for values of type " << ThenType;
Did you mean to delete the space before 'not'?

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:1595: StrBuf << "Branch condition " << *Cond << " not
i1";
Print the actual operand type as well as the operand, like all the changes
above?

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:1635: << " in phi instruction";
Print actual operand type?

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:1679: case naclbitc::FUNC_CODE_INST_UNREACHABLE: {
Would you mind explicitly enumerating all the currently unhandled cases?  E.g.:

  case naclbitc::FOO:
  case naclbitc::BAR:
    ErrorMessage(); break;

This would be a big help in seeing how much is left to implement.

https://codereview.chromium.org/577703002/diff/1/tests_lit/reader_tests/unrea...
File tests_lit/reader_tests/unreachable.ll (right):

https://codereview.chromium.org/577703002/diff/1/tests_lit/reader_tests/unrea...
tests_lit/reader_tests/unreachable.ll:18: %div = sdiv i32 %num, %den
If you're testing for exceptional conditions, also check for %num=MININT and
%den=-1.

(just kidding)

[Karl, 2014-09-16 20:35:40]

Committed patchset #2 (id:20001) manually as 9750183 (tree was closed).

[Karl, 2014-09-16 20:35:55]

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:1427: << "not allowed for values of type " << ThenType;
On 2014/09/16 20:16:31, stichnot wrote:
> Did you mean to delete the space before 'not'?

Yes. Added.

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:1595: StrBuf << "Branch condition " << *Cond << " not
i1";
On 2014/09/16 20:16:31, stichnot wrote:
> Print the actual operand type as well as the operand, like all the changes
> above?

Done.

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:1635: << " in phi instruction";
On 2014/09/16 20:16:31, stichnot wrote:
> Print actual operand type?

Done.

https://codereview.chromium.org/577703002/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:1679: case naclbitc::FUNC_CODE_INST_UNREACHABLE: {
On 2014/09/16 20:16:31, stichnot wrote:
> Would you mind explicitly enumerating all the currently unhandled cases? 
E.g.:
> 
>   case naclbitc::FOO:
>   case naclbitc::BAR:
>     ErrorMessage(); break;
> 
> This would be a big help in seeing how much is left to implement.

Done.

https://codereview.chromium.org/577703002/diff/1/tests_lit/reader_tests/unrea...
File tests_lit/reader_tests/unreachable.ll (right):

https://codereview.chromium.org/577703002/diff/1/tests_lit/reader_tests/unrea...
tests_lit/reader_tests/unreachable.ll:18: %div = sdiv i32 %num, %den
On 2014/09/16 20:16:31, stichnot wrote:
> If you're testing for exceptional conditions, also check for %num=MININT and
> %den=-1.
> 
> (just kidding)

Acknowledged.