Add support for movw/movt immediate loads to BackEdgeTable on ARM to enable ool constant pool.

Add support for movw/movt immediate loads to BackEdgeTable on ARM to enable ool constant pool.

R=rodolph.perfetta@arm.com, ulan@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=20023

[rmcilroy, 2014-03-07 18:51:50]

Part of the out-of-line constant pool change - PTAL.

[Rodolph Perfetta (ARM), 2014-03-10 16:19:59]

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4844:
Assembler::IsMovT(Memory::int32_at(immediate_load_address))) {
Is there a case where ool is enable and we don't have a movt here?

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4858: int branch_offset = (2 * kInstrSize) + (pc -
pc_immediate_load_address);
Some comments about the 2 * kInstrSize would be nice

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4862: patcher.masm()->b(branch_offset, pl);
While I agree the encoding isn't that interesting, having the patched code
sequence as comments make the code easier to read. Admittedly this is a bit more
complicated now.

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4863:
ASSERT(Assembler::IsBranch(Assembler::instr_at(branch_address)));
This assert check that b(...) will emit a branch, is that useful?

[rmcilroy, 2014-03-11 19:05:53]

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4844:
Assembler::IsMovT(Memory::int32_at(immediate_load_address))) {
On 2014/03/10 16:19:59, Rodolph Perfetta (ARM) wrote:
> Is there a case where ool is enable and we don't have a movt here?

Yes, the possibilities are:
 existing inline constant pool:
         ldr[pc...]
 new ool constant pool:
   - if the assembler could use the constant pool (e.g., was available and not
full):
         ldr[pp...]
   - if not able to use the constant pool:
         movt[...]
         movw[...]

I've reworked the function to be more understandable.

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4858: int branch_offset = (2 * kInstrSize) + (pc -
pc_immediate_load_address);
On 2014/03/10 16:19:59, Rodolph Perfetta (ARM) wrote:
> Some comments about the 2 * kInstrSize would be nice

Done (and reworked the calculation to be more understandable).

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4862: patcher.masm()->b(branch_offset, pl);
On 2014/03/10 16:19:59, Rodolph Perfetta (ARM) wrote:
> While I agree the encoding isn't that interesting, having the patched code
> sequence as comments make the code easier to read. Admittedly this is a bit
more
> complicated now.

Done.

https://codereview.chromium.org/190833002/diff/1/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4863:
ASSERT(Assembler::IsBranch(Assembler::instr_at(branch_address)));
On 2014/03/10 16:19:59, Rodolph Perfetta (ARM) wrote:
> This assert check that b(...) will emit a branch, is that useful?

Agreed, not useful.  Removed.

[Rodolph Perfetta (ARM), 2014-03-12 14:55:05]

lgtm

[rmcilroy, 2014-03-17 17:26:02]

On 2014/03/12 14:55:05, Rodolph Perfetta (ARM) wrote:
> lgtm

Ulan: Are you OK with this CL?

[ulan, 2014-03-18 09:20:23]

> Ulan: Are you OK with this CL?
LGTM, sorry I missed this CL.

[rmcilroy, 2014-03-18 11:36:24]

Committed patchset #3 manually as r20023 (presubmit successful).

[rmcilroy, 2014-03-18 11:36:26]

On 2014/03/18 09:20:23, ulan wrote:
> > Ulan: Are you OK with this CL?
> LGTM, sorry I missed this CL.

Thanks Ulan.