Fix ARM emit() methods to count instructions generated.

Fix ARM emit() methods to count instructions generated.

Previously, the code assumed that the emit() method of all ARM
instructions emitted a single instruction. This is false. Instructions
like PUSH and POP may generate multiple instructions.

This is only a problem when the hybrid ARM assembler reverts back to
using the stand-alone assembler to generate instructions the
integrated assembler can't handle.

The fix is to add infrastructure to allow ARM instructions to
communicate to the assembler, the number of instructions they
generate, so that the correct-sized filler is added to the assembly
buffer.

This fixes all cross-test failures for (pc-relative) branches, except
one.

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

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

[Karl, 2015-11-04 22:44:09]

Description was changed from

==========
Fix ARM emit() methods to count instructions generated.

Previously, the code assumed that the emit() method of all ARM
instructions emitted a single instruction. This is false. Instructions
like PUSH and POP may generate multiple instructions.

This is only a problem when the hybrid ARM assembler reverts back to
using the stand-alone assembler to generate instructions the
integrated assembler can't handle.

The fix is to add infrastructure to allow ARM instructions to
communicate to the assembler, the number of instructions they
generate, so that the correct-sized filler is added to the assembly
buffer.

This fixes all cross-test failures for (pc-relative) branches, except
one.

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

to

==========
Fix ARM emit() methods to count instructions generated.

Previously, the code assumed that the emit() method of all ARM
instructions emitted a single instruction. This is false. Instructions
like PUSH and POP may generate multiple instructions.

This is only a problem when the hybrid ARM assembler reverts back to
using the stand-alone assembler to generate instructions the
integrated assembler can't handle.

The fix is to add infrastructure to allow ARM instructions to
communicate to the assembler, the number of instructions they
generate, so that the correct-sized filler is added to the assembly
buffer.

This fixes all cross-test failures for (pc-relative) branches, except
one.

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

[Karl, 2015-11-04 22:44:09]

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

[Jim Stichnoth, 2015-11-04 23:45:27]

Otherwise LGTM, but +jpp please take note of my comment on the compound branch
instruction.

https://codereview.chromium.org/1426513004/diff/1/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1426513004/diff/1/src/IceAssemblerARM32.cpp#n...
src/IceAssemblerARM32.cpp:365: if (InstSize) {
Remove this?  It's redundant with the loop condition.

https://codereview.chromium.org/1426513004/diff/1/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1426513004/diff/1/src/IceAssemblerARM32.h#new...
src/IceAssemblerARM32.h:142: void startFirstInst() { TextInstCount = 1; }
Bikeshedding a bit.  It would be nice if the getter/setter/updater method names
more similar to the field name, for easier grepping and comprehension.

https://codereview.chromium.org/1426513004/diff/1/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1426513004/diff/1/src/IceInstARM32.cpp#newcod...
src/IceInstARM32.cpp:814: startNextInst(Func);
This is interesting.  Are our ARM conditional branches really represented as a
compound instruction of a conditional branch to the "true" target followed by an
unconditional branch to the "false" target?  This may need to be changed to
enable branch optimizations.

[Karl, 2015-11-05 16:18:31]

Committed patchset #2 (id:20001) manually as
856734ca6f51025ca428494ddd740f27fab40438 (presubmit successful).

[Karl, 2015-11-05 16:18:41]

https://codereview.chromium.org/1426513004/diff/1/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1426513004/diff/1/src/IceAssemblerARM32.cpp#n...
src/IceAssemblerARM32.cpp:365: if (InstSize) {
On 2015/11/04 23:45:26, stichnot wrote:
> Remove this?  It's redundant with the loop condition.

Done.

https://codereview.chromium.org/1426513004/diff/1/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1426513004/diff/1/src/IceAssemblerARM32.h#new...
src/IceAssemblerARM32.h:142: void startFirstInst() { TextInstCount = 1; }
On 2015/11/04 23:45:26, stichnot wrote:
> Bikeshedding a bit.  It would be nice if the getter/setter/updater method
names
> more similar to the field name, for easier grepping and comprehension.

Done.