Update and improve support for ARMv7 bitfield instructions....

Update and improve support for ARMv7 bitfield instructions.

BUG=none
TEST=added to cctest

[Erik Corry, 2010-05-26 07:02:43]

LGTM

http://codereview.chromium.org/2124022/diff/1/2
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/2124022/diff/1/2#newcode1111
src/arm/assembler-arm.cc:1111: emit(cond | 15*B23 | B22 | B21 | (width-1)*B16 |
dst.code()*B12 |
Spaces around '-'.  We make an exception to the 'spaces around binary ops' rule
in this file where we multiply by the Bnn constants.  I don't think we should
extend that style breach, which at least has the merit of consistency and
conciseness to other binary ops.

http://codereview.chromium.org/2124022/diff/1/2#newcode1131
src/arm/assembler-arm.cc:1131: emit(cond | 15*B23 | B21 | (width-1)*B16 |
dst.code()*B12 |
Spaces around '-'.

http://codereview.chromium.org/2124022/diff/1/2#newcode1131
src/arm/assembler-arm.cc:1131: emit(cond | 15*B23 | B21 | (width-1)*B16 |
dst.code()*B12 |
In this context, 15 is a bit pattern for the instruction.  We prefer hexadecimal
for bit patterns.  It doesn't matter for numbers < 10.

http://codereview.chromium.org/2124022/diff/1/4
File src/arm/disasm-arm.cc (right):

http://codereview.chromium.org/2124022/diff/1/4#newcode407
src/arm/disasm-arm.cc:407: if (!instr->Bit(21)) {
We don't allow implicit coercions to and from bool, so you should test the
result of Bit() against zero.

http://codereview.chromium.org/2124022/diff/1/4#newcode410
src/arm/disasm-arm.cc:410: width -= lsbit;
Should we assert that width doesn't go negative here.  Presumably that could be
used to code a completely different instruction at some point.

http://codereview.chromium.org/2124022/diff/1/4#newcode411
src/arm/disasm-arm.cc:411: }
Assert that width + lsbit <= 32?

[rodolph, 2010-05-27 12:24:54]

New draft uploaded

http://codereview.chromium.org/2124022/diff/1/2
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/2124022/diff/1/2#newcode1111
src/arm/assembler-arm.cc:1111: emit(cond | 15*B23 | B22 | B21 | (width-1)*B16 |
dst.code()*B12 |
On 2010/05/26 07:02:43, Erik Corry wrote:
> Spaces around '-'.  We make an exception to the 'spaces around binary ops'
rule
> in this file where we multiply by the Bnn constants.  I don't think we should
> extend that style breach, which at least has the merit of consistency and
> conciseness to other binary ops.

Done.

http://codereview.chromium.org/2124022/diff/1/2#newcode1131
src/arm/assembler-arm.cc:1131: emit(cond | 15*B23 | B21 | (width-1)*B16 |
dst.code()*B12 |
On 2010/05/26 07:02:43, Erik Corry wrote:
> Spaces around '-'.

Done.

http://codereview.chromium.org/2124022/diff/1/2#newcode1131
src/arm/assembler-arm.cc:1131: emit(cond | 15*B23 | B21 | (width-1)*B16 |
dst.code()*B12 |
On 2010/05/26 07:02:43, Erik Corry wrote:
> Spaces around '-'.

Done.

http://codereview.chromium.org/2124022/diff/1/4
File src/arm/disasm-arm.cc (right):

http://codereview.chromium.org/2124022/diff/1/4#newcode407
src/arm/disasm-arm.cc:407: if (!instr->Bit(21)) {
On 2010/05/26 07:02:43, Erik Corry wrote:
> We don't allow implicit coercions to and from bool, so you should test the
> result of Bit() against zero.

Done.

http://codereview.chromium.org/2124022/diff/1/4#newcode410
src/arm/disasm-arm.cc:410: width -= lsbit;
On 2010/05/26 07:02:43, Erik Corry wrote:
> Should we assert that width doesn't go negative here.  Presumably that could
be
> used to code a completely different instruction at some point.

Done.

http://codereview.chromium.org/2124022/diff/1/4#newcode411
src/arm/disasm-arm.cc:411: }
On 2010/05/26 07:02:43, Erik Corry wrote:
> Assert that width + lsbit <= 32?

Done.

[Erik Corry, 2010-05-27 13:46:40]

Committed as r4744.  Thanks.