Bug 1487022 - Fix repeated bailouts when constant-folding a never-ran 1/0 r=tcampbell
☠☠ backed out by 8ab15bf1e8cd ☠ ☠
authorAshley Hauck <khyperia@mozilla.com>
Tue, 09 Oct 2018 17:32:13 +0000
changeset 498745 3ffc1284220742b9c09c2b8d62c345114c8bd583
parent 498744 85f8ca59f7476aaeede452f7d0ecc7d478ca0144
child 498746 f3480613106993d658f9b5a46f41e59ca9934a5e
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1487022
milestone64.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 1487022 - Fix repeated bailouts when constant-folding a never-ran 1/0 r=tcampbell Differential Revision: https://phabricator.services.mozilla.com/D6067
js/src/jit/BaselineIC.cpp
js/src/jit/BaselineIC.h
js/src/jit/BaselineInspector.cpp
js/src/jit/BaselineInspector.h
js/src/jit/MIR.cpp
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -6071,16 +6071,19 @@ DoUnaryArithFallback(JSContext* cx, Base
         if (gen.tryAttachStub()) {
             bool attached = false;
             ICStub* newStub = AttachBaselineCacheIRStub(cx, gen.writerRef(), gen.cacheKind(),
                                                         BaselineCacheIRStubKind::Regular,
                                                         script, stub, &attached);
             if (newStub) {
                 JitSpew(JitSpew_BaselineIC, "  Attached UnaryArith CacheIR stub for %s", CodeName[op]);
             }
+
+        } else {
+            stub->noteUnoptimizableOperands();
         }
     }
 
     return true;
 }
 
 typedef bool (*DoUnaryArithFallbackFn)(JSContext*, BaselineFrame*, ICUnaryArith_Fallback*,
                                        HandleValue, MutableHandleValue);
--- a/js/src/jit/BaselineIC.h
+++ b/js/src/jit/BaselineIC.h
@@ -3024,22 +3024,31 @@ class ICUnaryArith_Fallback : public ICF
     friend class ICStubSpace;
 
     explicit ICUnaryArith_Fallback(JitCode* stubCode)
       : ICFallbackStub(UnaryArith_Fallback, stubCode)
     {
         extra_ = 0;
     }
 
+    static const uint16_t SAW_DOUBLE_RESULT_BIT = 0x1;
+    static const uint16_t UNOPTIMIZABLE_OPERANDS_BIT = 0x2;
+
   public:
-    bool sawDoubleResult() {
-        return extra_;
+    bool sawDoubleResult() const {
+        return extra_ & SAW_DOUBLE_RESULT_BIT;
     }
     void setSawDoubleResult() {
-        extra_ = 1;
+        extra_ |= SAW_DOUBLE_RESULT_BIT;
+    }
+    bool hadUnoptimizableOperands() const {
+        return extra_ & UNOPTIMIZABLE_OPERANDS_BIT;
+    }
+    void noteUnoptimizableOperands() {
+        extra_ |= UNOPTIMIZABLE_OPERANDS_BIT;
     }
 
     // Compiler for this stub kind.
     class Compiler : public ICStubCompiler {
       protected:
         MOZ_MUST_USE bool generateStubCode(MacroAssembler& masm) override;
 
       public:
--- a/js/src/jit/BaselineInspector.cpp
+++ b/js/src/jit/BaselineInspector.cpp
@@ -762,28 +762,44 @@ BaselineInspector::hasSeenNonStringIterM
     }
 
     const ICEntry& entry = icEntryFromPC(pc);
     ICStub* stub = entry.fallbackStub();
 
     return stub->toIteratorMore_Fallback()->hasNonStringResult();
 }
 
+// defaultIfEmpty: if we've not seen *anything* (neither double nor non-double),
+// return this value. This can happen with, for example, a never-taken branch
+// inside a hot loop.
 bool
-BaselineInspector::hasSeenDoubleResult(jsbytecode* pc)
+BaselineInspector::hasSeenDoubleResult(jsbytecode* pc, bool defaultIfEmpty)
 {
     if (!hasBaselineScript()) {
         return false;
     }
 
     const ICEntry& entry = icEntryFromPC(pc);
-    ICStub* stub = entry.fallbackStub();
+    ICFallbackStub* stub = entry.fallbackStub();
 
     MOZ_ASSERT(stub->isUnaryArith_Fallback() || stub->isBinaryArith_Fallback());
 
+    bool hasBeenExecuted = (stub->state().numOptimizedStubs() != 0);
+    if (!hasBeenExecuted) {
+        if (stub->isUnaryArith_Fallback()) {
+            hasBeenExecuted = stub->toUnaryArith_Fallback()->hadUnoptimizableOperands();
+        } else {
+            hasBeenExecuted = stub->toBinaryArith_Fallback()->hadUnoptimizableOperands();
+        }
+    }
+
+    if (!hasBeenExecuted) {
+        return defaultIfEmpty;
+    }
+
     if (stub->isUnaryArith_Fallback()) {
         return stub->toUnaryArith_Fallback()->sawDoubleResult();
     }
 
     return stub->toBinaryArith_Fallback()->sawDoubleResult();
 }
 
 JSObject*
--- a/js/src/jit/BaselineInspector.h
+++ b/js/src/jit/BaselineInspector.h
@@ -119,17 +119,17 @@ class BaselineInspector
 
     MIRType expectedResultType(jsbytecode* pc);
     MCompare::CompareType expectedCompareType(jsbytecode* pc);
     MIRType expectedBinaryArithSpecialization(jsbytecode* pc);
     MIRType expectedPropertyAccessInputType(jsbytecode* pc);
 
     bool hasSeenNegativeIndexGetElement(jsbytecode* pc);
     bool hasSeenAccessedGetter(jsbytecode* pc);
-    bool hasSeenDoubleResult(jsbytecode* pc);
+    bool hasSeenDoubleResult(jsbytecode* pc, bool defaultIfEmpty);
     bool hasSeenNonStringIterMore(jsbytecode* pc);
 
     MOZ_MUST_USE bool isOptimizableConstStringSplit(jsbytecode* pc, JSString** strOut,
                                                     JSString** sepOut, ArrayObject** objOut);
     JSObject* getTemplateObject(jsbytecode* pc);
     JSObject* getTemplateObjectForNative(jsbytecode* pc, Native native);
     JSObject* getTemplateObjectForClassHook(jsbytecode* pc, const Class* clasp);
 
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -93,17 +93,17 @@ MDefinition::PrintOpcodeName(GenericPrin
     size_t len = strlen(name);
     for (size_t i = 0; i < len; i++) {
         out.printf("%c", tolower(name[i]));
     }
 }
 #endif
 
 static MConstant*
-EvaluateConstantOperands(TempAllocator& alloc, MBinaryInstruction* ins, bool* ptypeChange = nullptr)
+EvaluateConstantOperands(TempAllocator& alloc, MBinaryInstruction* ins)
 {
     MDefinition* left = ins->getOperand(0);
     MDefinition* right = ins->getOperand(1);
 
     MOZ_ASSERT(IsTypeRepresentableAsDouble(left->type()));
     MOZ_ASSERT(IsTypeRepresentableAsDouble(right->type()));
 
     if (!left->isConstant() || !right->isConstant()) {
@@ -184,19 +184,16 @@ EvaluateConstantOperands(TempAllocator& 
     Value retVal;
     retVal.setNumber(JS::CanonicalizeNaN(ret));
 
     // If this was an int32 operation but the result isn't an int32 (for
     // example, a division where the numerator isn't evenly divisible by the
     // denominator), decline folding.
     MOZ_ASSERT(ins->type() == MIRType::Int32);
     if (!retVal.isInt32()) {
-        if (ptypeChange) {
-            *ptypeChange = true;
-        }
         return nullptr;
     }
 
     return MConstant::New(alloc, retVal);
 }
 
 static MMul*
 EvaluateExactReciprocal(TempAllocator& alloc, MDiv* ins)
@@ -2717,17 +2714,18 @@ MUrsh::infer(BaselineInspector* inspecto
     if (getOperand(0)->mightBeType(MIRType::Object) || getOperand(1)->mightBeType(MIRType::Object) ||
         getOperand(0)->mightBeType(MIRType::Symbol) || getOperand(1)->mightBeType(MIRType::Symbol))
     {
         specialization_ = MIRType::None;
         setResultType(MIRType::Value);
         return;
     }
 
-    if (inspector->hasSeenDoubleResult(pc)) {
+    // defaultIfEmpty: if we haven't seen anything, assume that's a "yes, we've seen a double".
+    if (inspector->hasSeenDoubleResult(pc, /* defaultIfEmpty: */ true)) {
         specialization_ = MIRType::Double;
         setResultType(MIRType::Double);
         return;
     }
 
     specialization_ = MIRType::Int32;
     setResultType(MIRType::Int32);
 }
@@ -2935,32 +2933,32 @@ MBinaryArithInstruction::New(TempAllocat
 void
 MBinaryArithInstruction::setNumberSpecialization(TempAllocator& alloc, BaselineInspector* inspector,
                                                  jsbytecode* pc)
 {
     setSpecialization(MIRType::Double);
 
     // Try to specialize as int32.
     if (getOperand(0)->type() == MIRType::Int32 && getOperand(1)->type() == MIRType::Int32) {
-        bool seenDouble = inspector->hasSeenDoubleResult(pc);
+        // defaultIfEmpty: if we haven't seen anything, assume that's a "yes, we've seen a double"
+        bool seenDouble = inspector->hasSeenDoubleResult(pc, /* defaultIfEmpty: */ true);
 
         // Use int32 specialization if the operation doesn't overflow on its
         // constant operands and if the operation has never overflowed.
         if (!seenDouble && !constantDoubleResult(alloc)) {
             setInt32Specialization();
         }
     }
 }
 
 bool
 MBinaryArithInstruction::constantDoubleResult(TempAllocator& alloc)
 {
-    bool typeChange = false;
-    EvaluateConstantOperands(alloc, this, &typeChange);
-    return typeChange;
+    MConstant* constantResult = EvaluateConstantOperands(alloc, this);
+    return constantResult != nullptr && constantResult->type() == MIRType::Double;
 }
 
 MDefinition*
 MRsh::foldsTo(TempAllocator& alloc)
 {
     MDefinition* f = MBinaryBitwiseInstruction::foldsTo(alloc);
 
     if (f != this) {