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=Function_Names#Function_Names
(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.

http://codereview.chromium.org/2019003/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to