Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(921)

Issue 115256: Fix fp problems in runtime code on ARM EABI by 8-byte aligning... (Closed)

Created:
11 years, 7 months ago by Erik Corry
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Fix fp problems in runtime code on ARM EABI by 8-byte aligning the stack on exit to C. Committed: http://code.google.com/p/v8/source/detail?r=1922

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -5 lines) Patch
M src/arm/assembler-arm.h View 1 chunk +2 lines, -2 lines 0 comments Download
M src/arm/macro-assembler-arm.cc View 3 chunks +17 lines, -3 lines 1 comment Download
M src/platform-linux.cc View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Erik Corry
11 years, 7 months ago (2009-05-12 19:44:32 UTC) #1
Søren Thygesen Gjesse
LGTM Though using the OS::ActivationFrameAlignment() is a bit misleading.
11 years, 7 months ago (2009-05-12 19:57:23 UTC) #2
Mark Lam
11 years, 7 months ago (2009-05-21 22:55:36 UTC) #3
Soren,

Sorry that I didn't have time to look at this until now.  The fix you guys came
up with is similar to mine except that you guys did the alignment fix in the
caller frame and I did mine in the callee frame.  Anyway, the fix looks good.  I
do have one comment below where you can remove one redundant "add" instruction.

Regards,
Mark

http://codereview.chromium.org/115256/diff/1/4
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/115256/diff/1/4#newcode305
Line 305: add(ip, sp, Operand(r0, LSL, kPointerSizeLog2));
This "add" instruction is functionally equivalent to the "add" at line 297.  You
can optimize the above 3 instructions slightly into the following 2
instructions:

add(ip, sp, Operand(r0, LSL, kPointerSizeLog2));
sub(r6, ip, Operand(kPointerSize));

Powered by Google App Engine
This is Rietveld 408576698