Fix evaluation order of GT and LTE operators.

According to the ES5 spec all ">" and "<=" expressions should be be
evaluated left-to-right. This obsoletes old hacks for reversing the
order to be ES3 compliant.

R=lrn@chromium.org
BUG=v8:1752

Review URL: http://codereview.chromium.org/8275035

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9641 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index 353ce5b..1db296a 100644 (file)
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -4071,33 +4071,25 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
         case Token::EQ_STRICT:
         case Token::EQ:
           cond = eq;
-          __ pop(r1);
           break;
         case Token::LT:
           cond = lt;
-          __ pop(r1);
           break;
         case Token::GT:
-          // Reverse left and right sides to obtain ECMA-262 conversion order.
-          cond = lt;
-          __ mov(r1, result_register());
-          __ pop(r0);
+          cond = gt;
          break;
         case Token::LTE:
-          // Reverse left and right sides to obtain ECMA-262 conversion order.
-          cond = ge;
-          __ mov(r1, result_register());
-          __ pop(r0);
+          cond = le;
           break;
         case Token::GTE:
           cond = ge;
-          __ pop(r1);
           break;
         case Token::IN:
         case Token::INSTANCEOF:
         default:
           UNREACHABLE();
       }
+      __ pop(r1);
 
       bool inline_smi_code = ShouldInlineSmiCase(op);
       JumpPatchSite patch_site(masm_);

diff --git a/src/arm/ic-arm.cc b/src/arm/ic-arm.cc
index 6e0badc..353450d 100644 (file)
--- a/src/arm/ic-arm.cc
+++ b/src/arm/ic-arm.cc
@@ -1559,11 +1559,9 @@ Condition CompareIC::ComputeCondition(Token::Value op) {
     case Token::LT:
       return lt;
     case Token::GT:
-      // Reverse left and right operands to obtain ECMA-262 conversion order.
-      return lt;
+      return gt;
     case Token::LTE:
-      // Reverse left and right operands to obtain ECMA-262 conversion order.
-      return ge;
+      return le;
     case Token::GTE:
       return ge;
     default:

diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc
index 8495939..7da5296 100644 (file)
--- a/src/arm/lithium-arm.cc
+++ b/src/arm/lithium-arm.cc
@@ -1404,12 +1404,10 @@ LInstruction* LChunkBuilder::DoPower(HPower* instr) {
 
 
 LInstruction* LChunkBuilder::DoCompareGeneric(HCompareGeneric* instr) {
-  Token::Value op = instr->token();
   ASSERT(instr->left()->representation().IsTagged());
   ASSERT(instr->right()->representation().IsTagged());
-  bool reversed = (op == Token::GT || op == Token::LTE);
-  LOperand* left = UseFixed(instr->left(), reversed ? r0 : r1);
-  LOperand* right = UseFixed(instr->right(), reversed ? r1 : r0);
+  LOperand* left = UseFixed(instr->left(), r1);
+  LOperand* right = UseFixed(instr->right(), r0);
   LCmpT* result = new LCmpT(left, right);
   return MarkAsCall(DefineFixed(result, r0), instr);
 }

diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 4605735..42d1ec9 100644 (file)
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -2176,9 +2176,6 @@ void LCodeGen::DoCmpT(LCmpT* instr) {
   __ cmp(r0, Operand(0));  // This instruction also signals no smi code inlined.
 
   Condition condition = ComputeCompareCondition(op);
-  if (op == Token::GT || op == Token::LTE) {
-    condition = ReverseCondition(condition);
-  }
   __ LoadRoot(ToRegister(instr->result()),
               Heap::kTrueValueRootIndex,
               condition);

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index 33d5cab..3b0c79b 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -4147,33 +4147,25 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
         case Token::EQ_STRICT:
         case Token::EQ:
           cc = equal;
-          __ pop(edx);
           break;
         case Token::LT:
           cc = less;
-          __ pop(edx);
           break;
         case Token::GT:
-          // Reverse left and right sizes to obtain ECMA-262 conversion order.
-          cc = less;
-          __ mov(edx, result_register());
-          __ pop(eax);
+          cc = greater;
          break;
         case Token::LTE:
-          // Reverse left and right sizes to obtain ECMA-262 conversion order.
-          cc = greater_equal;
-          __ mov(edx, result_register());
-          __ pop(eax);
+          cc = less_equal;
           break;
         case Token::GTE:
           cc = greater_equal;
-          __ pop(edx);
           break;
         case Token::IN:
         case Token::INSTANCEOF:
         default:
           UNREACHABLE();
       }
+      __ pop(edx);
       decrement_stack_height();
 
       bool inline_smi_code = ShouldInlineSmiCase(op);

diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc
index 8a98b17..57c9982 100644 (file)
--- a/src/ia32/ic-ia32.cc
+++ b/src/ia32/ic-ia32.cc
@@ -1591,11 +1591,9 @@ Condition CompareIC::ComputeCondition(Token::Value op) {
     case Token::LT:
       return less;
     case Token::GT:
-      // Reverse left and right operands to obtain ECMA-262 conversion order.
-      return less;
+      return greater;
     case Token::LTE:
-      // Reverse left and right operands to obtain ECMA-262 conversion order.
-      return greater_equal;
+      return less_equal;
     case Token::GTE:
       return greater_equal;
     default:

diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index 35fa618..51e0828 100644 (file)
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -2029,9 +2029,6 @@ void LCodeGen::DoCmpT(LCmpT* instr) {
   CallCode(ic, RelocInfo::CODE_TARGET, instr);
 
   Condition condition = ComputeCompareCondition(op);
-  if (op == Token::GT || op == Token::LTE) {
-    condition = ReverseCondition(condition);
-  }
   Label true_value, done;
   __ test(eax, Operand(eax));
   __ j(condition, &true_value, Label::kNear);

diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc
index 856106c..997f17e 100644 (file)
--- a/src/ia32/lithium-ia32.cc
+++ b/src/ia32/lithium-ia32.cc
@@ -1434,13 +1434,11 @@ LInstruction* LChunkBuilder::DoPower(HPower* instr) {
 
 
 LInstruction* LChunkBuilder::DoCompareGeneric(HCompareGeneric* instr) {
-  Token::Value op = instr->token();
   ASSERT(instr->left()->representation().IsTagged());
   ASSERT(instr->right()->representation().IsTagged());
-  bool reversed = (op == Token::GT || op == Token::LTE);
   LOperand* context = UseFixed(instr->context(), esi);
-  LOperand* left = UseFixed(instr->left(), reversed ? eax : edx);
-  LOperand* right = UseFixed(instr->right(), reversed ? edx : eax);
+  LOperand* left = UseFixed(instr->left(), edx);
+  LOperand* right = UseFixed(instr->right(), eax);
   LCmpT* result = new LCmpT(context, left, right);
   return MarkAsCall(DefineFixed(result, eax), instr);
 }

diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index b5c5fc5..39dfe84 100644 (file)
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -3997,33 +3997,25 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
         case Token::EQ_STRICT:
         case Token::EQ:
           cc = equal;
-          __ pop(rdx);
           break;
         case Token::LT:
           cc = less;
-          __ pop(rdx);
           break;
         case Token::GT:
-          // Reverse left and right sizes to obtain ECMA-262 conversion order.
-          cc = less;
-          __ movq(rdx, result_register());
-          __ pop(rax);
+          cc = greater;
          break;
         case Token::LTE:
-          // Reverse left and right sizes to obtain ECMA-262 conversion order.
-          cc = greater_equal;
-          __ movq(rdx, result_register());
-          __ pop(rax);
+          cc = less_equal;
           break;
         case Token::GTE:
           cc = greater_equal;
-          __ pop(rdx);
           break;
         case Token::IN:
         case Token::INSTANCEOF:
         default:
           UNREACHABLE();
       }
+      __ pop(rdx);
 
       bool inline_smi_code = ShouldInlineSmiCase(op);
       JumpPatchSite patch_site(masm_);

diff --git a/src/x64/ic-x64.cc b/src/x64/ic-x64.cc
index 27a9667..c79d44d 100644 (file)
--- a/src/x64/ic-x64.cc
+++ b/src/x64/ic-x64.cc
@@ -1613,11 +1613,9 @@ Condition CompareIC::ComputeCondition(Token::Value op) {
     case Token::LT:
       return less;
     case Token::GT:
-      // Reverse left and right operands to obtain ECMA-262 conversion order.
-      return less;
+      return greater;
     case Token::LTE:
-      // Reverse left and right operands to obtain ECMA-262 conversion order.
-      return greater_equal;
+      return less_equal;
     case Token::GTE:
       return greater_equal;
     default:

diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index 6b3a73a..8d806e6 100644 (file)
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -1979,9 +1979,6 @@ void LCodeGen::DoCmpT(LCmpT* instr) {
   CallCode(ic, RelocInfo::CODE_TARGET, instr);
 
   Condition condition = TokenToCondition(op, false);
-  if (op == Token::GT || op == Token::LTE) {
-    condition = ReverseCondition(condition);
-  }
   Label true_value, done;
   __ testq(rax, rax);
   __ j(condition, &true_value, Label::kNear);

diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc
index a67a593..8c21c2d 100644 (file)
--- a/src/x64/lithium-x64.cc
+++ b/src/x64/lithium-x64.cc
@@ -1396,12 +1396,10 @@ LInstruction* LChunkBuilder::DoPower(HPower* instr) {
 
 
 LInstruction* LChunkBuilder::DoCompareGeneric(HCompareGeneric* instr) {
-  Token::Value op = instr->token();
   ASSERT(instr->left()->representation().IsTagged());
   ASSERT(instr->right()->representation().IsTagged());
-  bool reversed = (op == Token::GT || op == Token::LTE);
-  LOperand* left = UseFixed(instr->left(), reversed ? rax : rdx);
-  LOperand* right = UseFixed(instr->right(), reversed ? rdx : rax);
+  LOperand* left = UseFixed(instr->left(), rdx);
+  LOperand* right = UseFixed(instr->right(), rax);
   LCmpT* result = new LCmpT(left, right);
   return MarkAsCall(DefineFixed(result, rax), instr);
 }

diff --git a/test/mjsunit/compiler/compare.js b/test/mjsunit/compiler/compare.js
index 3f96087..460b0ab 100644 (file)
--- a/test/mjsunit/compiler/compare.js
+++ b/test/mjsunit/compiler/compare.js
@@ -83,9 +83,9 @@ function TestNonPrimitive(order, f) {
 }
 
 TestNonPrimitive("xy", MaxLT);
-TestNonPrimitive("yx", MaxLE);
+TestNonPrimitive("xy", MaxLE);
 TestNonPrimitive("xy", MaxGE);
-TestNonPrimitive("yx", MaxGT);
+TestNonPrimitive("xy", MaxGT);
 
 // Test compare in case of aliased registers.
 function CmpX(x) { if (x == x) return 42; }

diff --git a/test/mjsunit/to_number_order.js b/test/mjsunit/to_number_order.js
index d17e600..50e4bc7 100644 (file)
--- a/test/mjsunit/to_number_order.js
+++ b/test/mjsunit/to_number_order.js
@@ -161,7 +161,7 @@ assertEquals("fiskfisk", x, "Compare objects b >= b valueOf order");
 
 x = "";
 assertFalse(a > b, "Compare objects a > b");
-assertEquals("fiskhest", x, "Compare objects a > b valueOf order");
+assertEquals("hestfisk", x, "Compare objects a > b valueOf order");
 
 x = "";
 assertFalse(a > void(0), "Compare objects a > undefined");
@@ -195,7 +195,7 @@ function identical_object_comparison() {
 
   x = "";
   assertFalse(a > b, "Compare objects a > b");
-  assertEquals("fiskhest", x, "Compare objects a > b valueOf order");
+  assertEquals("hestfisk", x, "Compare objects a > b valueOf order");
 
   x = "";
   assertFalse(a > void(0), "Compare objects a > undefined");

diff --git a/test/mozilla/mozilla.status b/test/mozilla/mozilla.status
index 6a5c086..6d3d0ca 100644 (file)
--- a/test/mozilla/mozilla.status
+++ b/test/mozilla/mozilla.status
@@ -410,12 +410,6 @@ js1_5/extensions/regress-435345-01: FAIL_OK
 js1_5/extensions/regress-455413: FAIL_OK
 
 
-# The spec specifies reverse evaluation order for < and >=.
-# See section 11.8.2 and 11.8.5.
-# We implement the spec here but the test tests the more straigtforward order.
-ecma_3/Operators/order-01: FAIL_OK
-
-
 # Uses Mozilla-specific QName, XML, XMLList and Iterator.
 js1_5/Regress/regress-407323: FAIL_OK
 js1_5/Regress/regress-407957: FAIL_OK

diff --git a/test/sputnik/sputnik.status b/test/sputnik/sputnik.status
index 99db598..ac02d25 100644 (file)
--- a/test/sputnik/sputnik.status
+++ b/test/sputnik/sputnik.status
@@ -162,6 +162,10 @@ S11.1.5_A4.2: FAIL_OK
 S9.9_A1: FAIL_OK
 S9.9_A2: FAIL_OK
 
+# The expected evaluation order of comparison operations changed.
+S11.8.2_A2.3_T1: FAIL_OK
+S11.8.3_A2.3_T1: FAIL_OK
+
 # Calls builtins without an explicit receiver which means that
 # undefined is passed to the builtin. The tests expect the global
 # object to be passed which was true in ES3 but not in ES5.

diff --git a/test/test262/test262.status b/test/test262/test262.status
index 1a61954..f871dd0 100644 (file)
--- a/test/test262/test262.status
+++ b/test/test262/test262.status
@@ -43,19 +43,6 @@ S8.7_A5_T2: FAIL
 # V8 Bug: http://code.google.com/p/v8/issues/detail?id=1624
 S10.4.2.1_A1: FAIL
 
-# V8 Bug: http://code.google.com/p/v8/issues/detail?id=1752
-S11.8.2_A2.3_T1: FAIL
-S11.8.3_A2.3_T1: FAIL
-11.8.2-1: FAIL
-11.8.2-2: FAIL
-11.8.2-3: FAIL
-11.8.2-4: FAIL
-11.8.3-1: FAIL
-11.8.3-2: FAIL
-11.8.3-3: FAIL
-11.8.3-4: FAIL
-11.8.3-5: FAIL
-
 # V8 Bug.
 S13.2.3_A1: FAIL