Add 64-bit stubs to call into the runtime and to call native functions.

Add 64-bit stubs to call into the runtime and to call native functions.
Fix 64-bit assembler bugs.
Committed: https://code.google.com/p/dart/source/detail?r=2206

[srdjan, 2011-12-06 17:37:44]

http://codereview.chromium.org/8818001/diff/1/runtime/vm/assembler_x64.cc
File runtime/vm/assembler_x64.cc (right):

http://codereview.chromium.org/8818001/diff/1/runtime/vm/assembler_x64.cc#new...
runtime/vm/assembler_x64.cc:555: void Assembler::cmpq(const Address& address,
Register reg) {
Could you add tests for the new operations/mode in assembler_test_x64?

[regis, 2011-12-06 21:41:52]

http://codereview.chromium.org/8818001/diff/1/runtime/vm/assembler_x64.cc
File runtime/vm/assembler_x64.cc (right):

http://codereview.chromium.org/8818001/diff/1/runtime/vm/assembler_x64.cc#new...
runtime/vm/assembler_x64.cc:555: void Assembler::cmpq(const Address& address,
Register reg) {
On 2011/12/06 17:37:44, srdjan wrote:
> Could you add tests for the new operations/mode in assembler_test_x64?

Done.

[Ivan Posva, 2011-12-07 08:34:51]

First round of comments.

-Ivan

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc
File runtime/vm/assembler_x64.cc (right):

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc#...
runtime/vm/assembler_x64.cc:359: EmitOperand(dst & 7, Operand(src));
Why is this necessary if you assert for dst <= XMM7?

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc#...
runtime/vm/assembler_x64.cc:369: EmitOperand(src & 7, Operand(dst));
ditto

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc#...
runtime/vm/assembler_x64.cc:844: EmitOperand(dst & 7, Operand(src));
What happens for registers greater than 8?

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc#...
runtime/vm/assembler_x64.cc:851: EmitOperand(reg & 7, Operand(reg));
ditto.

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.h
File runtime/vm/assembler_x64.h (right):

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.h#n...
runtime/vm/assembler_x64.h:27: __asm { mov current_sp, rsp }                    
                           \
This has certainly not been tested. Please replace it with UNIMPLEMENTED().

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/runtime_entry.h
File runtime/vm/runtime_entry.h (right):

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/runtime_entry.h#n...
runtime/vm/runtime_entry.h:38: static const Function& RegisterFakeFunction(const
char* name,
Please do not put testing code into non-test sources.

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/stack_frame_x64.cc
File runtime/vm/stack_frame_x64.cc (right):

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/stack_frame_x64.c...
runtime/vm/stack_frame_x64.cc:14: static const int kExitLinkOffsetInEntryFrame =
-8 * kWordSize;
Shouldn't this then refer to a constant used in the InvokeDartCode stub.

[regis, 2011-12-07 19:37:35]

First round of comments addressed and one question.
Thanks.

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc
File runtime/vm/assembler_x64.cc (right):

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc#...
runtime/vm/assembler_x64.cc:359: EmitOperand(dst & 7, Operand(src));
On 2011/12/07 08:34:51, Ivan Posva wrote:
> Why is this necessary if you assert for dst <= XMM7?

The first argument of EmitOperand is the 3-bit rm field of the instruction. The
'& 7' will remain after we support dst >= XMM8.

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc#...
runtime/vm/assembler_x64.cc:369: EmitOperand(src & 7, Operand(dst));
On 2011/12/07 08:34:51, Ivan Posva wrote:
> ditto

ditto

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc#...
runtime/vm/assembler_x64.cc:844: EmitOperand(dst & 7, Operand(src));
On 2011/12/07 08:34:51, Ivan Posva wrote:
> What happens for registers greater than 8?

Good catch. Emit of the REX byte was missing. Added test as well.

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.cc#...
runtime/vm/assembler_x64.cc:851: EmitOperand(reg & 7, Operand(reg));
On 2011/12/07 08:34:51, Ivan Posva wrote:
> ditto.

ditto

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.h
File runtime/vm/assembler_x64.h (right):

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/assembler_x64.h#n...
runtime/vm/assembler_x64.h:27: __asm { mov current_sp, rsp }                    
                           \
On 2011/12/07 08:34:51, Ivan Posva wrote:
> This has certainly not been tested. Please replace it with UNIMPLEMENTED().

As discussed, the compiler on Windows may dynamically align the stack. Removed
check on Windows here and in 32-bit as well.

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/runtime_entry.h
File runtime/vm/runtime_entry.h (right):

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/runtime_entry.h#n...
runtime/vm/runtime_entry.h:38: static const Function& RegisterFakeFunction(const
char* name,
On 2011/12/07 08:34:51, Ivan Posva wrote:
> Please do not put testing code into non-test sources.

Made the function global in runtime_entry_test.cc.

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/stack_frame_x64.cc
File runtime/vm/stack_frame_x64.cc (right):

http://codereview.chromium.org/8818001/diff/8001/runtime/vm/stack_frame_x64.c...
runtime/vm/stack_frame_x64.cc:14: static const int kExitLinkOffsetInEntryFrame =
-8 * kWordSize;
On 2011/12/07 08:34:51, Ivan Posva wrote:
> Shouldn't this then refer to a constant used in the InvokeDartCode stub.

Neither stub_code_<arch>.cc nor stack_frame_<arch>.cc have a corresponding
<arch>-dependent header. I could define this constant in constants_<arch>.h and
assert its value in both stub_code_<arch>.cc and stack_frame_<arch>.cc.
Any better idea?

[Ivan Posva, 2011-12-07 23:32:56]

LGTM -ip

http://codereview.chromium.org/8818001/diff/17001/runtime/vm/stub_code_x64.cc
File runtime/vm/stub_code_x64.cc (right):

http://codereview.chromium.org/8818001/diff/17001/runtime/vm/stub_code_x64.cc...
runtime/vm/stub_code_x64.cc:34: // to transition to dart VM code.
Dart VM

http://codereview.chromium.org/8818001/diff/17001/runtime/vm/stub_code_x64.cc...
runtime/vm/stub_code_x64.cc:125: // to transition to dart VM code.
Comment is confusing. We are transitioning into a native function here, correct?

http://codereview.chromium.org/8818001/diff/17001/runtime/vm/stub_code_x64.cc...
runtime/vm/stub_code_x64.cc:142: // Pass NativeArguments structure by value and
call runtime.
ditto

[regis, 2011-12-07 23:57:44]

Thanks

http://codereview.chromium.org/8818001/diff/17001/runtime/vm/stub_code_x64.cc
File runtime/vm/stub_code_x64.cc (right):

http://codereview.chromium.org/8818001/diff/17001/runtime/vm/stub_code_x64.cc...
runtime/vm/stub_code_x64.cc:34: // to transition to dart VM code.
On 2011/12/07 23:32:56, Ivan Posva wrote:
> Dart VM

Done here and in ia32 version.

http://codereview.chromium.org/8818001/diff/17001/runtime/vm/stub_code_x64.cc...
runtime/vm/stub_code_x64.cc:125: // to transition to dart VM code.
On 2011/12/07 23:32:56, Ivan Posva wrote:
> Comment is confusing. We are transitioning into a native function here,
correct?

Done.

http://codereview.chromium.org/8818001/diff/17001/runtime/vm/stub_code_x64.cc...
runtime/vm/stub_code_x64.cc:142: // Pass NativeArguments structure by value and
call runtime.
On 2011/12/07 23:32:56, Ivan Posva wrote:
> ditto

Done.