A few fixes for ARMv5TE.

A few fixes for ARMv5TE.

This CL:
- Adds SupportsHardwareDivision() to FlowGraphCompiler.
  In FlowGraphOptimizer, if SupportsHardwareDivision()
  is false, Smi and Mint division operations are not
  inlined.
- Reduces the supported addressing modes for ldrd, strd.
  For ARMv6 and up, the address only needs to be 4-byte
  aligned, for ARMv5TE and below the address must be
  8-byte aligned. So, on ARMv5TE, we can split these
  each into two instructions, which is easier to do if
  the addressing modes are restricted to base + offset,
  which is all we were using anyway.
- Uses the smull instruction on ARMv5TE and ARMv6. I don't
  remember why it was disabled, and the docs say
  it is supported. Verified working on ARMv5TE hardware.
- Fixes a bug in OSR frame entry caused by the
  difference between the PC read offset in store vs.
  other instructions.
- Fixes assembler tests that have a float return for
  targets without vfp.

R=regis@google.com

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

[zra, 2015-03-30 17:25:00]

zra@google.com changed reviewers:
+ regis@google.com, srdjan@google.com

[zra, 2015-03-30 17:25:01]

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/os_linux.cc#newc...
runtime/vm/os_linux.cc:511: asm("svc #0x9f0001");  // __ARM_NR_breakpoint
This is not a valid thumb instruction.

[srdjan, 2015-03-30 17:39:50]

LGTMwC

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
File runtime/vm/assembler_arm_test.cc (right):

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
runtime/vm/assembler_arm_test.cc:475: } else {
Maybe you just want to do nothing instead of running dummy code?

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
runtime/vm/assembler_arm_test.cc:1917: __ LoadImmediate(R0, 3);
ditto

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
runtime/vm/assembler_arm_test.cc:1946: __ LoadImmediate(R0, 3);
ditto

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
runtime/vm/assembler_arm_test.cc:3263: EXPECT_FLOAT_EQ(8.0, res, 0.0001f);
ditto

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/intermediate_lan...
File runtime/vm/intermediate_language_arm.cc (right):

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/intermediate_lan...
runtime/vm/intermediate_language_arm.cc:3243: } else {
You cold add an ASSERT for ARM version (and other places below).

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/os_linux.cc#newc...
runtime/vm/os_linux.cc:511: asm("svc #0x9f0001");  // __ARM_NR_breakpoint
On 2015/03/30 17:25:00, zra wrote:
> This is not a valid thumb instruction.

Is there a breakpoint instruction in thumb instruction set?

[regis, 2015-03-30 18:12:43]

LGTM

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/os_linux.cc#newc...
runtime/vm/os_linux.cc:515: UNIMPLEMENTED();
I am wondering how you figured out that this instruction was not available on
Thumb, since we only call this OS::DebugBreak() from the simulator to enter gdb
on Intel platforms.
As Srdjan indicates, an UNIMPLEMENTED is not really solving the problem. On the
other hand, there is no problem to solve so far :-)

Apparently, this is also called from a vm test "CodeImmutability", which I do
not understand.

I think some TODOs to remember to clean up this mess would be good.

[zra, 2015-03-30 20:02:06]

Thanks!

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
File runtime/vm/assembler_arm_test.cc (right):

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
runtime/vm/assembler_arm_test.cc:475: } else {
On 2015/03/30 17:39:50, srdjan wrote:
> Maybe you just want to do nothing instead of running dummy code?

Changed here, and used the same pattern for all neon, vfp, and division tests.

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
runtime/vm/assembler_arm_test.cc:1917: __ LoadImmediate(R0, 3);
On 2015/03/30 17:39:50, srdjan wrote:
> ditto

Done.

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
runtime/vm/assembler_arm_test.cc:1946: __ LoadImmediate(R0, 3);
On 2015/03/30 17:39:50, srdjan wrote:
> ditto

Done.

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm_te...
runtime/vm/assembler_arm_test.cc:3263: EXPECT_FLOAT_EQ(8.0, res, 0.0001f);
On 2015/03/30 17:39:50, srdjan wrote:
> ditto

Done.

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/intermediate_lan...
File runtime/vm/intermediate_language_arm.cc (right):

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/intermediate_lan...
runtime/vm/intermediate_language_arm.cc:3243: } else {
On 2015/03/30 17:39:50, srdjan wrote:
> You cold add an ASSERT for ARM version (and other places below).

smull() is supported on all versions.

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/os_linux.cc#newc...
runtime/vm/os_linux.cc:511: asm("svc #0x9f0001");  // __ARM_NR_breakpoint
On 2015/03/30 17:39:50, srdjan wrote:
> On 2015/03/30 17:25:00, zra wrote:
> > This is not a valid thumb instruction.
> 
> Is there a breakpoint instruction in thumb instruction set?

After cleanup, only the Intel instruction should be needed.

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/os_linux.cc#newc...
runtime/vm/os_linux.cc:515: UNIMPLEMENTED();
On 2015/03/30 18:12:43, regis wrote:
> I am wondering how you figured out that this instruction was not available on
> Thumb, since we only call this OS::DebugBreak() from the simulator to enter
gdb
> on Intel platforms.

gcc refuses to compile the inline assembly if you pass -mthumb.

> As Srdjan indicates, an UNIMPLEMENTED is not really solving the problem. On
the
> other hand, there is no problem to solve so far :-)
> 
> Apparently, this is also called from a vm test "CodeImmutability", which I do
> not understand.
> 
> I think some TODOs to remember to clean up this mess would be good.

TODO added.

[zra, 2015-03-30 20:47:07]

Committed patchset #2 (id:20001) manually as r44788.

[Vyacheslav Egorov (Google), 2015-03-31 08:05:29]

vegorov@google.com changed reviewers:
+ vegorov@google.com

[Vyacheslav Egorov (Google), 2015-03-31 08:05:30]

DBC

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

https://codereview.chromium.org/1043943002/diff/1/runtime/vm/assembler_arm.cc...
runtime/vm/assembler_arm.cc:3277: (TargetCPUFeatures::store_pc_read_offset() -
Instr::kPCReadOffset);
I find this comment and expression really confusing because store_pc_read_offset
does not explicitly surface in any expression below (instead it affects
EnterDartFrame above).

How about rewriting this expression to 

  CodeSize() + Instr::kPCReadOffset - EntryPointToPcMarkerOffset()

I think that'd be much easier to figure out.

Then the comment can just say that mov(IP, Operand(PC)) loads PC +
Instr::kPCRead (8)