ldrd/strd

[Erik Corry, 2010-05-07 08:24:24]

http://codereview.chromium.org/2019003/diff/2003/7005
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/2019003/diff/2003/7005#newcode1166
src/arm/assembler-arm.cc:1166: src1.SetOffset(src1.GetOffset() + 4);
If this only works for MemOperands that have an offset then you should assert
that that is the kind of MemOperand we are dealing with.  The assert should be
outside the ifdef so we get the check even when compiling for ARM7.

http://codereview.chromium.org/2019003/diff/2003/7005#newcode1179
src/arm/assembler-arm.cc:1179: MemOperand dst1(src);
See comments for ldrd.

http://codereview.chromium.org/2019003/diff/2003/7002
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/2019003/diff/2003/7002#newcode450
src/arm/assembler-arm.h:450: void SetOffset(int32_t offset) {offset_ = offset;}
SetOffset and GetOffset should be called set_offset() and offset() in accordance
with
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...
(they are accessors, not normal functions).

In addition not all MemOperands have a valid offset.  Look at the comments
immediately above.  The set_offset and offset functions should ASSERT that they
are dealing with the right kind of MemOperand ie that rm_.is(no_reg) (see
below).

http://codereview.chromium.org/2019003/diff/2003/7006
File src/arm/simulator-arm.cc (right):

http://codereview.chromium.org/2019003/diff/2003/7006#newcode1011
src/arm/simulator-arm.cc:1011: int32_t* Simulator::ReadDW(int32_t addr) {
This needs to handle unaligned reads just like ReadW does.  Some ARM7 platforms
don't allow unaligned accesses and the simulator needs to be able to check that.

http://codereview.chromium.org/2019003/diff/2003/7006#newcode1018
src/arm/simulator-arm.cc:1018: int32_t* ptr = reinterpret_cast<int32_t*>(addr);
And alignment here.

[zhangk, 2010-05-07 19:17:35]

http://codereview.chromium.org/2019003/diff/2003/7005
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/2019003/diff/2003/7005#newcode1166
src/arm/assembler-arm.cc:1166: src1.SetOffset(src1.GetOffset() + 4);
On 2010/05/07 08:24:25, Erik Corry wrote:

Done.

http://codereview.chromium.org/2019003/diff/2003/7005#newcode1179
src/arm/assembler-arm.cc:1179: MemOperand dst1(src);
On 2010/05/07 08:24:25, Erik Corry wrote:

Done.

http://codereview.chromium.org/2019003/diff/2003/7002
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/2019003/diff/2003/7002#newcode450
src/arm/assembler-arm.h:450: void SetOffset(int32_t offset) {offset_ = offset;}
On 2010/05/07 08:24:25, Erik Corry wrote:

Done.

http://codereview.chromium.org/2019003/diff/2003/7006
File src/arm/simulator-arm.cc (right):

http://codereview.chromium.org/2019003/diff/2003/7006#newcode1011
src/arm/simulator-arm.cc:1011: int32_t* Simulator::ReadDW(int32_t addr) {
On 2010/05/07 08:24:25, Erik Corry wrote:

Done.

http://codereview.chromium.org/2019003/diff/2003/7006#newcode1018
src/arm/simulator-arm.cc:1018: int32_t* ptr = reinterpret_cast<int32_t*>(addr);
On 2010/05/07 08:24:25, Erik Corry wrote:

Done.

[Erik Corry, 2010-05-07 20:04:34]

I fixed the remaining issues (see below) and committed this as r4618.

http://codereview.chromium.org/2019003/diff/39001/7009
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/2019003/diff/39001/7009#newcode450
src/arm/assembler-arm.h:450: void set_offset(int32_t offset) {
I added single spaces around these functions to match the style of the
surrounding code.

http://codereview.chromium.org/2019003/diff/39001/7013
File src/arm/simulator-arm.cc (right):

http://codereview.chromium.org/2019003/diff/39001/7013#newcode1013
src/arm/simulator-arm.cc:1013: int32_t* ptr = reinterpret_cast<int32_t *>(addr);
I removed a space here for you.

http://codereview.chromium.org/2019003/diff/39001/7013#newcode1018
src/arm/simulator-arm.cc:1018: return *ptr;
I don't see how this compiles.  It should be returning an int32_t*, not an
intptr_t.

http://codereview.chromium.org/2019003/diff/39001/7013#newcode1040
src/arm/simulator-arm.cc:1040: PrintF("Unaligned write at 0x%08x, pc=%p\n",
addr, instr);
There's no variable called instr here so this doesn't compile either.  Fixed for
you.