Add cast instructions to subzero's pnacl bitcode translator.

Add cast instructions to subzero's pnacl bitcode translator.

Also clean up other error cases (in function block) to simply return, since they have already generated an error message.

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

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

[Karl, 2014-08-28 18:22:22]

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

[jvoung (off chromium), 2014-08-28 21:35:03]

looks mostly good to me

https://codereview.chromium.org/514273002/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/514273002/diff/20001/src/PNaClTranslator.cpp#...
src/PNaClTranslator.cpp:1174: if (!naclbitc::DecodeCastOpcode(Values[2],
LLVMCastOp)
Should we just make one switch statement that folds the
naclbitc::DecodeCastOpcode(Values[2], ...) -> convertLLVMCastOpToIceOp(...,
CastKind). That is, skips through making an LLVMCastOp?

Okay, so the issue is checking CastInst::castIsValid?

https://codereview.chromium.org/514273002/diff/20001/src/PNaClTranslator.cpp#...
src/PNaClTranslator.cpp:1199: break;
Should this be a return too?

[Jim Stichnoth, 2014-08-28 21:58:54]

stichnot@chromium.org changed reviewers:
+ stichnot@chromium.org
- stichnot@google.com

[Jim Stichnoth, 2014-08-28 22:16:04]

https://codereview.chromium.org/514273002/diff/20001/tests_lit/reader_tests/c...
File tests_lit/reader_tests/casts.ll (right):

https://codereview.chromium.org/514273002/diff/20001/tests_lit/reader_tests/c...
tests_lit/reader_tests/casts.ll:446: define double @BitcastI64(i64 %v) {
For completeness, add BitcastFloat and BitcastDouble?

Also, would it make sense to spot-check some of the vector types?

[Karl, 2014-08-29 16:00:30]

https://codereview.chromium.org/514273002/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/514273002/diff/20001/src/PNaClTranslator.cpp#...
src/PNaClTranslator.cpp:1174: if (!naclbitc::DecodeCastOpcode(Values[2],
LLVMCastOp)
On 2014/08/28 21:35:03, jvoung wrote:
> Should we just make one switch statement that folds the
> naclbitc::DecodeCastOpcode(Values[2], ...) -> convertLLVMCastOpToIceOp(...,
> CastKind). That is, skips through making an LLVMCastOp?
> 
> Okay, so the issue is checking CastInst::castIsValid?

Done.

https://codereview.chromium.org/514273002/diff/20001/src/PNaClTranslator.cpp#...
src/PNaClTranslator.cpp:1199: break;
On 2014/08/28 21:35:03, jvoung wrote:
> Should this be a return too?

Good point.

https://codereview.chromium.org/514273002/diff/20001/tests_lit/reader_tests/c...
File tests_lit/reader_tests/casts.ll (right):

https://codereview.chromium.org/514273002/diff/20001/tests_lit/reader_tests/c...
tests_lit/reader_tests/casts.ll:446: define double @BitcastI64(i64 %v) {
On 2014/08/28 22:16:04, stichnot wrote:
> For completeness, add BitcastFloat and BitcastDouble?
> 
> Also, would it make sense to spot-check some of the vector types?

Added missing cases.

[Jim Stichnoth, 2014-08-29 17:24:00]

lgtm

[jvoung (off chromium), 2014-08-29 17:26:34]

Otherwise LGTM too

https://codereview.chromium.org/514273002/diff/40001/tests_lit/reader_tests/c...
File tests_lit/reader_tests/casts.ll (right):

https://codereview.chromium.org/514273002/diff/40001/tests_lit/reader_tests/c...
tests_lit/reader_tests/casts.ll:516: ; CHECK-NEXT: define void @BitcastV4xi16(<8
x i16> %__0) {
Function name should be V8xi16 ?

[Karl, 2014-09-02 17:47:40]

Committed patchset #3 (id:40001) manually as c0fdc27 (tree was closed).

[Karl, 2014-09-02 17:47:57]

https://codereview.chromium.org/514273002/diff/40001/tests_lit/reader_tests/c...
File tests_lit/reader_tests/casts.ll (right):

https://codereview.chromium.org/514273002/diff/40001/tests_lit/reader_tests/c...
tests_lit/reader_tests/casts.ll:516: ; CHECK-NEXT: define void @BitcastV4xi16(<8
x i16> %__0) {
On 2014/08/29 17:26:34, jvoung wrote:
> Function name should be V8xi16 ?

Done.