Clean up emitInst() in the integrated ARM assembler.

Clean up emitInst() in the integrated ARM assembler.

Moves "EnsureCapacity Buffer" declarations inside emitInst(), so that
all callers need not add the declaration before calling emitInst().

BUG=None
R=stichnot@chromium.org

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

[Karl, 2016-01-25 16:37:24]

Description was changed from

==========
Clean up emitInst() in the integrated ARM assembler.

Moves "EnsureCapacity Buffer" declarations inside emitInst(), so that
all callers need not add the declaration before calling emitInst().

BUG=None
==========

to

==========
Clean up emitInst() in the integrated ARM assembler.

Moves "EnsureCapacity Buffer" declarations inside emitInst(), so that
all callers need not add the declaration before calling emitInst().

BUG=None
==========

[Karl, 2016-01-25 16:37:24]

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

[Jim Stichnoth, 2016-01-25 16:57:15]

Channeling Jan here...

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:646: AssemblerBuffer::EnsureCapacity ensured(&Buffer);
What about this one?

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:1962: AssemblerBuffer::EnsureCapacity
ensured(&Buffer);
What about this one?

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.h...
src/IceAssemblerARM32.h:459: AssemblerBuffer::EnsureCapacity ensured(&Buffer);
John has a coding style I like where these RAII placeholder variables are named
"_".  I.e.:

  AssemblerBuffer::EnsureCapacity _(&Buffer);

[Karl, 2016-01-25 17:14:07]

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:646: AssemblerBuffer::EnsureCapacity ensured(&Buffer);
On 2016/01/25 16:57:14, stichnot wrote:
> What about this one?

The issue here is that the API is to create an instance of EnsureCapacity before
each sequence of emit() calls, to guarantee there is space in the buffer (i.e.
predefined constant 32). The destructor ~EnsureCapacity() then checks that the
sequence of emits() doesn't exceed this gap constant.

This for of emit could be more intelligent about this and cluster emits.
However, this would complicate the logic (as well as expose the gap limit).
Since text instructions are not in the minimal build, I did not worry about
efficiency, just simplicity.

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:1962: AssemblerBuffer::EnsureCapacity
ensured(&Buffer);
On 2016/01/25 16:57:14, stichnot wrote:
> What about this one?

I would have used emitInst(), but the representation of TrapBytes (already
defined for use by the assembler elsewhere) is not type compatible.

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1636513002/diff/20001/src/IceAssemblerARM32.h...
src/IceAssemblerARM32.h:459: AssemblerBuffer::EnsureCapacity ensured(&Buffer);
On 2016/01/25 16:57:14, stichnot wrote:
> John has a coding style I like where these RAII placeholder variables are
named
> "_".  I.e.:
> 
>   AssemblerBuffer::EnsureCapacity _(&Buffer);

I really don't like this style! However, if we are using it, so will I.

[Jim Stichnoth, 2016-01-25 17:20:47]

lgtm

[Karl, 2016-01-25 17:54:23]

Description was changed from

==========
Clean up emitInst() in the integrated ARM assembler.

Moves "EnsureCapacity Buffer" declarations inside emitInst(), so that
all callers need not add the declaration before calling emitInst().

BUG=None
==========

to

==========
Clean up emitInst() in the integrated ARM assembler.

Moves "EnsureCapacity Buffer" declarations inside emitInst(), so that
all callers need not add the declaration before calling emitInst().

BUG=None
R=stichnot@chromium.org

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

[Karl, 2016-01-25 17:54:24]

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