Fix vldrd/vstrd handling of immediate offsets in ARM.

Fix vldrd/vstrd handling of immediate offsets in ARM.

Fixes the ARM integrated assembler by dividing the immediate offset
of the instruction by 4 before encoding.

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

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

[Karl, 2016-01-21 22:07:45]

Description was changed from

==========
Fix vldrd/vstrd handling of immediate offsets in ARM.

Fixes the ARM integrated assembler by dividing the immediate offset
of the instruction by 4 before encoding.

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

to

==========
Fix vldrd/vstrd handling of immediate offsets in ARM.

Fixes the ARM integrated assembler by dividing the immediate offset
of the instruction by 4 before encoding.

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

[Karl, 2016-01-21 22:07:45]

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

[Jim Stichnoth, 2016-01-22 05:10:30]

lgtm

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

https://codereview.chromium.org/1619703008/diff/1/src/IceAssemblerARM32.cpp#n...
src/IceAssemblerARM32.cpp:2230: (void)AddressEncoding;
remove this

https://codereview.chromium.org/1619703008/diff/1/src/IceAssemblerARM32.cpp#n...
src/IceAssemblerARM32.cpp:2307: (void)AddressEncoding;
remove this

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

https://codereview.chromium.org/1619703008/diff/1/src/IceInstARM32.cpp#newcod...
src/IceInstARM32.cpp:1375: Type DestTy = Dest->getType();
const, if you feel so inclined...

https://codereview.chromium.org/1619703008/diff/1/tests_lit/assembler/arm32/v...
File tests_lit/assembler/arm32/vldr.vstr.imm.ll (right):

https://codereview.chromium.org/1619703008/diff/1/tests_lit/assembler/arm32/v...
tests_lit/assembler/arm32/vldr.vstr.imm.ll:1: ; Test vldrd and vstrd when
address is offset with an immediate.
Did you mean "vldr and vstr"?

[John, 2016-01-22 15:04:28]

lgtm

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

https://codereview.chromium.org/1619703008/diff/1/src/IceInstARM32.cpp#newcod...
src/IceInstARM32.cpp:1380: // TODO(kschimpf) Does this happen?
It does not happen at the moment, but it will once we have vldr for vector
types.

[Karl, 2016-01-22 16:22:46]

Description was changed from

==========
Fix vldrd/vstrd handling of immediate offsets in ARM.

Fixes the ARM integrated assembler by dividing the immediate offset
of the instruction by 4 before encoding.

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

to

==========
Fix vldrd/vstrd handling of immediate offsets in ARM.

Fixes the ARM integrated assembler by dividing the immediate offset
of the instruction by 4 before encoding.

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

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

[Karl, 2016-01-22 16:22:47]

Committed patchset #2 (id:20001) manually as
08f7910790691880b5701eed43cc8cc0b2281afc (presubmit successful).

[Karl, 2016-01-22 16:24:29]

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

https://codereview.chromium.org/1619703008/diff/1/src/IceAssemblerARM32.cpp#n...
src/IceAssemblerARM32.cpp:2230: (void)AddressEncoding;
On 2016/01/22 05:10:30, stichnot wrote:
> remove this

Done.

https://codereview.chromium.org/1619703008/diff/1/src/IceAssemblerARM32.cpp#n...
src/IceAssemblerARM32.cpp:2307: (void)AddressEncoding;
On 2016/01/22 05:10:30, stichnot wrote:
> remove this

Done.

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

https://codereview.chromium.org/1619703008/diff/1/src/IceInstARM32.cpp#newcod...
src/IceInstARM32.cpp:1375: Type DestTy = Dest->getType();
On 2016/01/22 05:10:30, stichnot wrote:
> const, if you feel so inclined...

Done.

https://codereview.chromium.org/1619703008/diff/1/src/IceInstARM32.cpp#newcod...
src/IceInstARM32.cpp:1380: // TODO(kschimpf) Does this happen?
On 2016/01/22 15:04:28, John wrote:
> It does not happen at the moment, but it will once we have vldr for vector
> types.

Actually, due to the if above, this shouldn't happen. However, there are so many
type cases to this code that we should flatten to a single switch. However, I
will do that in a separate CL.

https://codereview.chromium.org/1619703008/diff/1/tests_lit/assembler/arm32/v...
File tests_lit/assembler/arm32/vldr.vstr.imm.ll (right):

https://codereview.chromium.org/1619703008/diff/1/tests_lit/assembler/arm32/v...
tests_lit/assembler/arm32/vldr.vstr.imm.ll:1: ; Test vldrd and vstrd when
address is offset with an immediate.
On 2016/01/22 05:10:30, stichnot wrote:
> Did you mean "vldr and vstr"?

This file doesn't test vstrs/vlds (or many other variants not implemented yet).