Adds support for ARMv6.

Adds support for ARMv6.

When we detect ARMv6, instead of using movw and
movt, this change loads each individual byte.
Although this is not the best way to achieve this,
a modification to store large constants in the
object pool would be more invasive. Further,
this change will be easier to back-out once
ARMv6 is obsolete.

R=regis@google.com

Committed: https://code.google.com/p/dart/source/detail?r=33442

[zra, 2014-03-06 00:21:09]

On 2014/03/06 00:18:06, zra wrote:

Just to clarify, this change does not totally remove movw and movt. We still use
them on ARMv7.

[regis, 2014-03-06 17:32:11]

LGTM with a rename suggestion.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm.cc
File runtime/vm/assembler_arm.cc (right):

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1460: LoadLargeImmediate(rd, offset_hi, cond);
I would simply call LoadImmediate here.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1483: LoadLargeImmediate(rd, object_raw, cond);
ditto

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1777: movt(IP, high);
Call LoadPatchableImmediate independently of the arm version.
LoadPatchableImmediate then checks the arm version.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1901: buffer_.Load<int32_t>(position);
You added  + 0 * Instr::kInstrSize below. For symmetry, you could add it here
too.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1924: buffer_.Load<int32_t>(position);
Here too? And below too? Not important.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2182: }
LoadPatchableImmediate for both. LoadPatchableImmediate checks the arm version.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2222: if (TargetCPUFeatures::arm_version() == ARMv6)
{
See my comment in the header.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2250: LoadLargeImmediate(rd, value, cond);
LoadDecodableImmediate

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2440: LoadLargeImmediate(IP, value, cond);
LoadDecodableImmediate

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2463: LoadLargeImmediate(IP, value, cond);
LoadDecodableImmediate

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2486: LoadLargeImmediate(IP, value, cond);
LoadDecodableImmediate

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.h:594: void LoadLargeImmediate(Register rd, int32_t
value, Condition cond = AL);
The function LoadImmediate below is also able to load a large immediate value.
The key here is not that you load a large immediate, but that you can decode the
emitted instructions, or that you can patch the immediate value.
Instead of adding LoadLargeImmediate, I would add 2 functions for clarity:
LoadDecodableImmediate and LoadPatchableImmediate.
On ARMv6, LoadDecodableImmediate would forward to LoadPatchableImmediate (same
code as in your LoadLargeImmediate).
On ARMv7, LoadDecodableImmediate would emit movw and optionally movt, whereas
LoadPatchableImmediate would always emit both movw and movt.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
File runtime/vm/assembler_arm_test.cc (right):

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm_test.cc:65: __ LoadLargeImmediate(R0, 0x12345678 << 1);
LoadDecodableImmediate

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm_test.cc:78: __ bx(LR);
__ bx(LR) should be outside of the branch.

Remove the version check anyway:

#else
  __ LoadDecodableImmediate(R0, 0x12345678);
  __ bx(LR);
#endif

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/instructions_...
File runtime/vm/instructions_arm_test.cc (right):

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/instructions_...
runtime/vm/instructions_arm_test.cc:32: // On the simulator, also try the ARMv6
version.
I do not understand this comment. Are you missing a test?

[zra, 2014-03-07 19:00:16]

Thanks!

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm.cc
File runtime/vm/assembler_arm.cc (right):

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1460: LoadLargeImmediate(rd, offset_hi, cond);
On 2014/03/06 17:32:11, regis wrote:
> I would simply call LoadImmediate here.

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1483: LoadLargeImmediate(rd, object_raw, cond);
On 2014/03/06 17:32:11, regis wrote:
> ditto

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1777: movt(IP, high);
On 2014/03/06 17:32:11, regis wrote:
> Call LoadPatchableImmediate independently of the arm version.
> LoadPatchableImmediate then checks the arm version.

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1901: buffer_.Load<int32_t>(position);
On 2014/03/06 17:32:11, regis wrote:
> You added  + 0 * Instr::kInstrSize below. For symmetry, you could add it here
> too.

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:1924: buffer_.Load<int32_t>(position);
On 2014/03/06 17:32:11, regis wrote:
> Here too? And below too? Not important.

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2182: }
On 2014/03/06 17:32:11, regis wrote:
> LoadPatchableImmediate for both. LoadPatchableImmediate checks the arm
version.

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2222: if (TargetCPUFeatures::arm_version() == ARMv6)
{
On 2014/03/06 17:32:11, regis wrote:
> See my comment in the header.

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2250: LoadLargeImmediate(rd, value, cond);
On 2014/03/06 17:32:11, regis wrote:
> LoadDecodableImmediate

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2440: LoadLargeImmediate(IP, value, cond);
On 2014/03/06 17:32:11, regis wrote:
> LoadDecodableImmediate

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2463: LoadLargeImmediate(IP, value, cond);
On 2014/03/06 17:32:11, regis wrote:
> LoadDecodableImmediate

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.cc:2486: LoadLargeImmediate(IP, value, cond);
On 2014/03/06 17:32:11, regis wrote:
> LoadDecodableImmediate

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm.h:594: void LoadLargeImmediate(Register rd, int32_t
value, Condition cond = AL);
On 2014/03/06 17:32:11, regis wrote:
> The function LoadImmediate below is also able to load a large immediate value.
> The key here is not that you load a large immediate, but that you can decode
the
> emitted instructions, or that you can patch the immediate value.
> Instead of adding LoadLargeImmediate, I would add 2 functions for clarity:
> LoadDecodableImmediate and LoadPatchableImmediate.
> On ARMv6, LoadDecodableImmediate would forward to LoadPatchableImmediate (same
> code as in your LoadLargeImmediate).
> On ARMv7, LoadDecodableImmediate would emit movw and optionally movt, whereas
> LoadPatchableImmediate would always emit both movw and movt.

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
File runtime/vm/assembler_arm_test.cc (right):

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm_test.cc:65: __ LoadLargeImmediate(R0, 0x12345678 << 1);
On 2014/03/06 17:32:11, regis wrote:
> LoadDecodableImmediate

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm_test.cc:78: __ bx(LR);
On 2014/03/06 17:32:11, regis wrote:
> __ bx(LR) should be outside of the branch.
> 
> Remove the version check anyway:
> 
> #else
>   __ LoadDecodableImmediate(R0, 0x12345678);
>   __ bx(LR);
> #endif

Done.

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/instructions_...
File runtime/vm/instructions_arm_test.cc (right):

https://codereview.chromium.org/183803024/diff/20001/runtime/vm/instructions_...
runtime/vm/instructions_arm_test.cc:32: // On the simulator, also try the ARMv6
version.
On 2014/03/06 17:32:11, regis wrote:
> I do not understand this comment. Are you missing a test?

Removed. I had a test here that switched to ARMv6 in the simulator, but the
generated code was the same, so I removed it.

[zra, 2014-03-07 19:17:41]

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