From 113d3120dbd7538f7a6578406b655a557dc00137 Mon Sep 17 00:00:00 2001 From: "Nicolas B. Pierron" Date: Tue, 1 Sep 2015 11:58:45 +0200 Subject: [PATCH] Bug 1184965 part 3 - Use RAII for redundant profilerPreCall and profilerPostReturn calls. r=djvj --- js/src/jit/MacroAssembler-inl.h | 6 +-- js/src/jit/MacroAssembler.cpp | 59 +++++++++++---------- js/src/jit/MacroAssembler.h | 77 +++++++++++++++------------- js/src/jit/shared/Assembler-shared.h | 5 -- 4 files changed, 74 insertions(+), 73 deletions(-) diff --git a/js/src/jit/MacroAssembler-inl.h b/js/src/jit/MacroAssembler-inl.h index bd9922e3c7f3..6c8778ee9deb 100644 --- a/js/src/jit/MacroAssembler-inl.h +++ b/js/src/jit/MacroAssembler-inl.h @@ -94,9 +94,8 @@ MacroAssembler::passABIArg(FloatRegister reg, MoveOp::Type type) template 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; } diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp index 44a78dc1e91d..cf2cb46e5391 100644 --- a/js/src/jit/MacroAssembler.cpp +++ b/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(); } } diff --git a/js/src/jit/MacroAssembler.h b/js/src/jit/MacroAssembler.h index a6c5c6ccf93e..840d0803d7c6 100644 --- a/js/src/jit/MacroAssembler.h +++ b/js/src/jit/MacroAssembler.h @@ -320,21 +320,16 @@ class MacroAssembler : public MacroAssemblerSpecific mozilla::Maybe 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 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 diff --git a/js/src/jit/shared/Assembler-shared.h b/js/src/jit/shared/Assembler-shared.h index 7c86d9073ffb..5f26b43c3019 100644 --- a/js/src/jit/shared/Assembler-shared.h +++ b/js/src/jit/shared/Assembler-shared.h @@ -914,7 +914,6 @@ class AssemblerShared Vector asmJSAbsoluteLinks_; protected: - Vector 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_; }