Intrinsify UserTag operations on all architectures

Intrinsify UserTag operations on all architectures

R=srdjan@google.com, zra@google.com

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

[Cutch, 2014-04-14 14:23:57]

+zra for ARM/MIPS.

[srdjan, 2014-04-14 15:08:27]

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

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_ia32...
runtime/vm/intrinsifier_ia32.cc:1747: __ leal(EBX,
Address::Absolute(reinterpret_cast<uword>(isolate)));
Couldn't you get the address of current_tag here directly? Also for other
architectures.

[zra, 2014-04-14 15:31:29]

What is the performance impact?

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_arm6...
File runtime/vm/intrinsifier_arm64.cc (right):

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_arm6...
runtime/vm/intrinsifier_arm64.cc:406: void
Intrinsifier::UserTag_makeCurrent(Assembler* assembler) {
Thanks for remembering to add these!

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

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_ia32...
runtime/vm/intrinsifier_ia32.cc:1747: __ leal(EBX,
Address::Absolute(reinterpret_cast<uword>(isolate)));
On 2014/04/14 15:08:27, srdjan wrote:
> Couldn't you get the address of current_tag here directly? Also for other
> architectures.

At least on ARM and MIPS, I don't think it would save any instructions, and this
way is consistent with how we load object fields in other places.

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

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_mips...
runtime/vm/intrinsifier_mips.cc:1783: // V1: Isolate.
(optional) Maybe use T0 or T1 instead of V1 for consistency. Doesn't really
matter though.

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_mips...
runtime/vm/intrinsifier_mips.cc:1821: __ mov(T0, ZR);
Remove.

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_mips...
runtime/vm/intrinsifier_mips.cc:1822: __ sw(T0, Address(V1,
Isolate::user_tag_offset()));
__ sw(ZR, ...);

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_mips...
runtime/vm/intrinsifier_mips.cc:1823: __ Ret();
You can also use the delay slot here.

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

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:1668:
Immediate(reinterpret_cast<int64_t>(Object::null()));
__ LoadObject(RAX, Object::null_object(), PP);

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:1696:
Immediate(reinterpret_cast<int64_t>(UserTag::null()));
Use LoadObject here, too.

[Cutch, 2014-04-15 14:36:42]

Results from a simple micro-benchmark that performs 100,000,000 makeCurrent and
clearCurrentTag operations:

before: 8.07 seconds
after: 0.587 seconds

~13x speedup.

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

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_ia32...
runtime/vm/intrinsifier_ia32.cc:1747: __ leal(EBX,
Address::Absolute(reinterpret_cast<uword>(isolate)));
On 2014/04/14 15:08:27, srdjan wrote:
> Couldn't you get the address of current_tag here directly? Also for other
> architectures.

Done on IA32.

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

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_mips...
runtime/vm/intrinsifier_mips.cc:1783: // V1: Isolate.
On 2014/04/14 15:31:29, zra wrote:
> (optional) Maybe use T0 or T1 instead of V1 for consistency. Doesn't really
> matter though.

Done here and elsewhere.

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_mips...
runtime/vm/intrinsifier_mips.cc:1821: __ mov(T0, ZR);
On 2014/04/14 15:31:29, zra wrote:
> Remove.

Done.

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_mips...
runtime/vm/intrinsifier_mips.cc:1823: __ Ret();
On 2014/04/14 15:31:29, zra wrote:
> You can also use the delay slot here.

Done.

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

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:1668:
Immediate(reinterpret_cast<int64_t>(Object::null()));
On 2014/04/14 15:31:29, zra wrote:
> __ LoadObject(RAX, Object::null_object(), PP);

Done.

https://codereview.chromium.org/237063004/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:1696:
Immediate(reinterpret_cast<int64_t>(UserTag::null()));
On 2014/04/14 15:31:29, zra wrote:
> Use LoadObject here, too.

Done.

[zra, 2014-04-15 16:10:56]

arm & mips lgtm

[srdjan, 2014-04-18 18:28:40]

lgtm

[Cutch, 2014-04-18 18:46:00]

On 2014/04/18 18:28:40, srdjan wrote:
> lgtm

Patch Set 3 includes the final change requested.

[Cutch, 2014-04-18 18:56:15]

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