Bug 1184965 part 3 - Use RAII for redundant profilerPreCall and profilerPostReturn calls. r=djvj

js/src/jit/MacroAssembler-inl.h

@@ -94,9 +94,8 @@ MacroAssembler::passABIArg(FloatRegister reg, MoveOp::Type type)
 template <typename T> void
 MacroAssembler::callWithABI(const T& fun, MoveOp::Type result)
 {
-    profilerPreCall();
+    AutoProfilerCallInstrumentation profiler(*this);
     callWithABINoProfiler(fun, result);
-    profilerPostReturn();
 }
 
 void
@@ -158,9 +157,8 @@ MacroAssembler::signature() const
 uint32_t
 MacroAssembler::callJit(Register callee)
 {
-    profilerPreCall();
+    AutoProfilerCallInstrumentation profiler(*this);
     uint32_t ret = callJitNoProfiler(callee);
-    profilerPostReturn();
     return ret;
 }
 

js/src/jit/MacroAssembler.cpp

@@ -2375,14 +2375,7 @@ MacroAssembler::link(JitCode* code)
                                 ImmPtr((void*)-1));
     }
 
-    // Fix up the code pointers to be written for locations where profilerCallSite
-    // emitted moves of RIP to a register.
-    for (size_t i = 0; i < profilerCallSites_.length(); i++) {
-        CodeOffsetLabel offset = profilerCallSites_[i];
-        offset.fixup(this);
-        CodeLocationLabel location(code, offset);
-        PatchDataWithValueCheck(location, ImmPtr(location.raw()), ImmPtr((void*)-1));
-    }
+    linkProfilerCallSites(code);
 }
 
 void
@@ -2436,29 +2429,41 @@ MacroAssembler::branchEqualTypeIfNeeded(MIRType type, MDefinition* maybeDef, Reg
     }
 }
 
-void
-MacroAssembler::profilerPreCallImpl()
+MacroAssembler::AutoProfilerCallInstrumentation::AutoProfilerCallInstrumentation(
+    MacroAssembler& masm
+    MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
 {
+    MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    if (!masm.emitProfilingInstrumentation_)
+        return;
+
     Register reg = CallTempReg0;
     Register reg2 = CallTempReg1;
-    push(reg);
-    push(reg2);
-    profilerPreCallImpl(reg, reg2);
-    pop(reg2);
-    pop(reg);
-}
+    masm.push(reg);
+    masm.push(reg2);
 
-void
-MacroAssembler::profilerPreCallImpl(Register reg, Register reg2)
-{
     JitContext* icx = GetJitContext();
     AbsoluteAddress profilingActivation(icx->runtime->addressOfProfilingActivation());
 
-    CodeOffsetLabel label = movWithPatch(ImmWord(uintptr_t(-1)), reg);
-    loadPtr(profilingActivation, reg2);
-    storePtr(reg, Address(reg2, JitActivation::offsetOfLastProfilingCallSite()));
+    CodeOffsetLabel label = masm.movWithPatch(ImmWord(uintptr_t(-1)), reg);
+    masm.loadPtr(profilingActivation, reg2);
+    masm.storePtr(reg, Address(reg2, JitActivation::offsetOfLastProfilingCallSite()));
 
-    appendProfilerCallSite(label);
+    masm.appendProfilerCallSite(label);
+
+    masm.pop(reg2);
+    masm.pop(reg);
+}
+
+void
+MacroAssembler::linkProfilerCallSites(JitCode* code)
+{
+    for (size_t i = 0; i < profilerCallSites_.length(); i++) {
+        CodeOffsetLabel offset = profilerCallSites_[i];
+        offset.fixup(this);
+        CodeLocationLabel location(code, offset);
+        PatchDataWithValueCheck(location, ImmPtr(location.raw()), ImmPtr((void*)-1));
+    }
 }
 
 void
@@ -2546,11 +2551,11 @@ MacroAssembler::alignJitStackBasedOnNArgs(uint32_t nargs)
 
 MacroAssembler::MacroAssembler(JSContext* cx, IonScript* ion,
                                JSScript* script, jsbytecode* pc)
-  : emitProfilingInstrumentation_(false),
-    framePushed_(0)
+  : framePushed_(0),
 #ifdef DEBUG
-  , inCall_(false)
+    inCall_(false),
 #endif
+    emitProfilingInstrumentation_(false)
 {
     constructRoot(cx);
     jitContext_.emplace(cx, (js::jit::TempAllocator*)nullptr);
@@ -2566,7 +2571,7 @@ MacroAssembler::MacroAssembler(JSContext* cx, IonScript* ion,
     if (ion) {
         setFramePushed(ion->frameSize());
         if (pc && cx->runtime()->spsProfiler.enabled())
-            emitProfilingInstrumentation_ = true;
+            enableProfilingInstrumentation();
     }
 }
 

js/src/jit/MacroAssembler.h

@@ -320,21 +320,16 @@ class MacroAssembler : public MacroAssemblerSpecific
     mozilla::Maybe<AutoJitContextAlloc> alloc_;
 
   private:
-    // This field is used to manage profiling instrumentation output. If
-    // provided and enabled, then instrumentation will be emitted around call
-    // sites.
-    bool emitProfilingInstrumentation_;
-
     // Labels for handling exceptions and failures.
     NonAssertingLabel failureLabel_;
 
   public:
     MacroAssembler()
-      : emitProfilingInstrumentation_(false),
-        framePushed_(0)
+      : framePushed_(0),
 #ifdef DEBUG
-      , inCall_(false)
+        inCall_(false),
 #endif
+        emitProfilingInstrumentation_(false)
     {
         JitContext* jcx = GetJitContext();
         JSContext* cx = jcx->cx;
@@ -365,11 +360,11 @@ class MacroAssembler : public MacroAssemblerSpecific
     // asm.js compilation handles its own JitContext-pushing
     struct AsmJSToken {};
     explicit MacroAssembler(AsmJSToken)
-      : emitProfilingInstrumentation_(false),
-        framePushed_(0)
+      : framePushed_(0),
 #ifdef DEBUG
-      , inCall_(false)
+        inCall_(false),
 #endif
+        emitProfilingInstrumentation_(false)
     {
 #if defined(JS_CODEGEN_ARM)
         initWithAllocator();
@@ -380,10 +375,6 @@ class MacroAssembler : public MacroAssemblerSpecific
 #endif
     }
 
-    void enableProfilingInstrumentation() {
-        emitProfilingInstrumentation_ = true;
-    }
-
     void resetForNewCodeGenerator(TempAllocator& alloc);
 
     void constructRoot(JSContext* cx) {
@@ -1116,28 +1107,25 @@ class MacroAssembler : public MacroAssemblerSpecific
 
     // see above comment for what is returned
     uint32_t callWithExitFrame(Label* target) {
-        profilerPreCall();
+        AutoProfilerCallInstrumentation profiler(*this);
         MacroAssemblerSpecific::callWithExitFrame(target);
         uint32_t ret = currentOffset();
-        profilerPostReturn();
         return ret;
     }
 
     // see above comment for what is returned
     uint32_t callWithExitFrame(JitCode* target) {
-        profilerPreCall();
+        AutoProfilerCallInstrumentation profiler(*this);
         MacroAssemblerSpecific::callWithExitFrame(target);
         uint32_t ret = currentOffset();
-        profilerPostReturn();
         return ret;
     }
 
     // see above comment for what is returned
     uint32_t callWithExitFrame(JitCode* target, Register dynStack) {
-        profilerPreCall();
+        AutoProfilerCallInstrumentation profiler(*this);
         MacroAssemblerSpecific::callWithExitFrame(target, dynStack);
         uint32_t ret = currentOffset();
-        profilerPostReturn();
         return ret;
     }
 
@@ -1231,22 +1219,41 @@ class MacroAssembler : public MacroAssemblerSpecific
     }
 #endif // !JS_CODEGEN_ARM64
 
-  private:
-    // These two functions are helpers used around call sites throughout the
-    // assembler. They are called from the above call wrappers to emit the
-    // necessary instrumentation.
-    void profilerPreCall() {
-        if (!emitProfilingInstrumentation_)
-            return;
-        profilerPreCallImpl();
+  public:
+    void enableProfilingInstrumentation() {
+        emitProfilingInstrumentation_ = true;
     }
 
-    void profilerPostReturn() {
-        if (!emitProfilingInstrumentation_)
-            return;
-        profilerPostReturnImpl();
+  private:
+    // This class is used to surround call sites throughout the assembler. This
+    // is used by callWithABI, callJit, and callWithExitFrame functions, except
+    // if suffixed by NoProfiler.
+    class AutoProfilerCallInstrumentation {
+        MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER;
+
+      public:
+        explicit AutoProfilerCallInstrumentation(MacroAssembler& masm
+                                                 MOZ_GUARD_OBJECT_NOTIFIER_PARAM);
+        ~AutoProfilerCallInstrumentation() {}
+    };
+    friend class AutoProfilerCallInstrumentation;
+
+    void appendProfilerCallSite(CodeOffsetLabel label) {
+        propagateOOM(profilerCallSites_.append(label));
     }
 
+    // Fix up the code pointers to be written for locations where profilerCallSite
+    // emitted moves of RIP to a register.
+    void linkProfilerCallSites(JitCode* code);
+
+    // This field is used to manage profiling instrumentation output. If
+    // provided and enabled, then instrumentation will be emitted around call
+    // sites.
+    bool emitProfilingInstrumentation_;
+
+    // Record locations of the call sites.
+    Vector<CodeOffsetLabel, 0, SystemAllocPolicy> profilerCallSites_;
+
   public:
     void loadBaselineOrIonRaw(Register script, Register dest, Label* failure);
     void loadBaselineOrIonNoArgCheck(Register callee, Register dest, Label* failure);
@@ -1558,10 +1565,6 @@ class MacroAssembler : public MacroAssemblerSpecific
         bind(&ok);
 #endif
     }
-
-    void profilerPreCallImpl();
-    void profilerPreCallImpl(Register reg, Register reg2);
-    void profilerPostReturnImpl() {}
 };
 
 static inline Assembler::DoubleCondition

js/src/jit/shared/Assembler-shared.h

@@ -914,7 +914,6 @@ class AssemblerShared
     Vector<AsmJSAbsoluteLink, 0, SystemAllocPolicy> asmJSAbsoluteLinks_;
 
   protected:
-    Vector<CodeOffsetLabel, 0, SystemAllocPolicy> profilerCallSites_;
     bool enoughMemory_;
     bool embedsNurseryPointers_;
 
@@ -936,10 +935,6 @@ class AssemblerShared
         return !enoughMemory_;
     }
 
-    void appendProfilerCallSite(CodeOffsetLabel label) {
-        enoughMemory_ &= profilerCallSites_.append(label);
-    }
-
     bool embedsNurseryPointers() const {
         return embedsNurseryPointers_;
     }