[compiler] Introduce initial StrictEqualStub.

[compiler] Introduce initial StrictEqualStub.

Initial version of a new StrictEqualStub written as TurboFan code stub,
that implements the full strict equality comparison and is usable for
both TurboFan and Ignition (and soon for the generic CompareIC case
too). The stub is not fully optimized yet, i.e. we still go to the
runtime for string comparisons, but that'll be addressed in a follow-up
CL.

R=yangguo@chromium.org

Committed: https://crrev.com/4acb492e145b39700bbbc36ff0da5830de50f814
Cr-Commit-Position: refs/heads/master@{#34423}

[Benedikt Meurer, 2016-03-02 09:41:51]

Hey Yang,

Here's an initial version for the StrictEqualStub.
Please take a look.

Thanks,
Benedikt

[Yang, 2016-03-02 10:29:16]

LGTM, but I got some comments.

https://codereview.chromium.org/1753173003/diff/1/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

https://codereview.chromium.org/1753173003/diff/1/src/arm/code-stubs-arm.cc#n...
src/arm/code-stubs-arm.cc:3189: __ sub(r0, r0, r1);
Whoa, this is kind of hacky here. In the original Runtime_StringEqual we assert
that the result can only be a smi-0 or a smi-1. This here returns smi-0 on
equal, but definitely not smi-1 on unequality. Does the CompareIC only care
whether the result is 0 or not?

https://codereview.chromium.org/1753173003/diff/1/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/1753173003/diff/1/src/code-stubs.cc#newcode484
src/code-stubs.cc:484: 
This is really hard to read... :P
I'd have preferred a comment, something like pseudo code:

if (lhs == rhs) {
  if (lhs->IsHeapNumber()) return HeapNumber::cast(lhs)->value() != NaN;
  return true;
}
if (!lhs->IsSmi()) {
  if (lhs->IsHeapNumber()) {
    if (rhs->IsSmi()) {
      return Smi::cast(rhs)->value() == HeapNumber::cast(lhs)->value();
    } else if (rhs->IsHeapNumber()) {
      return HeapNumber::cast(rhs)->value() == HeapNumber::cast(lhs)->value();
    } else {
      return false;
    }
  } else {
    if (rhs->IsSmi()) {
      return false;
    } else {
      if (lhs->IsString()) {
        if (rhs->IsString()) {
          return %StringEqual(lhs, rhs);
        } else {
          return false;
        }
      } else if (lhs->IsSimd128()) {
        if (rhs->IsSimd128()) {
          return %StrictEqual(lhs, rhs);
        }
      } else {
        return false;
      }
    }
  }
} else {
  if (rhs->IsSmi()) {
    return false;
  } else {
    if (rhs->IsHeapNumber()) {
      return Smi::cast(lhs)->value() == HeapNumber::cast(rhs)->value();
    } else {
      return false;
    }
  }
}

And maybe this works, but is simpler?

if (lhs == rhs) {
  if (lhs->IsHeapNumber()) return HeapNumber::cast(lhs)->value() != NaN;
  return true;
}

if (lhs->IsSmi()) swap(lhs, rhs);

// now only comparing: smi-smi, obj-smi, obj-obj
if (lhs->IsSmi()) {
  // rhs->IsSmi, but rhs != lhs.
  return false;
}

// now only comparing: obj-smi, obj-obj
if (rhs->IsSmi()) {
  if (lhs->IsHeapNumber()) {
    return Smi::cast(rhs)->value() == HeapNumber::cast(lhs)->value();
  } else {
    return false;
  }
}

// now only comparing: obj-obj
if (lhs->instance_type() != rhs->instance_type()) return false;

if (lhs->IsHeapNumber()) {
  return HeapNumber::cast(lhs)->value() == HeapNumber::cast(rhs)->value();
} else if (lhs->IsString()) {
  return %StringEqual(lhs, rhs);
} else if (lhs->IsSimd128()) {
  return %StrictEqual(lhs, rhs);
} else {
  return false;
}

[Benedikt Meurer, 2016-03-02 11:27:04]

https://codereview.chromium.org/1753173003/diff/1/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

https://codereview.chromium.org/1753173003/diff/1/src/arm/code-stubs-arm.cc#n...
src/arm/code-stubs-arm.cc:3189: __ sub(r0, r0, r1);
Yes, 0 is for EQUAL, everything else for NOT_EQUAL (CompareIC is weird).

https://codereview.chromium.org/1753173003/diff/1/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/1753173003/diff/1/src/code-stubs.cc#newcode484
src/code-stubs.cc:484: 
Added the pseudo-code thanks. The short-cut unfortunately doesn't work because
the instance types for Strings are different. And the swap would only introduce
Phis, which make it less readable (currently; hopefully a DSL will help).

[Benedikt Meurer, 2016-03-02 11:27:12]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2016-03-02 11:27:13]

The patchset sent to the CQ was uploaded after l-g-t-m from yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1753173003/#ps20001
(title: "Add pseudo-code.")

[commit-bot: I haz the power, 2016-03-02 11:27:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1753173003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1753173003/20001

[commit-bot: I haz the power, 2016-03-02 11:47:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-02 11:47:54]

Description was changed from

==========
[compiler] Introduce initial StrictEqualStub.

Initial version of a new StrictEqualStub written as TurboFan code stub,
that implements the full strict equality comparison and is usable for
both TurboFan and Ignition (and soon for the generic CompareIC case
too). The stub is not fully optimized yet, i.e. we still go to the
runtime for string comparisons, but that'll be addressed in a follow-up
CL.

R=yangguo@chromium.org
==========

to

==========
[compiler] Introduce initial StrictEqualStub.

Initial version of a new StrictEqualStub written as TurboFan code stub,
that implements the full strict equality comparison and is usable for
both TurboFan and Ignition (and soon for the generic CompareIC case
too). The stub is not fully optimized yet, i.e. we still go to the
runtime for string comparisons, but that'll be addressed in a follow-up
CL.

R=yangguo@chromium.org

Committed: https://crrev.com/4acb492e145b39700bbbc36ff0da5830de50f814
Cr-Commit-Position: refs/heads/master@{#34423}
==========

[commit-bot: I haz the power, 2016-03-02 11:47:55]

Patchset 2 (id:??) landed as
https://crrev.com/4acb492e145b39700bbbc36ff0da5830de50f814
Cr-Commit-Position: refs/heads/master@{#34423}