[es6] Fix symbol comparison on some architectures

https://codereview.chromium.org/1125783002 did not handle all cases for some
architectures. These cases are now covered, and tests have been extended to
check them.

BUG=v8:4073
LOG=N

Review URL: https://codereview.chromium.org/1128143002

Cr-Commit-Position: refs/heads/master@{#28266}

diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc
index c69b382..6133281 100644 (file)
--- a/src/arm/code-stubs-arm.cc
+++ b/src/arm/code-stubs-arm.cc
@@ -253,7 +253,7 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm,
   if (cond == lt || cond == gt) {
     __ CompareObjectType(r0, r4, r4, FIRST_SPEC_OBJECT_TYPE);
     __ b(ge, slow);
-    __ CompareObjectType(r0, r4, r4, SYMBOL_TYPE);
+    __ cmp(r4, Operand(SYMBOL_TYPE));
     __ b(eq, slow);
   } else {
     __ CompareObjectType(r0, r4, r4, HEAP_NUMBER_TYPE);
@@ -262,6 +262,8 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm,
     if (cond != eq) {
       __ cmp(r4, Operand(FIRST_SPEC_OBJECT_TYPE));
       __ b(ge, slow);
+      __ cmp(r4, Operand(SYMBOL_TYPE));
+      __ b(eq, slow);
       // Normally here we fall through to return_equal, but undefined is
       // special: (undefined == undefined) == true, but
       // (undefined <= undefined) == false!  See ECMAScript 11.8.5.

diff --git a/src/arm64/code-stubs-arm64.cc b/src/arm64/code-stubs-arm64.cc
index f71822e..9ce5a05 100644 (file)
--- a/src/arm64/code-stubs-arm64.cc
+++ b/src/arm64/code-stubs-arm64.cc
@@ -221,19 +221,22 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm,
   // so we do the second best thing - test it ourselves.
   // They are both equal and they are not both Smis so both of them are not
   // Smis.  If it's not a heap number, then return equal.
+  Register right_type = scratch;
   if ((cond == lt) || (cond == gt)) {
-    __ JumpIfObjectType(right, scratch, scratch, FIRST_SPEC_OBJECT_TYPE, slow,
-                        ge);
-    __ JumpIfObjectType(right, scratch, scratch, SYMBOL_TYPE, slow, eq);
+    __ JumpIfObjectType(right, right_type, right_type, FIRST_SPEC_OBJECT_TYPE,
+                        slow, ge);
+    __ Cmp(right_type, SYMBOL_TYPE);
+    __ B(eq, slow);
   } else if (cond == eq) {
     __ JumpIfHeapNumber(right, &heap_number);
   } else {
-    Register right_type = scratch;
     __ JumpIfObjectType(right, right_type, right_type, HEAP_NUMBER_TYPE,
                         &heap_number);
     // Comparing JS objects with <=, >= is complicated.
     __ Cmp(right_type, FIRST_SPEC_OBJECT_TYPE);
     __ B(ge, slow);
+    __ Cmp(right_type, SYMBOL_TYPE);
+    __ B(eq, slow);
     // Normally here we fall through to return_equal, but undefined is
     // special: (undefined == undefined) == true, but
     // (undefined <= undefined) == false!  See ECMAScript 11.8.5.

diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc
index 484c81a..5b5e050 100644 (file)
--- a/src/mips/code-stubs-mips.cc
+++ b/src/mips/code-stubs-mips.cc
@@ -291,16 +291,16 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm,
   // so we do the second best thing - test it ourselves.
   // They are both equal and they are not both Smis so both of them are not
   // Smis. If it's not a heap number, then return equal.
+  __ GetObjectType(a0, t4, t4);
   if (cc == less || cc == greater) {
-    __ GetObjectType(a0, t4, t4);
     __ Branch(slow, greater, t4, Operand(FIRST_SPEC_OBJECT_TYPE));
     __ Branch(slow, eq, t4, Operand(SYMBOL_TYPE));
   } else {
-    __ GetObjectType(a0, t4, t4);
     __ Branch(&heap_number, eq, t4, Operand(HEAP_NUMBER_TYPE));
     // Comparing JS objects with <=, >= is complicated.
     if (cc != eq) {
     __ Branch(slow, greater, t4, Operand(FIRST_SPEC_OBJECT_TYPE));
+    __ Branch(slow, eq, t4, Operand(SYMBOL_TYPE));
       // Normally here we fall through to return_equal, but undefined is
       // special: (undefined == undefined) == true, but
       // (undefined <= undefined) == false!  See ECMAScript 11.8.5.

diff --git a/src/mips64/code-stubs-mips64.cc b/src/mips64/code-stubs-mips64.cc
index 0a872c1..fb9c4ee 100644 (file)
--- a/src/mips64/code-stubs-mips64.cc
+++ b/src/mips64/code-stubs-mips64.cc
@@ -287,16 +287,16 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm,
   // so we do the second best thing - test it ourselves.
   // They are both equal and they are not both Smis so both of them are not
   // Smis. If it's not a heap number, then return equal.
+  __ GetObjectType(a0, t0, t0);
   if (cc == less || cc == greater) {
-    __ GetObjectType(a0, t0, t0);
     __ Branch(slow, greater, t0, Operand(FIRST_SPEC_OBJECT_TYPE));
     __ Branch(slow, eq, t0, Operand(SYMBOL_TYPE));
   } else {
-    __ GetObjectType(a0, t0, t0);
     __ Branch(&heap_number, eq, t0, Operand(HEAP_NUMBER_TYPE));
     // Comparing JS objects with <=, >= is complicated.
     if (cc != eq) {
     __ Branch(slow, greater, t0, Operand(FIRST_SPEC_OBJECT_TYPE));
+    __ Branch(slow, eq, t0, Operand(SYMBOL_TYPE));
       // Normally here we fall through to return_equal, but undefined is
       // special: (undefined == undefined) == true, but
       // (undefined <= undefined) == false!  See ECMAScript 11.8.5.

diff --git a/src/ppc/code-stubs-ppc.cc b/src/ppc/code-stubs-ppc.cc
index 94a2ae0..abcad08 100644 (file)
--- a/src/ppc/code-stubs-ppc.cc
+++ b/src/ppc/code-stubs-ppc.cc
@@ -262,7 +262,7 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm, Label* slow,
   if (cond == lt || cond == gt) {
     __ CompareObjectType(r3, r7, r7, FIRST_SPEC_OBJECT_TYPE);
     __ bge(slow);
-    __ CompareObjectType(r3, r7, r7, SYMBOL_TYPE);
+    __ cmpi(r7, Operand(SYMBOL_TYPE));
     __ beq(slow);
   } else {
     __ CompareObjectType(r3, r7, r7, HEAP_NUMBER_TYPE);
@@ -271,6 +271,8 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm, Label* slow,
     if (cond != eq) {
       __ cmpi(r7, Operand(FIRST_SPEC_OBJECT_TYPE));
       __ bge(slow);
+      __ cmpi(r7, Operand(SYMBOL_TYPE));
+      __ beq(slow);
       // Normally here we fall through to return_equal, but undefined is
       // special: (undefined == undefined) == true, but
       // (undefined <= undefined) == false!  See ECMAScript 11.8.5.

diff --git a/test/mjsunit/es6/symbols.js b/test/mjsunit/es6/symbols.js
index 7203f67..05601bd 100644 (file)
--- a/test/mjsunit/es6/symbols.js
+++ b/test/mjsunit/es6/symbols.js
@@ -507,30 +507,22 @@ TestGetOwnPropertySymbolsOnPrimitives();
 
 
 function TestComparison() {
-  function f() {
-    var a = Symbol();
-    a < a;
-    a > a;
-    a <= a;
-    a >= a;
-  }
-
-  assertThrows(f, TypeError);
-  %OptimizeFunctionOnNextCall(f);
-  assertThrows(f, TypeError);
-  assertThrows(f, TypeError);
-
-  function g() {
-    var a = Symbol();
-    var b = Symbol();
-    a < b;
-    a > b;
-    a <= b;
-    a >= b;
-  }
-  assertThrows(g, TypeError);
-  %OptimizeFunctionOnNextCall(g);
-  assertThrows(g, TypeError);
-  assertThrows(g, TypeError);
+  function lt() { var a = Symbol(); var b = Symbol(); a < b; }
+  function gt() { var a = Symbol(); var b = Symbol(); a > b; }
+  function le() { var a = Symbol(); var b = Symbol(); a <= b; }
+  function ge() { var a = Symbol(); var b = Symbol(); a >= b; }
+  function lt_same() { var a = Symbol(); a < a; }
+  function gt_same() { var a = Symbol(); a > a; }
+  function le_same() { var a = Symbol(); a <= a; }
+  function ge_same() { var a = Symbol(); a >= a; }
+
+  var throwFuncs = [lt, gt, le, ge, lt_same, gt_same, le_same, ge_same];
+
+  for (var f of throwFuncs) {
+    assertThrows(f, TypeError);
+    %OptimizeFunctionOnNextCall(f);
+    assertThrows(f, TypeError);
+    assertThrows(f, TypeError);
+  }
 }
 TestComparison();