Subzero. X8664. Fixes various small bugs.

Subzero. X8664. Fixes various small bugs.

These were all pointed out by the llvm test suite, the gcc torture
tests, and the scons tests.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4077
R=stichnot@chromium.org

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

[John, 2016-01-26 20:52:48]

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

[John, 2016-01-26 20:58:41]

Description was changed from

==========
Subzero. X8664. Fixes various small bugs.

These were all pointed out by the llvm test suite, the gcc torture
tests, and the scons tests.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4077
==========

to

==========
Subzero. X8664. Fixes various small bugs.

These were all pointed out by the llvm test suite, the gcc torture
tests, and the scons tests.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4077
==========

[John, 2016-01-26 20:58:41]

jpp@chromium.org changed reviewers:
+ eholk@chromium.org

[John, 2016-01-26 21:04:38]

On 2016/01/26 20:52:49, John wrote:

FYI: this is presubmit happy.

[Jim Stichnoth, 2016-01-26 23:58:56]

https://codereview.chromium.org/1631383002/diff/1/src/IceTargetLoweringX86Bas...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/1631383002/diff/1/src/IceTargetLoweringX86Bas...
src/IceTargetLoweringX86BaseImpl.h:4603: //   bsr IF_NOT_ZERO, Val
These comments should be updated.

More importantly, I'd like to understand why our cross tests didn't catch this,
and to update them so that the new cases are handled/tested.

[John, 2016-01-27 13:03:15]

https://codereview.chromium.org/1631383002/diff/1/src/IceTargetLoweringX86Bas...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/1631383002/diff/1/src/IceTargetLoweringX86Bas...
src/IceTargetLoweringX86BaseImpl.h:4603: //   bsr IF_NOT_ZERO, Val
On 2016/01/26 23:58:55, stichnot wrote:
> These comments should be updated.
> 
> More importantly, I'd like to understand why our cross tests didn't catch
this,
> and to update them so that the new cases are handled/tested.

Look at the subzero output for one of the test_bitmanip tests:

        .text
        .globl  _ZN8Subzero_15test_const_ctlzEj
        .type   _ZN8Subzero_15test_const_ctlzEj,%function
        .p2align 5,0xf4
_ZN8Subzero_15test_const_ctlzEj:
.L_Z15test_const_ctlzj$entry:
        subl    $28, %esp
        movl    $74560, (%esp)
        call    _Z7my_ctlzj  /* wrong! should've been _ZN8Subzero_7my_ctlzEj */
        addl    $28, %esp
        ret


Essentially, for this test, we have

main (pnacl-clang)
test_bitmanip (llc)
test_bitmanip_intrin (llc) [+]
test_bitmanip (sz)  [*]
test_bitmanip_intrin (sz)

when subzero translates [*] the calls to my_cttz (and friends) are external to
the translation unit. Therefore, subzero knows better, and it does not mangle
those names.

Comes link time, and [*] is referring to the my_cttz (and friends) in [+], which
were translated by llc.

Effectively, the crosstest is testing if a sz-translated module can call a
routine an llc-translated routine.

It seems an fix for this would be to remove the test_bitmanip.cpp file and
inline its contents in the driver. The problem with this approach is that the
different tests there (_alloca, _const, and regular) will be reduntant.

Anyway, this is a serious issue (I am sure there are other instances of this
problem in our test suite) so maybe now is a good time to think/rework
crosstests (on another CL, of course)?

[Jim Stichnoth, 2016-01-27 13:08:20]

lgtm

[John, 2016-01-27 14:31:57]

Description was changed from

==========
Subzero. X8664. Fixes various small bugs.

These were all pointed out by the llvm test suite, the gcc torture
tests, and the scons tests.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4077
==========

to

==========
Subzero. X8664. Fixes various small bugs.

These were all pointed out by the llvm test suite, the gcc torture
tests, and the scons tests.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4077
R=stichnot@chromium.org

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

[John, 2016-01-27 14:31:58]

Committed patchset #2 (id:20001) manually as
34d276a31732a9bbf514dbc9a52c7488e15c5bba (presubmit successful).