Chromium Code Reviews
Help | Chromium Project | Sign in
(178)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 6 months ago by Michael Starzinger
Modified:
1 year, 6 months ago
CC:
v8-dev_googlegroups.com
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) Lint Patch
M src/arm/macro-assembler-arm.h View 1 1 chunk +1 line, -1 line 0 comments ? errors Download
M src/arm/macro-assembler-arm.cc View 1 2 chunks +5 lines, -3 lines 0 comments ? errors Download
Trybot results:
Commit:

Messages

Total messages: 8
Michael Starzinger
1 year, 6 months ago #1
Please use jfb - chromium.org
LGTM, pending perf results.
1 year, 6 months ago #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 ...
1 year, 6 months ago #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 ...
1 year, 6 months ago #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, ...
1 year, 6 months ago #5
ulan
LGTM
1 year, 6 months ago #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: > ...
1 year, 6 months ago #7
m.m.capewell
1 year, 6 months ago #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 ;)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6