Bug 1277562 - Part 11: Adapt to tiering in instance list search. r=luke

--HG--
extra : rebase_source : c71fc2d2384e1665806edd4af1027f9c274b2195
extra : intermediate-source : db5208041134516b23467b0465ecf089692f6210
extra : source : 0d6c1bdb97fd8909442a7d9f1fea9b6ecf27d546

js/src/jit/arm/Simulator-arm.cpp

@@ -1597,7 +1597,14 @@ Simulator::handleWasmFault(int32_t addr, unsigned numBytes)
 
     void* pc = reinterpret_cast<void*>(get_pc());
     uint8_t* fp = reinterpret_cast<uint8_t*>(get_register(r11));
-    wasm::Instance* instance = wasm::LookupFaultingInstance(act, pc, fp);
+
+    // Cache the wasm::Code to avoid lookup on every load/store.
+    if (!wasm_code_ || !wasm_code_->containsCodePC(pc))
+        wasm_code_ = act->compartment()->wasm.lookupCode(pc);
+    if (!wasm_code_)
+        return false;
+
+    wasm::Instance* instance = wasm::LookupFaultingInstance(*wasm_code_, pc, fp);
     if (!instance || !instance->memoryAccessInGuardRegion((uint8_t*)addr, numBytes))
         return false;
 

js/src/jit/arm/Simulator-arm.h

@@ -38,6 +38,7 @@
 #include "jit/IonTypes.h"
 #include "threading/Thread.h"
 #include "vm/MutexIDs.h"
+#include "wasm/WasmCode.h"
 
 namespace js {
 
@@ -423,8 +424,9 @@ class Simulator
     bool pc_modified_;
     int64_t icount_;
 
-    // wasm async interrupt support
+    // wasm async interrupt / fault support
     bool wasm_interrupt_;
+    wasm::SharedCode wasm_code_;
 
     // Debugger input.
     char* lastDebuggerInput_;

js/src/wasm/WasmCompartment.cpp

@@ -27,9 +27,6 @@
 using namespace js;
 using namespace wasm;
 
-// With tiering, instances can have one or two code segments, and code that
-// searches the instance list will change.  Search for Tier::TBD below.
-
 Compartment::Compartment(Zone* zone)
   : mutatingInstances_(false)
 {}
@@ -52,10 +49,16 @@ struct InstanceComparator
         // Instances can share code, so the segments can be equal (though they
         // can't partially overlap).  If the codeBases are equal, we sort by
         // Instance address.  Thus a Code may map to many instances.
-        if (instance->codeBase(Tier::TBD) == target.codeBase(Tier::TBD))
+
+        // Compare by the first tier, always.
+
+        Tier instanceTier = instance->code().stableTier();
+        Tier targetTier = target.code().stableTier();
+
+        if (instance->codeBase(instanceTier) == target.codeBase(targetTier))
             return instance < &target ? -1 : 1;
 
-        return target.codeBase(Tier::TBD) < instance->codeBase(Tier::TBD) ? -1 : 1;
+        return target.codeBase(targetTier) < instance->codeBase(instanceTier) ? -1 : 1;
     }
 };
 
@@ -100,18 +103,6 @@ Compartment::unregisterInstance(Instance& instance)
     instances_.erase(instances_.begin() + index);
 }
 
-struct PCComparator
-{
-    const void* pc;
-    explicit PCComparator(const void* pc) : pc(pc) {}
-
-    int operator()(const Instance* instance) const {
-        if (instance->codeSegment(Tier::TBD).containsCodePC(pc))
-            return 0;
-        return pc < instance->codeBase(Tier::TBD) ? -1 : 1;
-    }
-};
-
 const Code*
 Compartment::lookupCode(const void* pc, const CodeSegment** segmentp) const
 {
@@ -122,14 +113,22 @@ Compartment::lookupCode(const void* pc, const CodeSegment** segmentp) const
     if (mutatingInstances_)
         return nullptr;
 
-    size_t index;
-    if (!BinarySearchIf(instances_, 0, instances_.length(), PCComparator(pc), &index))
-        return nullptr;
+    // Linear search because instances are only ordered by their first tiers,
+    // but may have two.  This code should not be hot anyway, we avoid
+    // lookupCode when we can.
+    for (auto i : instances_) {
+        const Code& code = i->code();
+        for (auto t : code.tiers()) {
+            const CodeSegment& segment = code.segment(t);
+            if (segment.containsCodePC(pc)) {
+                if (segmentp)
+                    *segmentp = &segment;
+                return &code;
+            }
+        }
+    }
 
-    const Code& code = instances_[index]->code();
-    if (segmentp)
-        *segmentp = &code.segment(Tier::TBD);
-    return &code;
+    return nullptr;
 }
 
 void

js/src/wasm/WasmFrameIter.cpp

@@ -60,9 +60,8 @@ WasmFrameIter::WasmFrameIter(WasmActivation* activation, Unwind unwind)
     // but this is fine because CallSite is only used for line number for which
     // we can use the beginning of the function from the CodeRange instead.
 
-    code_ = activation_->compartment()->wasm.lookupCode(activation->unwindPC());
-    MOZ_ASSERT(code_);
-    MOZ_ASSERT(&fp_->tls->instance->code() == code_);
+    code_ = &fp_->tls->instance->code();
+    MOZ_ASSERT(code_ == activation->compartment()->wasm.lookupCode(activation->unwindPC()));
 
     codeRange_ = code_->lookupRange(activation->unwindPC());
     MOZ_ASSERT(codeRange_->kind() == CodeRange::Function);
@@ -124,8 +123,8 @@ WasmFrameIter::popFrame()
 
     void* returnAddress = prevFP->returnAddress;
 
-    code_ = activation_->compartment()->wasm.lookupCode(returnAddress);
-    MOZ_ASSERT(code_);
+    code_ = &fp_->tls->instance->code();
+    MOZ_ASSERT(code_ == activation_->compartment()->wasm.lookupCode(returnAddress));
 
     codeRange_ = code_->lookupRange(returnAddress);
     MOZ_ASSERT(codeRange_->kind() == CodeRange::Function);
@@ -800,17 +799,21 @@ ProfilingFrameIterator::operator++()
         return;
     }
 
-    code_ = activation_->compartment()->wasm.lookupCode(callerPC_);
-    MOZ_ASSERT(code_);
+    if (!callerFP_) {
+        codeRange_ = code_->lookupRange(callerPC_);
+        MOZ_ASSERT(codeRange_->kind() == CodeRange::Entry);
+        callerPC_ = nullptr;
+        MOZ_ASSERT(!done());
+        return;
+    }
+
+    code_ = &callerFP_->tls->instance->code();
+    MOZ_ASSERT(code_ == activation_->compartment()->wasm.lookupCode(callerPC_));
 
     codeRange_ = code_->lookupRange(callerPC_);
     MOZ_ASSERT(codeRange_);
 
     switch (codeRange_->kind()) {
-      case CodeRange::Entry:
-        MOZ_ASSERT(callerFP_ == nullptr);
-        callerPC_ = nullptr;
-        break;
       case CodeRange::Function:
       case CodeRange::ImportJitExit:
       case CodeRange::ImportInterpExit:
@@ -824,6 +827,8 @@ ProfilingFrameIterator::operator++()
         AssertMatchesCallSite(*activation_, callerPC_, callerFP_->callerFP);
         callerFP_ = callerFP_->callerFP;
         break;
+      case CodeRange::Entry:
+        MOZ_CRASH("should have had null caller fp");
       case CodeRange::Interrupt:
       case CodeRange::Throw:
         MOZ_CRASH("code range doesn't have frame");
@@ -993,7 +998,7 @@ ProfilingFrameIterator::label() const
 }
 
 Instance*
-wasm::LookupFaultingInstance(WasmActivation* activation, void* pc, void* fp)
+wasm::LookupFaultingInstance(const Code& code, void* pc, void* fp)
 {
     // Assume bug-caused faults can be raised at any PC and apply the logic of
     // ProfilingFrameIterator to reject any pc outside the (post-prologue,
@@ -1001,12 +1006,8 @@ wasm::LookupFaultingInstance(WasmActivation* activation, void* pc, void* fp)
     // simulators which call this function at every load/store before even
     // knowing whether there is a fault.
 
-    const Code* code = activation->compartment()->wasm.lookupCode(pc);
-    if (!code)
-        return nullptr;
-
     const CodeSegment* codeSegment;
-    const CodeRange* codeRange = code->lookupRange(pc, &codeSegment);
+    const CodeRange* codeRange = code.lookupRange(pc, &codeSegment);
     if (!codeRange || !codeRange->isFunction())
         return nullptr;
 
@@ -1017,7 +1018,7 @@ wasm::LookupFaultingInstance(WasmActivation* activation, void* pc, void* fp)
         return nullptr;
 
     Instance* instance = reinterpret_cast<Frame*>(fp)->tls->instance;
-    MOZ_RELEASE_ASSERT(&instance->code() == code);
+    MOZ_RELEASE_ASSERT(&instance->code() == &code);
     return instance;
 }
 

js/src/wasm/WasmFrameIter.h

@@ -193,7 +193,7 @@ GenerateFunctionEpilogue(jit::MacroAssembler& masm, unsigned framePushed, FuncOf
 // is such a plausible instance, and otherwise null.
 
 Instance*
-LookupFaultingInstance(WasmActivation* activation, void* pc, void* fp);
+LookupFaultingInstance(const Code& code, void* pc, void* fp);
 
 // If the innermost (active) Activation is a WasmActivation, return it.
 

js/src/wasm/WasmSignalHandlers.cpp

@@ -1015,7 +1015,7 @@ HandleFault(PEXCEPTION_POINTERS exception)
         return false;
     }
 
-    const Instance* instance = LookupFaultingInstance(activation, pc, ContextToFP(context));
+    const Instance* instance = LookupFaultingInstance(*code, pc, ContextToFP(context));
     if (!instance)
         return false;
 
@@ -1134,7 +1134,11 @@ HandleMachException(JSContext* cx, const ExceptionRequest& request)
     if (!activation)
         return false;
 
-    const Instance* instance = LookupFaultingInstance(activation, pc, ContextToFP(&context));
+    const Code* code = activation->compartment()->wasm.lookupCode(pc);
+    if (!code)
+        return false;
+
+    const Instance* instance = LookupFaultingInstance(*code, pc, ContextToFP(&context));
     if (!instance || !instance->code().containsFunctionPC(pc))
         return false;
 
@@ -1342,7 +1346,11 @@ HandleFault(int signum, siginfo_t* info, void* ctx)
         return false;
 
     const CodeSegment* segment;
-    const Instance* instance = LookupFaultingInstance(activation, pc, ContextToFP(context));
+    const Code* code = activation->compartment()->wasm.lookupCode(pc, &segment);
+    if (!code)
+        return false;
+
+    const Instance* instance = LookupFaultingInstance(*code, pc, ContextToFP(context));
     if (!instance || !instance->code().containsFunctionPC(pc, &segment))
         return false;