Bug 1542740: Convert CompareIRGenerator to use AttachDecision r=mgaudet
authorIain Ireland <iireland@mozilla.com>
Fri, 26 Apr 2019 14:29:13 +0000
changeset 530330 ffd868dfa8b16483a6f19394254c7de02aeedf68
parent 530329 ec38ecf241e2974a594e4ed362632dc3ccde03ac
child 530331 cd4e0264ddea5ccb318aa68f00538ede3c31b145
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgaudet
bugs1542740
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 1542740: Convert CompareIRGenerator to use AttachDecision r=mgaudet Differential Revision: https://phabricator.services.mozilla.com/D27305
js/src/jit/BaselineIC.cpp
js/src/jit/CacheIR.cpp
js/src/jit/CacheIR.h
js/src/jit/IonIC.cpp
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -5888,19 +5888,19 @@ bool DoCompareFallback(JSContext* cx, Ba
       break;
     default:
       MOZ_ASSERT_UNREACHABLE("Unhandled baseline compare op");
       return false;
   }
 
   ret.setBoolean(out);
 
-  TryAttachStubOld<CompareIRGenerator>("Compare", cx, frame, stub,
-                                       BaselineCacheIRStubKind::Regular, op,
-                                       lhs, rhs);
+  TryAttachStub<CompareIRGenerator>("Compare", cx, frame, stub,
+                                    BaselineCacheIRStubKind::Regular, op, lhs,
+                                    rhs);
   return true;
 }
 
 bool FallbackICCodeCompiler::emit_Compare() {
   MOZ_ASSERT(R0 == JSReturnOperand);
 
   // Restore the tail call register.
   EmitRestoreTailCallReg(masm);
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -5669,191 +5669,191 @@ CompareIRGenerator::CompareIRGenerator(J
                                        jsbytecode* pc, ICState::Mode mode,
                                        JSOp op, HandleValue lhsVal,
                                        HandleValue rhsVal)
     : IRGenerator(cx, script, pc, CacheKind::Compare, mode),
       op_(op),
       lhsVal_(lhsVal),
       rhsVal_(rhsVal) {}
 
-bool CompareIRGenerator::tryAttachString(ValOperandId lhsId,
-                                         ValOperandId rhsId) {
+AttachDecision CompareIRGenerator::tryAttachString(ValOperandId lhsId,
+                                                   ValOperandId rhsId) {
   if (!lhsVal_.isString() || !rhsVal_.isString()) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   StringOperandId lhsStrId = writer.guardIsString(lhsId);
   StringOperandId rhsStrId = writer.guardIsString(rhsId);
   writer.compareStringResult(op_, lhsStrId, rhsStrId);
   writer.returnFromIC();
 
   trackAttached("String");
-  return true;
-}
-
-bool CompareIRGenerator::tryAttachObject(ValOperandId lhsId,
-                                         ValOperandId rhsId) {
+  return AttachDecision::Attach;
+}
+
+AttachDecision CompareIRGenerator::tryAttachObject(ValOperandId lhsId,
+                                                   ValOperandId rhsId) {
   MOZ_ASSERT(IsEqualityOp(op_));
 
   if (!lhsVal_.isObject() || !rhsVal_.isObject()) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   ObjOperandId lhsObjId = writer.guardIsObject(lhsId);
   ObjOperandId rhsObjId = writer.guardIsObject(rhsId);
   writer.compareObjectResult(op_, lhsObjId, rhsObjId);
   writer.returnFromIC();
 
   trackAttached("Object");
-  return true;
-}
-
-bool CompareIRGenerator::tryAttachSymbol(ValOperandId lhsId,
-                                         ValOperandId rhsId) {
+  return AttachDecision::Attach;
+}
+
+AttachDecision CompareIRGenerator::tryAttachSymbol(ValOperandId lhsId,
+                                                   ValOperandId rhsId) {
   MOZ_ASSERT(IsEqualityOp(op_));
 
   if (!lhsVal_.isSymbol() || !rhsVal_.isSymbol()) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   SymbolOperandId lhsSymId = writer.guardIsSymbol(lhsId);
   SymbolOperandId rhsSymId = writer.guardIsSymbol(rhsId);
   writer.compareSymbolResult(op_, lhsSymId, rhsSymId);
   writer.returnFromIC();
 
   trackAttached("Symbol");
-  return true;
-}
-
-bool CompareIRGenerator::tryAttachStrictDifferentTypes(ValOperandId lhsId,
-                                                       ValOperandId rhsId) {
+  return AttachDecision::Attach;
+}
+
+AttachDecision CompareIRGenerator::tryAttachStrictDifferentTypes(
+    ValOperandId lhsId, ValOperandId rhsId) {
   MOZ_ASSERT(IsEqualityOp(op_));
 
   if (op_ != JSOP_STRICTEQ && op_ != JSOP_STRICTNE) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   // Probably can't hit some of these.
   if (SameType(lhsVal_, rhsVal_) ||
       (lhsVal_.isNumber() && rhsVal_.isNumber())) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   // Compare tags
   ValueTagOperandId lhsTypeId = writer.loadValueTag(lhsId);
   ValueTagOperandId rhsTypeId = writer.loadValueTag(rhsId);
   writer.guardTagNotEqual(lhsTypeId, rhsTypeId);
 
   // Now that we've passed the guard, we know differing types, so return the
   // bool result.
   writer.loadBooleanResult(op_ == JSOP_STRICTNE ? true : false);
   writer.returnFromIC();
 
   trackAttached("StrictDifferentTypes");
-  return true;
-}
-
-bool CompareIRGenerator::tryAttachInt32(ValOperandId lhsId,
-                                        ValOperandId rhsId) {
+  return AttachDecision::Attach;
+}
+
+AttachDecision CompareIRGenerator::tryAttachInt32(ValOperandId lhsId,
+                                                  ValOperandId rhsId) {
   if ((!lhsVal_.isInt32() && !lhsVal_.isBoolean()) ||
       (!rhsVal_.isInt32() && !rhsVal_.isBoolean())) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   Int32OperandId lhsIntId = lhsVal_.isBoolean() ? writer.guardIsBoolean(lhsId)
                                                 : writer.guardIsInt32(lhsId);
   Int32OperandId rhsIntId = rhsVal_.isBoolean() ? writer.guardIsBoolean(rhsId)
                                                 : writer.guardIsInt32(rhsId);
 
   // Strictly different types should have been handed by
   // tryAttachStrictDifferentTypes
   MOZ_ASSERT_IF(op_ == JSOP_STRICTEQ || op_ == JSOP_STRICTNE,
                 lhsVal_.isInt32() == rhsVal_.isInt32());
 
   writer.compareInt32Result(op_, lhsIntId, rhsIntId);
   writer.returnFromIC();
 
   trackAttached(lhsVal_.isBoolean() ? "Boolean" : "Int32");
-  return true;
-}
-
-bool CompareIRGenerator::tryAttachNumber(ValOperandId lhsId,
-                                         ValOperandId rhsId) {
+  return AttachDecision::Attach;
+}
+
+AttachDecision CompareIRGenerator::tryAttachNumber(ValOperandId lhsId,
+                                                   ValOperandId rhsId) {
   if (!lhsVal_.isNumber() || !rhsVal_.isNumber()) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   writer.guardIsNumber(lhsId);
   writer.guardIsNumber(rhsId);
   writer.compareDoubleResult(op_, lhsId, rhsId);
   writer.returnFromIC();
 
   trackAttached("Number");
-  return true;
-}
-
-bool CompareIRGenerator::tryAttachObjectUndefined(ValOperandId lhsId,
-                                                  ValOperandId rhsId) {
+  return AttachDecision::Attach;
+}
+
+AttachDecision CompareIRGenerator::tryAttachObjectUndefined(
+    ValOperandId lhsId, ValOperandId rhsId) {
   if (!(lhsVal_.isNullOrUndefined() && rhsVal_.isObject()) &&
       !(rhsVal_.isNullOrUndefined() && lhsVal_.isObject()))
-    return false;
+    return AttachDecision::NoAction;
 
   if (op_ != JSOP_EQ && op_ != JSOP_NE) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   ValOperandId obj = rhsVal_.isObject() ? rhsId : lhsId;
   ValOperandId undefOrNull = rhsVal_.isObject() ? lhsId : rhsId;
 
   writer.guardIsNullOrUndefined(undefOrNull);
   ObjOperandId objOperand = writer.guardIsObject(obj);
   writer.compareObjectUndefinedNullResult(op_, objOperand);
   writer.returnFromIC();
 
   trackAttached("ObjectUndefined");
-  return true;
+  return AttachDecision::Attach;
 }
 
 // Handle NumberUndefined comparisons
-bool CompareIRGenerator::tryAttachNumberUndefined(ValOperandId lhsId,
-                                                  ValOperandId rhsId) {
+AttachDecision CompareIRGenerator::tryAttachNumberUndefined(
+    ValOperandId lhsId, ValOperandId rhsId) {
   if (!(lhsVal_.isUndefined() && rhsVal_.isNumber()) &&
       !(rhsVal_.isUndefined() && lhsVal_.isNumber())) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   lhsVal_.isNumber() ? writer.guardIsNumber(lhsId)
                      : writer.guardIsUndefined(lhsId);
   rhsVal_.isNumber() ? writer.guardIsNumber(rhsId)
                      : writer.guardIsUndefined(rhsId);
 
   // Comparing a number with undefined will always be true for NE/STRICTNE,
   // and always be false for other compare ops.
   writer.loadBooleanResult(op_ == JSOP_NE || op_ == JSOP_STRICTNE);
   writer.returnFromIC();
 
   trackAttached("NumberUndefined");
-  return true;
+  return AttachDecision::Attach;
 }
 
 // Handle Primitive x {undefined,null} equality comparisons
-bool CompareIRGenerator::tryAttachPrimitiveUndefined(ValOperandId lhsId,
-                                                     ValOperandId rhsId) {
+AttachDecision CompareIRGenerator::tryAttachPrimitiveUndefined(
+    ValOperandId lhsId, ValOperandId rhsId) {
   MOZ_ASSERT(IsEqualityOp(op_));
 
   // The set of primitive cases we want to handle here (excluding null,
   // undefined)
   auto isPrimitive = [](HandleValue& x) {
     return x.isString() || x.isSymbol() || x.isBoolean() || x.isNumber() ||
            x.isBigInt();
   };
 
   if (!(lhsVal_.isNullOrUndefined() && isPrimitive(rhsVal_)) &&
       !(rhsVal_.isNullOrUndefined() && isPrimitive(lhsVal_))) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   auto guardPrimitive = [&](HandleValue v, ValOperandId id) {
     if (v.isNumber()) {
       writer.guardIsNumber(id);
       return;
     }
     switch (v.extractNonDoubleType()) {
@@ -5881,24 +5881,24 @@ bool CompareIRGenerator::tryAttachPrimit
                        : writer.guardIsNullOrUndefined(rhsId);
 
   // Comparing a primitive with undefined/null will always be true for
   // NE/STRICTNE, and always be false for other compare ops.
   writer.loadBooleanResult(op_ == JSOP_NE || op_ == JSOP_STRICTNE);
   writer.returnFromIC();
 
   trackAttached("PrimitiveUndefined");
-  return true;
+  return AttachDecision::Attach;
 }
 
 // Handle {null/undefined} x {null,undefined} equality comparisons
-bool CompareIRGenerator::tryAttachNullUndefined(ValOperandId lhsId,
-                                                ValOperandId rhsId) {
+AttachDecision CompareIRGenerator::tryAttachNullUndefined(ValOperandId lhsId,
+                                                          ValOperandId rhsId) {
   if (!lhsVal_.isNullOrUndefined() || !rhsVal_.isNullOrUndefined()) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   if (op_ == JSOP_EQ || op_ == JSOP_NE) {
     writer.guardIsNullOrUndefined(lhsId);
     writer.guardIsNullOrUndefined(rhsId);
     // Sloppy equality means we actually only care about the op:
     writer.loadBooleanResult(op_ == JSOP_EQ);
     trackAttached("SloppyNullUndefined");
@@ -5911,25 +5911,25 @@ bool CompareIRGenerator::tryAttachNullUn
                      : writer.guardIsUndefined(lhsId);
     rhsVal_.isNull() ? writer.guardIsNull(rhsId)
                      : writer.guardIsUndefined(rhsId);
     writer.loadBooleanResult(op_ == JSOP_STRICTEQ);
     trackAttached("StrictNullUndefinedEquality");
   }
 
   writer.returnFromIC();
-  return true;
-}
-
-bool CompareIRGenerator::tryAttachStringNumber(ValOperandId lhsId,
-                                               ValOperandId rhsId) {
+  return AttachDecision::Attach;
+}
+
+AttachDecision CompareIRGenerator::tryAttachStringNumber(ValOperandId lhsId,
+                                                         ValOperandId rhsId) {
   // Ensure String x Number
   if (!(lhsVal_.isString() && rhsVal_.isNumber()) &&
       !(rhsVal_.isString() && lhsVal_.isNumber())) {
-    return false;
+    return AttachDecision::NoAction;
   }
 
   // Case should have been handled by tryAttachStrictlDifferentTypes
   MOZ_ASSERT(op_ != JSOP_STRICTEQ && op_ != JSOP_STRICTNE);
 
   auto createGuards = [&](HandleValue v, ValOperandId vId) {
     if (v.isString()) {
       StringOperandId strId = writer.guardIsString(vId);
@@ -5941,20 +5941,20 @@ bool CompareIRGenerator::tryAttachString
   };
 
   ValOperandId lhsGuardedId = createGuards(lhsVal_, lhsId);
   ValOperandId rhsGuardedId = createGuards(rhsVal_, rhsId);
   writer.compareDoubleResult(op_, lhsGuardedId, rhsGuardedId);
   writer.returnFromIC();
 
   trackAttached("StringNumber");
-  return true;
-}
-
-bool CompareIRGenerator::tryAttachStub() {
+  return AttachDecision::Attach;
+}
+
+AttachDecision CompareIRGenerator::tryAttachStub() {
   MOZ_ASSERT(cacheKind_ == CacheKind::Compare);
   MOZ_ASSERT(IsEqualityOp(op_) || IsRelationalOp(op_));
 
   AutoAssertNoPendingException aanpe(cx_);
 
   constexpr uint8_t lhsIndex = 0;
   constexpr uint8_t rhsIndex = 1;
 
@@ -5965,72 +5965,50 @@ 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 (tryAttachObject(lhsId, rhsId)) {
-      return true;
-    }
-    if (tryAttachSymbol(lhsId, rhsId)) {
-      return true;
-    }
+    TRY_ATTACH(tryAttachObject(lhsId, rhsId));
+    TRY_ATTACH(tryAttachSymbol(lhsId, rhsId));
 
     // Handle the special case of Object compared to null/undefined.
-    // This is special due to the IsHTMLDDA internal slot semantic,
-    if (tryAttachObjectUndefined(lhsId, rhsId)) {
-      return true;
-    }
+    // This is special due to the IsHTMLDDA internal slot semantics.
+    TRY_ATTACH(tryAttachObjectUndefined(lhsId, rhsId));
 
     // This covers -strict- equality/inequality using a type tag check, so
     // catches all different type pairs outside of Numbers, which cannot be
     // checked on tags alone.
-    if (tryAttachStrictDifferentTypes(lhsId, rhsId)) {
-      return true;
-    }
+    TRY_ATTACH(tryAttachStrictDifferentTypes(lhsId, rhsId));
 
     // These checks should come after tryAttachStrictDifferentTypes since it
     // handles strict inequality with a more generic IC.
-    if (tryAttachPrimitiveUndefined(lhsId, rhsId)) {
-      return true;
-    }
-
-    if (tryAttachNullUndefined(lhsId, rhsId)) {
-      return true;
-    }
+    TRY_ATTACH(tryAttachPrimitiveUndefined(lhsId, rhsId));
+
+    TRY_ATTACH(tryAttachNullUndefined(lhsId, rhsId));
   }
 
   // This should preceed the Int32/Number cases to allow
   // them to not concern themselves with handling undefined
   // or null.
-  if (tryAttachNumberUndefined(lhsId, rhsId)) {
-    return true;
-  }
+  TRY_ATTACH(tryAttachNumberUndefined(lhsId, rhsId));
 
   // 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;
-  }
+  TRY_ATTACH(tryAttachInt32(lhsId, rhsId));
+  TRY_ATTACH(tryAttachNumber(lhsId, rhsId));
+  TRY_ATTACH(tryAttachString(lhsId, rhsId));
+
+  TRY_ATTACH(tryAttachStringNumber(lhsId, rhsId));
 
   trackAttached(IRGenerator::NotAttached);
-  return false;
+  return AttachDecision::NoAction;
 }
 
 void CompareIRGenerator::trackAttached(const char* name) {
 #ifdef JS_CACHEIR_SPEW
   if (const CacheIRSpewer::Guard& sp = CacheIRSpewer::Guard(*this, name)) {
     sp.valueProperty("lhs", lhsVal_);
     sp.valueProperty("rhs", rhsVal_);
     sp.opcodeProperty("op", op_);
--- a/js/src/jit/CacheIR.h
+++ b/js/src/jit/CacheIR.h
@@ -2393,36 +2393,40 @@ class MOZ_RAII CallIRGenerator : public 
   const PropertyTypeCheckInfo* typeCheckInfo() const { return &typeCheckInfo_; }
 };
 
 class MOZ_RAII CompareIRGenerator : public IRGenerator {
   JSOp op_;
   HandleValue lhsVal_;
   HandleValue rhsVal_;
 
-  bool tryAttachString(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachObject(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachSymbol(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachStrictDifferentTypes(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachInt32(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachNumber(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachNumberUndefined(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachPrimitiveUndefined(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachObjectUndefined(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachNullUndefined(ValOperandId lhsId, ValOperandId rhsId);
-  bool tryAttachStringNumber(ValOperandId lhsId, ValOperandId rhsId);
+  AttachDecision tryAttachString(ValOperandId lhsId, ValOperandId rhsId);
+  AttachDecision tryAttachObject(ValOperandId lhsId, ValOperandId rhsId);
+  AttachDecision tryAttachSymbol(ValOperandId lhsId, ValOperandId rhsId);
+  AttachDecision tryAttachStrictDifferentTypes(ValOperandId lhsId,
+                                               ValOperandId rhsId);
+  AttachDecision tryAttachInt32(ValOperandId lhsId, ValOperandId rhsId);
+  AttachDecision tryAttachNumber(ValOperandId lhsId, ValOperandId rhsId);
+  AttachDecision tryAttachNumberUndefined(ValOperandId lhsId,
+                                          ValOperandId rhsId);
+  AttachDecision tryAttachPrimitiveUndefined(ValOperandId lhsId,
+                                             ValOperandId rhsId);
+  AttachDecision tryAttachObjectUndefined(ValOperandId lhsId,
+                                          ValOperandId rhsId);
+  AttachDecision tryAttachNullUndefined(ValOperandId lhsId, ValOperandId rhsId);
+  AttachDecision tryAttachStringNumber(ValOperandId lhsId, ValOperandId rhsId);
 
   void trackAttached(const char* name);
 
  public:
   CompareIRGenerator(JSContext* cx, HandleScript, jsbytecode* pc,
                      ICState::Mode mode, JSOp op, HandleValue lhsVal,
                      HandleValue rhsVal);
 
-  bool tryAttachStub();
+  AttachDecision tryAttachStub();
 };
 
 class MOZ_RAII ToBoolIRGenerator : public IRGenerator {
   HandleValue val_;
 
   bool tryAttachInt32();
   bool tryAttachDouble();
   bool tryAttachString();
--- a/js/src/jit/IonIC.cpp
+++ b/js/src/jit/IonIC.cpp
@@ -678,18 +678,18 @@ bool IonCompareIC::update(JSContext* cx,
         return false;
       }
       break;
     default:
       MOZ_ASSERT_UNREACHABLE("Unhandled ion compare op");
       return false;
   }
 
-  TryAttachIonStubOld<CompareIRGenerator, IonCompareIC>(cx, ic, ionScript, op,
-                                                        lhs, rhs);
+  TryAttachIonStub<CompareIRGenerator, IonCompareIC>(cx, ic, ionScript, op, lhs,
+                                                     rhs);
 
   return true;
 }
 
 uint8_t* IonICStub::stubDataStart() {
   return reinterpret_cast<uint8_t*>(this) + stubInfo_->stubDataOffset();
 }