Bug 1538692 - Part 2: Support relational string comparison in CacheIR. r=mgaudet
authorAndré Bargull <andre.bargull@gmail.com>
Tue, 26 Mar 2019 14:49:35 +0000
changeset 466218 e30a80c7854ff12c5c792a10402eed2d4d30c56e
parent 466217 998ea689fe149c52983e32af9e7cf50a8cc95c37
child 466219 3697baa3225394badb3c53571f4d5870ed002385
push id81482
push userccoroiu@mozilla.com
push dateTue, 26 Mar 2019 19:52:14 +0000
treeherderautoland@3697baa32253 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgaudet
bugs1538692
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1538692 - Part 2: Support relational string comparison in CacheIR. r=mgaudet Differential Revision: https://phabricator.services.mozilla.com/D24707
js/src/jit-test/tests/cacheir/compare.js
js/src/jit/BaselineCacheIRCompiler.cpp
js/src/jit/CacheIR.cpp
js/src/jit/IonCacheIRCompiler.cpp
--- a/js/src/jit-test/tests/cacheir/compare.js
+++ b/js/src/jit-test/tests/cacheir/compare.js
@@ -219,8 +219,65 @@ warmup(String_Number_NEQ1, [["1.4",2, tr
 
 var String_Number_SEQ1 = (a, b) => { return a === b; }
 warmup(String_Number_SEQ1, [["1.4",2, false],  ["1",1, false], [3,"4", false],
                             [-1024, "-1023", false], [1.2, "1.2", false]]);
 
 var String_Number_SNEQ1 = (a, b) => { return a !== b; }
 warmup(String_Number_SNEQ1, [["1.4",2, true], ["1",1, true], [3,"4", true],
                              [-1024, "-1023", true], [1.2, "1.2", true]]);
+
+// String + String
+var String_String_GT1 = (a, b) => a > b;
+warmup(String_String_GT1, [["", "", false], ["a", "a", false], ["aa", "aa", false],
+                           ["a", "", true], ["", "a", false],
+                           ["a", "b", false], ["b", "a", true],
+                           ["a", "ab", false], ["ab", "a", true],
+                          ]);
+
+var String_String_GE1 = (a, b) => a >= b;
+warmup(String_String_GE1, [["", "", true], ["a", "a", true], ["aa", "aa", true],
+                           ["a", "", true], ["", "a", false],
+                           ["a", "b", false], ["b", "a", true],
+                           ["a", "ab", false], ["ab", "a", true],
+                          ]);
+
+var String_String_LT1 = (a, b) => a < b;
+warmup(String_String_LT1, [["", "", false], ["a", "a", false], ["aa", "aa", false],
+                           ["a", "", false], ["", "a", true],
+                           ["a", "b", true], ["b", "a", false],
+                           ["a", "ab", true], ["ab", "a", false],
+                          ]);
+
+var String_String_LE1 = (a, b) => a <= b;
+warmup(String_String_LE1, [["", "", true], ["a", "a", true], ["aa", "aa", true],
+                           ["a", "", false], ["", "a", true],
+                           ["a", "b", true], ["b", "a", false],
+                           ["a", "ab", true], ["ab", "a", false],
+                          ]);
+
+var String_String_EQ1 = (a, b) => a == b;
+warmup(String_String_EQ1, [["", "", true], ["a", "a", true], ["aa", "aa", true],
+                           ["a", "", false], ["", "a", false],
+                           ["a", "b", false], ["b", "a", false],
+                           ["a", "ab", false], ["ab", "a", false],
+                          ]);
+
+var String_String_SEQ1 = (a, b) => a === b;
+warmup(String_String_SEQ1, [["", "", true], ["a", "a", true], ["aa", "aa", true],
+                            ["a", "", false], ["", "a", false],
+                            ["a", "b", false], ["b", "a", false],
+                            ["a", "ab", false], ["ab", "a", false],
+                           ]);
+
+var String_String_NE1 = (a, b) => a != b;
+warmup(String_String_NE1, [["", "", false], ["a", "a", false], ["aa", "aa", false],
+                           ["a", "", true], ["", "a", true],
+                           ["a", "b", true], ["b", "a", true],
+                           ["a", "ab", true], ["ab", "a", true],
+                          ]);
+
+var String_String_SNE1 = (a, b) => a !== b;
+warmup(String_String_SNE1, [["", "", false], ["a", "a", false], ["aa", "aa", false],
+                            ["a", "", true], ["", "a", true],
+                            ["a", "b", true], ["b", "a", true],
+                            ["a", "ab", true], ["ab", "a", true],
+                           ]);
--- a/js/src/jit/BaselineCacheIRCompiler.cpp
+++ b/js/src/jit/BaselineCacheIRCompiler.cpp
@@ -918,24 +918,37 @@ bool BaselineCacheIRCompiler::emitCompar
   Label slow, done;
   masm.compareStrings(op, left, right, scratch, &slow);
   masm.jump(&done);
   masm.bind(&slow);
   {
     AutoStubFrame stubFrame(*this);
     stubFrame.enter(masm, scratch);
 
-    masm.Push(right);
-    masm.Push(left);
+    // Push the operands in reverse order for JSOP_LE and JSOP_GT:
+    // - |left <= right| is implemented as |right >= left|.
+    // - |left > right| is implemented as |right < left|.
+    if (op == JSOP_LE || op == JSOP_GT) {
+      masm.Push(left);
+      masm.Push(right);
+    } else {
+      masm.Push(right);
+      masm.Push(left);
+    }
 
     using Fn = bool (*)(JSContext*, HandleString, HandleString, bool*);
     if (op == JSOP_EQ || op == JSOP_STRICTEQ) {
       callVM<Fn, jit::StringsEqual<true>>(masm);
+    } else if (op == JSOP_NE || op == JSOP_STRICTNE) {
+      callVM<Fn, jit::StringsEqual<false>>(masm);
+    } else if (op == JSOP_LT || op == JSOP_GT) {
+      callVM<Fn, jit::StringsCompare<true>>(masm);
     } else {
-      callVM<Fn, jit::StringsEqual<false>>(masm);
+      MOZ_ASSERT(op == JSOP_LE || op == JSOP_GE);
+      callVM<Fn, jit::StringsCompare<false>>(masm);
     }
 
     stubFrame.leave(masm);
     masm.mov(ReturnReg, scratch);
   }
   masm.bind(&done);
   masm.tagValue(JSVAL_TYPE_BOOLEAN, scratch, output.valueReg());
   return true;
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -5460,18 +5460,16 @@ CompareIRGenerator::CompareIRGenerator(J
                                        HandleValue rhsVal)
     : IRGenerator(cx, script, pc, CacheKind::Compare, mode),
       op_(op),
       lhsVal_(lhsVal),
       rhsVal_(rhsVal) {}
 
 bool CompareIRGenerator::tryAttachString(ValOperandId lhsId,
                                          ValOperandId rhsId) {
-  MOZ_ASSERT(IsEqualityOp(op_));
-
   if (!lhsVal_.isString() || !rhsVal_.isString()) {
     return false;
   }
 
   StringOperandId lhsStrId = writer.guardIsString(lhsId);
   StringOperandId rhsStrId = writer.guardIsString(rhsId);
   writer.compareStringResult(op_, lhsStrId, rhsStrId);
   writer.returnFromIC();
@@ -5756,19 +5754,16 @@ bool CompareIRGenerator::tryAttachStub()
   ValOperandId rhsId(writer.setInputOperandId(rhsIndex));
 
   // For sloppy equality ops, there are cases this IC does not handle:
   // - {Symbol} x {Null, Undefined, String, Bool, Number}.
   // - {String} x {Null, Undefined, Symbol, Bool, Number}.
   // - {Bool} x {Double}.
   // - {Object} x {String, Symbol, Bool, Number}.
   if (IsEqualityOp(op_)) {
-    if (tryAttachString(lhsId, rhsId)) {
-      return true;
-    }
     if (tryAttachObject(lhsId, rhsId)) {
       return true;
     }
     if (tryAttachSymbol(lhsId, rhsId)) {
       return true;
     }
 
     // Handle the special case of Object compared to null/undefined.
@@ -5805,16 +5800,19 @@ bool CompareIRGenerator::tryAttachStub()
   // We want these to be last, to allow us to bypass the
   // strictly-different-types cases in the below attachment code
   if (tryAttachInt32(lhsId, rhsId)) {
     return true;
   }
   if (tryAttachNumber(lhsId, rhsId)) {
     return true;
   }
+  if (tryAttachString(lhsId, rhsId)) {
+    return true;
+  }
 
   if (tryAttachStringNumber(lhsId, rhsId)) {
     return true;
   }
 
   trackAttached(IRGenerator::NotAttached);
   return false;
 }
--- a/js/src/jit/IonCacheIRCompiler.cpp
+++ b/js/src/jit/IonCacheIRCompiler.cpp
@@ -1317,24 +1317,38 @@ bool IonCacheIRCompiler::emitCompareStri
   Label slow, done;
   MOZ_ASSERT(!output.hasValue());
   masm.compareStrings(op, left, right, output.typedReg().gpr(), &slow);
 
   masm.jump(&done);
   masm.bind(&slow);
 
   prepareVMCall(masm, save);
-  masm.Push(right);
-  masm.Push(left);
+
+  // Push the operands in reverse order for JSOP_LE and JSOP_GT:
+  // - |left <= right| is implemented as |right >= left|.
+  // - |left > right| is implemented as |right < left|.
+  if (op == JSOP_LE || op == JSOP_GT) {
+    masm.Push(left);
+    masm.Push(right);
+  } else {
+    masm.Push(right);
+    masm.Push(left);
+  }
 
   using Fn = bool (*)(JSContext*, HandleString, HandleString, bool*);
   if (op == JSOP_EQ || op == JSOP_STRICTEQ) {
     callVM<Fn, jit::StringsEqual<true>>(masm);
+  } else if (op == JSOP_NE || op == JSOP_STRICTNE) {
+    callVM<Fn, jit::StringsEqual<false>>(masm);
+  } else if (op == JSOP_LT || op == JSOP_GT) {
+    callVM<Fn, jit::StringsCompare<true>>(masm);
   } else {
-    callVM<Fn, jit::StringsEqual<false>>(masm);
+    MOZ_ASSERT(op == JSOP_LE || op == JSOP_GE);
+    callVM<Fn, jit::StringsCompare<false>>(masm);
   }
 
   masm.storeCallBoolResult(output.typedReg().gpr());
   masm.bind(&done);
   return true;
 }
 
 static bool GroupHasPropertyTypes(ObjectGroup* group, jsid* id, Value* v) {