ARM: Clean up literal pool generation.... (issue7108061)

[m.m.ca...@googlemail.com]

Reviewers: Mads Ager,

Description:
ARM: Clean up literal pool generation.
Remove dead code, and generate pools less frequently.

BUG=none
TEST=none

Please review this at http://codereview.chromium.org/7108061/

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/

Affected files:
M src/arm/assembler-arm.h
M src/arm/assembler-arm.cc
M src/arm/macro-assembler-arm.h
M src/arm/macro-assembler-arm.cc
M src/arm/regexp-macro-assembler-arm.cc

[ag...@chromium.org]

Redirecting.

http://codereview.chromium.org/7108061/

[sgj...@chromium.org]

LGTM, but please consider the comments

And while you are changing these files how about renaming the prinfo in
prinfo_
and num_prinfo_ to something more else e.g. pending_reloc_info?

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode327
src/arm/assembler-arm.cc:327: first_const_pool_use_ = 0;
Maybe initialize to -1, as the value is only valid when num_prinfo_ is >
0.

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode2589
src/arm/assembler-arm.cc:2589: // the constant pool is at least
kMaxDistToPool / 2.
And then add ASSERT(first_const_pool_use_ >= 0).

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode2628
src/arm/assembler-arm.cc:2628: ASSERT(IsLdrPcImmediateOffset(instr) &&
(instr & kOff12Mask) == 0);
How about adding GetLdrPcImmediateOffsetOffset?

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode2637
src/arm/assembler-arm.cc:2637: instr_at_put(rinfo.pc(), instr + delta);
How about adding SetLdrPcImmediateOffsetOffset?

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode2642
src/arm/assembler-arm.cc:2642: first_const_pool_use_ = 0;
0 -> -1?

http://codereview.chromium.org/7108061/

[rodolph....@gmail.com]

Comments addressed, new patch will be uploaded soon.

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode327
src/arm/assembler-arm.cc:327: first_const_pool_use_ = 0;

On 2011/06/14 07:50:56, Søren Gjesse wrote:
> Maybe initialize to -1, as the value is only valid when num_prinfo_ is
> 0.

Done.

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode2589
src/arm/assembler-arm.cc:2589: // the constant pool is at least
kMaxDistToPool / 2.

On 2011/06/14 07:50:56, Søren Gjesse wrote:
> And then add ASSERT(first_const_pool_use_ >= 0).

Done.

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode2628
src/arm/assembler-arm.cc:2628: ASSERT(IsLdrPcImmediateOffset(instr) &&
(instr & kOff12Mask) == 0);

Used GetLdrRegImmediateOffset which was already implemented.

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode2637
src/arm/assembler-arm.cc:2637: instr_at_put(rinfo.pc(), instr + delta);

Used SetLdrRegImmediateOffset which was already implemented.

http://codereview.chromium.org/7108061/diff/4001/src/arm/assembler-arm.cc#newcode2642
src/arm/assembler-arm.cc:2642: first_const_pool_use_ = 0;

On 2011/06/14 07:50:56, Søren Gjesse wrote:
> 0 -> -1?

Done.

http://codereview.chromium.org/7108061/

[m.m.ca...@googlemail.com]

http://codereview.chromium.org/7108061/

[sgj...@chromium.org]

LGTM

I will commit it.

http://codereview.chromium.org/7108061/