Fix vldrs/vstrs handling of immediate offsets in ARM.

Fix vldrs/vstrs handling of immediate offsets in ARM.

A previous patch fixed vldrd/vstrd by dividing the immediate offset of
the instruction by 4 before encoding. This does the same for
vldrs/vstrs. It fixes the remaining problems with compiling spec2k
using -filetype=iasm.

It also fixes a minor bug in the divsion by 4, in the case that the
immediate value is negative.

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

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

[Karl, 2016-01-22 17:50:21]

Description was changed from

==========
Fix vldrs/vstrs handling of immediate offsets in ARM.

A previous patch fixed vldrd/vstrd by dividing the immediate offset of
the instruction by 4 before encoding. This does the same for
vldrs/vstrs. It fixes the remaining problems with compiling spec2k
using -filetype=iasm.

It also fixes a minor bug in the divsion by 4, in the case that the
immediate value is negative.

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

to

==========
Fix vldrs/vstrs handling of immediate offsets in ARM.

A previous patch fixed vldrd/vstrd by dividing the immediate offset of
the instruction by 4 before encoding. This does the same for
vldrs/vstrs. It fixes the remaining problems with compiling spec2k
using -filetype=iasm.

It also fixes a minor bug in the divsion by 4, in the case that the
immediate value is negative.

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

[Karl, 2016-01-22 17:50:21]

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

[John, 2016-01-22 18:47:41]

lgtm

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

https://codereview.chromium.org/1617993005/diff/1/src/IceAssemblerARM32.cpp#n...
src/IceAssemblerARM32.cpp:2247: (void)AddressEncoding; // Make MINIMAL build
happy.
no need for this comment. This is a pretty common idiom in subzero
(unfortunately)

[Karl, 2016-01-22 20:39:54]

Description was changed from

==========
Fix vldrs/vstrs handling of immediate offsets in ARM.

A previous patch fixed vldrd/vstrd by dividing the immediate offset of
the instruction by 4 before encoding. This does the same for
vldrs/vstrs. It fixes the remaining problems with compiling spec2k
using -filetype=iasm.

It also fixes a minor bug in the divsion by 4, in the case that the
immediate value is negative.

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

to

==========
Fix vldrs/vstrs handling of immediate offsets in ARM.

A previous patch fixed vldrd/vstrd by dividing the immediate offset of
the instruction by 4 before encoding. This does the same for
vldrs/vstrs. It fixes the remaining problems with compiling spec2k
using -filetype=iasm.

It also fixes a minor bug in the divsion by 4, in the case that the
immediate value is negative.

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

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

[Karl, 2016-01-22 20:39:55]

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

[Karl, 2016-01-22 20:42:19]

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

https://codereview.chromium.org/1617993005/diff/1/src/IceAssemblerARM32.cpp#n...
src/IceAssemblerARM32.cpp:2247: (void)AddressEncoding; // Make MINIMAL build
happy.
On 2016/01/22 18:47:40, John wrote:
> no need for this comment. This is a pretty common idiom in subzero
> (unfortunately)

Done.

[Jim Stichnoth, 2016-01-22 22:47:15]

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

https://codereview.chromium.org/1617993005/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:363: Offset = -Offset;
I assume this is probably not possible, but it sets off a boundary-case trigger
for me - what happens when Offset == std::numeric_limits<IValueT>::lowest()? 
(i.e. MININT)

[John, 2016-01-22 23:33:25]

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

https://codereview.chromium.org/1617993005/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:363: Offset = -Offset;
On 2016/01/22 22:47:15, stichnot wrote:
> I assume this is probably not possible, but it sets off a boundary-case
trigger
> for me - what happens when Offset == std::numeric_limits<IValueT>::lowest()? 
> (i.e. MININT)

arm32 immediates are small (usually 8-bits, sometimes 12-, rarely 24-bits) so
this should not be a problem.

[Karl, 2016-01-25 19:03:26]

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

https://codereview.chromium.org/1617993005/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:363: Offset = -Offset;
On 2016/01/22 23:33:25, John wrote:
> On 2016/01/22 22:47:15, stichnot wrote:
> > I assume this is probably not possible, but it sets off a boundary-case
> trigger
> > for me - what happens when Offset == std::numeric_limits<IValueT>::lowest()?

> > (i.e. MININT)
> 
> arm32 immediates are small (usually 8-bits, sometimes 12-, rarely 24-bits) so
> this should not be a problem.

Fixing with CL https://codereview.chromium.org/1630863002