Subzero. Misc ARM32 bugfixes.

Subzero. Misc ARM32 bugfixes.

With this CL, Spec2k built by the Sz ARM32 backend runs and verifies
successfully.

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

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

[John, 2015-10-15 22:38:34]

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

[John, 2015-10-15 22:39:09]

If you want to try this, you need to patch 

https://codereview.chromium.org/1412503002/

and build_toolchain_pnacl.py

[Jim Stichnoth, 2015-10-15 23:34:21]

Otherwise LGTM.

Sweet CL to review!

https://chromiumcodereview.appspot.com/1407063002/diff/40001/pydir/szbuild.py
File pydir/szbuild.py (right):

https://chromiumcodereview.appspot.com/1407063002/diff/40001/pydir/szbuild.py...
pydir/szbuild.py:93: help='Tells Subzero to generate code for the '
Bikeshed:
  help='Generate code for specified target'

https://chromiumcodereview.appspot.com/1407063002/diff/40001/pydir/szbuild_sp...
File pydir/szbuild_spec2k.py (right):

https://chromiumcodereview.appspot.com/1407063002/diff/40001/pydir/szbuild_sp...
pydir/szbuild_spec2k.py:39: # suffix = 'pnacl.opt.arm32' if args.sandbox else
'gcc.opt.arm32'
Delete this comment?

https://chromiumcodereview.appspot.com/1407063002/diff/40001/src/IceCfg.cpp
File src/IceCfg.cpp (right):

https://chromiumcodereview.appspot.com/1407063002/diff/40001/src/IceCfg.cpp#n...
src/IceCfg.cpp:685: Str << "\t.section\t.text." << MangledName <<
",\"ax\",%progbits\n";
Just checking -- is this syntax also valid for x86-32?

https://chromiumcodereview.appspot.com/1407063002/diff/40001/src/IceTargetLow...
File src/IceTargetLoweringARM32.cpp (right):

https://chromiumcodereview.appspot.com/1407063002/diff/40001/src/IceTargetLow...
src/IceTargetLoweringARM32.cpp:1816: if (Src0R->getType() != IceType_i32)
Won't it always be the case that Src0R->getType() == IceType_i1 ?

https://chromiumcodereview.appspot.com/1407063002/diff/40001/src/IceTargetLow...
src/IceTargetLoweringARM32.cpp:3335: if (CmpOpnd0->getType() != IceType_i32)
Same comment as above -- isn't the type always IceType_i1?

[Jim Stichnoth, 2015-10-15 23:34:21]

Otherwise LGTM.

Sweet CL to review!

[John, 2015-10-16 17:34:08]

Committed patchset #4 (id:60001) manually as
afc92af527400151b903745534c3aaf4bbedc61c (presubmit successful).

[John, 2015-11-05 20:25:13]

https://codereview.chromium.org/1407063002/diff/40001/pydir/szbuild.py
File pydir/szbuild.py (right):

https://codereview.chromium.org/1407063002/diff/40001/pydir/szbuild.py#newcode93
pydir/szbuild.py:93: help='Tells Subzero to generate code for the '
On 2015/10/15 23:34:21, stichnot wrote:
> Bikeshed:
>   help='Generate code for specified target'

Done.

https://codereview.chromium.org/1407063002/diff/40001/pydir/szbuild_spec2k.py
File pydir/szbuild_spec2k.py (right):

https://codereview.chromium.org/1407063002/diff/40001/pydir/szbuild_spec2k.py...
pydir/szbuild_spec2k.py:39: # suffix = 'pnacl.opt.arm32' if args.sandbox else
'gcc.opt.arm32'
On 2015/10/15 23:34:21, stichnot wrote:
> Delete this comment?

Done.

https://codereview.chromium.org/1407063002/diff/40001/src/IceCfg.cpp
File src/IceCfg.cpp (right):

https://codereview.chromium.org/1407063002/diff/40001/src/IceCfg.cpp#newcode685
src/IceCfg.cpp:685: Str << "\t.section\t.text." << MangledName <<
",\"ax\",%progbits\n";
On 2015/10/15 23:34:21, stichnot wrote:
> Just checking -- is this syntax also valid for x86-32?

well, szbuild_spec2k.py --target=x8632 --filetype=asm && run_all.sh reported
success, so I think it is.

https://codereview.chromium.org/1407063002/diff/40001/src/IceTargetLoweringAR...
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1407063002/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:1816: if (Src0R->getType() != IceType_i32)
On 2015/10/15 23:34:21, stichnot wrote:
> Won't it always be the case that Src0R->getType() == IceType_i1 ?

maybe, but better safe than sorry. I left an assert for quick test failure, but
I left the if here.

https://codereview.chromium.org/1407063002/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:3335: if (CmpOpnd0->getType() != IceType_i32)
On 2015/10/15 23:34:21, stichnot wrote:
> Same comment as above -- isn't the type always IceType_i1?

Done.