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

Issue 11090021: Improve page flag checking sequence on ARM. (Closed)

Created:
8 years, 2 months ago by Michael Starzinger
Modified:
8 years, 2 months ago
CC:
v8-dev
Visibility:
Public.

Description

Improve page flag checking sequence on ARM. R=ulan@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=12682

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -4 lines) Patch
M src/arm/macro-assembler-arm.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/arm/macro-assembler-arm.cc View 1 2 chunks +5 lines, -3 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Michael Starzinger
8 years, 2 months ago (2012-10-09 12:54:38 UTC) #1
Please use jfb - chromium.org
LGTM, pending perf results.
8 years, 2 months ago (2012-10-09 12:59:58 UTC) #2
Please use jfb - chromium.org
https://codereview.chromium.org/11090021/diff/1/src/arm/macro-assembler-arm.cc File src/arm/macro-assembler-arm.cc (right): https://codereview.chromium.org/11090021/diff/1/src/arm/macro-assembler-arm.cc#newcode3500 src/arm/macro-assembler-arm.cc:3500: Move(scratch, object); Actually, let me retract my LGTM: this ...
8 years, 2 months ago (2012-10-09 13:09:21 UTC) #3
m.m.capewell
https://codereview.chromium.org/11090021/diff/1/src/arm/macro-assembler-arm.cc File src/arm/macro-assembler-arm.cc (right): https://codereview.chromium.org/11090021/diff/1/src/arm/macro-assembler-arm.cc#newcode3501 src/arm/macro-assembler-arm.cc:3501: Bfc(scratch, 0, kPageSizeBits); A more generic way of doing ...
8 years, 2 months ago (2012-10-09 13:25:27 UTC) #4
Michael Starzinger
https://codereview.chromium.org/11090021/diff/1/src/arm/macro-assembler-arm.cc File src/arm/macro-assembler-arm.cc (right): https://codereview.chromium.org/11090021/diff/1/src/arm/macro-assembler-arm.cc#newcode3500 src/arm/macro-assembler-arm.cc:3500: Move(scratch, object); On 2012/10/09 13:09:21, jfb wrote: > Actually, ...
8 years, 2 months ago (2012-10-09 13:36:19 UTC) #5
ulan
LGTM
8 years, 2 months ago (2012-10-09 13:41:29 UTC) #6
Michael Starzinger
https://codereview.chromium.org/11090021/diff/1/src/arm/macro-assembler-arm.cc File src/arm/macro-assembler-arm.cc (right): https://codereview.chromium.org/11090021/diff/1/src/arm/macro-assembler-arm.cc#newcode3501 src/arm/macro-assembler-arm.cc:3501: Bfc(scratch, 0, kPageSizeBits); On 2012/10/09 13:25:27, m.m.capewell wrote: > ...
8 years, 2 months ago (2012-10-09 13:54:34 UTC) #7
m.m.capewell
8 years, 2 months ago (2012-10-09 14:07:27 UTC) #8
> So just to clarify. Do you think that the two-shift-approach would perform
> better than move+bfc on ARMv7? That would surprise me, but I am no ARM expert.

If you look at the A9 Technical Reference Manual, this page explains that BFC is
a two cycle instructions, whereas MOV with constant shift is single cycle:
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0388i/Chdgjcci.html

So shift right/left will be two cycles, and MOV + BFC three. However, if the
source and destination register are the same, only BFC would be required, and
would take two cycles with slightly smaller code.

The only way to be sure is to benchmark it ;)

Powered by Google App Engine
This is Rietveld 408576698