Speed up Binops

Speed up binary operations by considering HType info and fast-tracking constants.

BUG=

[Jakob Kummerow, 2013-07-18 11:56:14]

High-level comment: I consider HType deprecated, to be replaced by a Type (or
more likely a Bounds struct) being stored in every HValue. That makes most of
this change temporary code, and I'm not sure adding more machinery (your custom
Type/HType intersection function) to support this transient state. Maybe we
should do the HType->Type transition first, before adding more inference logic
to the Hydrogen typing system.

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen-instructions...
src/hydrogen-instructions.cc:466: if (IsSmi())     return
handle(::v8::internal::Type::Smi(), isolate);
This is not a generic implementation of intersection. At the very least, you
should ASSERT(type->Is(Type::Smi())).

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen-instructions...
src/hydrogen-instructions.h:367: Handle< ::v8::internal::Type> intersect(
Wouldn't it be easier to rename HType::Type to HType::InternalTypeEnum or
whatever so you don't need the namespace-qualified name here?

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc#newcode7899
src/hydrogen.cc:7899: ? graph()->GetConstant1() : graph()->GetConstant0();
nit: indentation

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc#newcode7941
src/hydrogen.cc:7941: left->KnownOptimalRepresentation() :
Representation::FromType(left_type);
nit: indentation. Also, prefer the '?' on the next line. Maybe:
  Representation left_rep = left->IsConstant()
      ? left->KnownOptimalRepresentation()
      : Representation::FromType(left_type);

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc#newcode7950
src/hydrogen.cc:7950: if (!left->type().IsString() || !right->type().IsString())
{
Why is this one atomic block rather than "if (!left is string) { add check for
left; }" and then the same for |right|?

https://codereview.chromium.org/19271008/diff/23001/src/types.h
File src/types.h (right):

https://codereview.chromium.org/19271008/diff/23001/src/types.h#newcode131
src/types.h:131: V(NonSmi,          kAny - kSmi)                                
  \
I'm not sure this is what you want. Type::Smi denotes a value that can be stored
as a Smi, i.e. a number in int31/int32 range (depending on platform). It is
therefore not the same as HType's "IsNonPrimitive() || IsJSObject()". I think
I'd prefer an explicit union of Types instead (likely just Receiver, Undefined).

[oliv, 2013-07-18 12:22:52]

> High-level comment: I consider HType deprecated, to be replaced by a Type (or
> more likely a Bounds struct) being stored in every HValue.

I agree in theory, but currently this is the only place where we have type
guarantees recorded and it would be a waste not to consume this information if
it is there already....

>
https://codereview.chromium.org/19271008/diff/23001/src/hydrogen-instructions...
> src/hydrogen-instructions.cc:466: if (IsSmi())     return
> handle(::v8::internal::Type::Smi(), isolate);
> This is not a generic implementation of intersection. At the very least, you
> should ASSERT(type->Is(Type::Smi())).

Ok maybe this should not be called intersect.
If HType says it is a smi, then this is unconditionally true. So if Type says
something else, then our typefeedback was just wrong, but it don't think we can
assert this...

[Toon Verwaest, 2013-07-18 13:57:12]

Added some more comments. Perhaps we could split up the CL into a part that
handles the HConstants, and the extra type inference?

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc#newcode7894
src/hydrogen.cc:7894: HValue* HGraphBuilder::ConstantToNumber(HValue* value) {
Perhaps you can also handle strings here.

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc#newcode7895
src/hydrogen.cc:7895: if (value->IsConstant()) {
Move this code together with HConstant::CopyToTruncatedInt32 as "CopyToDouble",
and perhaps extend CopyToTruncatedInt32 to also directly handle the special
values handled below.

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc#newcode7901
src/hydrogen.cc:7901: constant->handle()->IsTheHole()) {
Remove TheHole case.

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc#newcode7941
src/hydrogen.cc:7941: left->KnownOptimalRepresentation() :
Representation::FromType(left_type);
Could we calculate this in "type" and only at the end convert to Representation?
Now it's pretty schizophrenic mixing HType, Type and Representation.

https://codereview.chromium.org/19271008/diff/23001/src/hydrogen.cc#newcode7954
src/hydrogen.cc:7954: AddInstruction(HCheckInstanceType::NewIsString(right,
zone()));
I'd use canonicalization to eliminate CheckInstanceType(string) of values that
are known to be type().IsString(); rather than conditionally emit the
instructions.