Adding unary op optimizations. missing assembly operations/

Adding unary op optimizations, missing assembly operations.
Committed: https://code.google.com/p/dart/source/detail?r=8174

[srdjan, 2012-05-30 21:58:17]

Note that this code includes excerpts from
http://codereview.chromium.org/10458031/ which needs to be submitted first.

[Florian Schneider, 2012-05-31 09:22:54]

LGTM with comments.

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

http://codereview.chromium.org/10440099/diff/1/runtime/vm/assembler_x64.cc#ne...
runtime/vm/assembler_x64.cc:1577: ASSERT(RawObject::kClassTagSize == 16);
These two would be candidates for a compile-time assert (STATIC_ASSERT).

http://codereview.chromium.org/10440099/diff/1/runtime/vm/assembler_x64.cc#ne...
runtime/vm/assembler_x64.cc:1590: movq(result, Address(result, class_id,
TIMES_8, 0));
Maybe add an assert: ASSERT(ClassTable::kEntrySize == 8)

http://codereview.chromium.org/10440099/diff/1/runtime/vm/intermediate_langua...
File runtime/vm/intermediate_language_x64.cc (right):

http://codereview.chromium.org/10440099/diff/1/runtime/vm/intermediate_langua...
runtime/vm/intermediate_language_x64.cc:1244: if (test_class.index() == kDouble)
{
Maybe it would make sense to split UnaryOp into two: UnarySmiOp and
UnaryDoubleOp. This way the instruction template gets shorter and you can
specify the precise number of registers needed for each: 1 temp for the double
case, 0 temps for the smi-case.

[regis, 2012-05-31 09:48:19]

LGTM

https://chromiumcodereview.appspot.com/10440099/diff/1/runtime/vm/assembler_x...
File runtime/vm/assembler_x64_test.cc (right):

https://chromiumcodereview.appspot.com/10440099/diff/1/runtime/vm/assembler_x...
runtime/vm/assembler_x64_test.cc:1249: 
Shouldn't you add tests for notl and xorpd?

https://chromiumcodereview.appspot.com/10440099/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.cc (right):

https://chromiumcodereview.appspot.com/10440099/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:549: // TODO(srdjan): Compute based on input
types (ICData).
I am not sure that StaticType will depend on ICData. We may instead have a
StaticType and a RuntimeType.
Whereas the RuntimeType would depend on the ICData, the StaticType would depend
on the StaticType of the operands.
The most specific of StaticType or RuntimeType would then be used to eliminate
type tests.
But you can leave the comment as is for now.

[srdjan, 2012-05-31 20:02:44]

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

http://codereview.chromium.org/10440099/diff/1/runtime/vm/assembler_x64.cc#ne...
runtime/vm/assembler_x64.cc:1577: ASSERT(RawObject::kClassTagSize == 16);
On 2012/05/31 09:22:54, Florian Schneider wrote:
> These two would be candidates for a compile-time assert (STATIC_ASSERT).

This code is not part of the CL, just borrowed from vegorov. However it would be
a good idea to add STATIC_ASSERTs to the VM.

http://codereview.chromium.org/10440099/diff/1/runtime/vm/assembler_x64.cc#ne...
runtime/vm/assembler_x64.cc:1590: movq(result, Address(result, class_id,
TIMES_8, 0));
On 2012/05/31 09:22:54, Florian Schneider wrote:
> Maybe add an assert: ASSERT(ClassTable::kEntrySize == 8)

Code is part of Slava's CL. CC-ing him

http://codereview.chromium.org/10440099/diff/1/runtime/vm/intermediate_langua...
File runtime/vm/intermediate_language_x64.cc (right):

http://codereview.chromium.org/10440099/diff/1/runtime/vm/intermediate_langua...
runtime/vm/intermediate_language_x64.cc:1244: if (test_class.index() == kDouble)
{
On 2012/05/31 09:22:54, Florian Schneider wrote:
> Maybe it would make sense to split UnaryOp into two: UnarySmiOp and
> UnaryDoubleOp. This way the instruction template gets shorter and you can
> specify the precise number of registers needed for each: 1 temp for the double
> case, 0 temps for the smi-case.

UnaryOpComp -> UnarySmiOpComp  and adding NumberNegateComp which should also
handle mixed int/double receivers (differentiate it from existing
BooleanNegateComp)

[srdjan, 2012-05-31 20:20:41]

https://chromiumcodereview.appspot.com/10440099/diff/1/runtime/vm/assembler_x...
File runtime/vm/assembler_x64_test.cc (right):

https://chromiumcodereview.appspot.com/10440099/diff/1/runtime/vm/assembler_x...
runtime/vm/assembler_x64_test.cc:1249: 
On 2012/05/31 09:48:19, regis wrote:
> Shouldn't you add tests for notl and xorpd?

Removed notl (it was never implemented), added test for xorpd.

https://chromiumcodereview.appspot.com/10440099/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.cc (right):

https://chromiumcodereview.appspot.com/10440099/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:549: // TODO(srdjan): Compute based on input
types (ICData).
On 2012/05/31 09:48:19, regis wrote:
> I am not sure that StaticType will depend on ICData. We may instead have a
> StaticType and a RuntimeType.
> Whereas the RuntimeType would depend on the ICData, the StaticType would
depend
> on the StaticType of the operands.
> The most specific of StaticType or RuntimeType would then be used to eliminate
> type tests.
> But you can leave the comment as is for now.

Changed to NumberInterface, which is the static type for this node.