From dbc0dce92e1f0855be8239f3a2aad48e14744b2a Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Wed, 23 Dec 2015 11:28:54 +0100 Subject: [PATCH] Bug 1233818 part 1 - Use explicit interrupt checks in Ion for loops that affect GC. r=bhackett --- js/src/jit/CodeGenerator.cpp | 23 ++++---- js/src/jit/CodeGenerator.h | 4 +- js/src/jit/Lowering.cpp | 64 +++++++++++++++++----- js/src/jit/shared/CodeGenerator-shared.cpp | 6 +- js/src/jit/shared/LIR-shared.h | 32 ++++++----- js/src/jit/shared/LOpcodes-shared.h | 1 - js/src/jsgc.cpp | 10 +++- 7 files changed, 93 insertions(+), 47 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index e08280125860..dd2a8e69bc17 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -1966,9 +1966,9 @@ class OutOfLineInterruptCheckImplicit : public OutOfLineCodeBase { public: LBlock* block; - LInterruptCheckImplicit* lir; + LInterruptCheck* lir; - OutOfLineInterruptCheckImplicit(LBlock* block, LInterruptCheckImplicit* lir) + OutOfLineInterruptCheckImplicit(LBlock* block, LInterruptCheck* lir) : block(block), lir(lir) { } @@ -2010,16 +2010,6 @@ CodeGenerator::visitOutOfLineInterruptCheckImplicit(OutOfLineInterruptCheckImpli masm.jump(ool->rejoin()); } -void -CodeGenerator::visitInterruptCheckImplicit(LInterruptCheckImplicit* lir) -{ - OutOfLineInterruptCheckImplicit* ool = new(alloc()) OutOfLineInterruptCheckImplicit(current, lir); - addOutOfLineCode(ool, lir->mir()); - - lir->setOolEntry(ool->entry()); - masm.bind(ool->rejoin()); -} - void CodeGenerator::visitTableSwitch(LTableSwitch* ins) { @@ -10212,6 +10202,15 @@ CodeGenerator::visitAssertRangeV(LAssertRangeV* ins) void CodeGenerator::visitInterruptCheck(LInterruptCheck* lir) { + if (lir->implicit()) { + OutOfLineInterruptCheckImplicit* ool = new(alloc()) OutOfLineInterruptCheckImplicit(current, lir); + addOutOfLineCode(ool, lir->mir()); + + lir->setOolEntry(ool->entry()); + masm.bind(ool->rejoin()); + return; + } + OutOfLineCode* ool = oolCallVM(InterruptCheckInfo, lir, ArgList(), StoreNothing()); AbsoluteAddress interruptAddr(GetJitContext()->runtime->addressOfInterruptUint32()); diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h index 0467cd935b49..d3b035a8ef4b 100644 --- a/js/src/jit/CodeGenerator.h +++ b/js/src/jit/CodeGenerator.h @@ -351,9 +351,6 @@ class CodeGenerator : public CodeGeneratorSpecific void visitCheckOverRecursed(LCheckOverRecursed* lir); void visitCheckOverRecursedFailure(CheckOverRecursedFailure* ool); - void visitInterruptCheckImplicit(LInterruptCheckImplicit* ins); - void visitOutOfLineInterruptCheckImplicit(OutOfLineInterruptCheckImplicit* ins); - void visitUnboxFloatingPoint(LUnboxFloatingPoint* lir); void visitOutOfLineUnboxFloatingPoint(OutOfLineUnboxFloatingPoint* ool); void visitOutOfLineStoreElementHole(OutOfLineStoreElementHole* ool); @@ -387,6 +384,7 @@ class CodeGenerator : public CodeGeneratorSpecific void emitAssertObjectOrStringResult(Register input, MIRType type, const TemporaryTypeSet* typeset); void visitInterruptCheck(LInterruptCheck* lir); + void visitOutOfLineInterruptCheckImplicit(OutOfLineInterruptCheckImplicit* ins); void visitAsmJSInterruptCheck(LAsmJSInterruptCheck* lir); void visitRecompileCheck(LRecompileCheck* ins); diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index c6c77c985979..65ae9143d9c8 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -89,9 +89,59 @@ LIRGenerator::visitIsConstructing(MIsConstructing* ins) define(new(alloc()) LIsConstructing(), ins); } +static void +TryToUseImplicitInterruptCheck(MIRGraph& graph, MBasicBlock* backedge) +{ + // Implicit interrupt checks require asm.js signal handlers to be installed. + // They also require writable JIT code: reprotecting in patchIonBackedges + // would be expensive and using AutoWritableJitCode in the signal handler + // is complicated because there could be another AutoWritableJitCode on the + // stack. + if (!GetJitContext()->runtime->canUseSignalHandlers() || + ExecutableAllocator::nonWritableJitCode) + { + return; + } + + // To avoid triggering expensive interrupts (backedge patching) in + // requestMajorGC and requestMinorGC, use an implicit interrupt check only + // if the loop body can not trigger GC or affect GC state like the store + // buffer. We do this by checking there are no safepoints attached to LIR + // instructions inside the loop. + + MBasicBlockIterator block = graph.begin(backedge->loopHeaderOfBackedge()); + LInterruptCheck* check = nullptr; + while (true) { + LBlock* lir = block->lir(); + for (LInstructionIterator iter = lir->begin(); iter != lir->end(); iter++) { + if (iter->isInterruptCheck()) { + if (!check) { + MOZ_ASSERT(*block == backedge->loopHeaderOfBackedge()); + check = iter->toInterruptCheck(); + } + continue; + } + + MOZ_ASSERT_IF(iter->isPostWriteBarrierO() || iter->isPostWriteBarrierV(), + iter->safepoint()); + + if (iter->safepoint()) + return; + } + if (*block == backedge) + break; + block++; + } + + check->setImplicit(); +} + void LIRGenerator::visitGoto(MGoto* ins) { + if (!gen->compilingAsmJS() && ins->block()->isLoopBackedge()) + TryToUseImplicitInterruptCheck(graph, ins->block()); + add(new(alloc()) LGoto(ins->target())); } @@ -2304,19 +2354,7 @@ LIRGenerator::visitFunctionEnvironment(MFunctionEnvironment* ins) void LIRGenerator::visitInterruptCheck(MInterruptCheck* ins) { - // Implicit interrupt checks require asm.js signal handlers to be installed. - // They also require writable JIT code: reprotecting in patchIonBackedges - // would be expensive and using AutoWritableJitCode in the signal handler - // is complicated because there could be another AutoWritableJitCode on the - // stack. - LInstructionHelper<0, 0, 0>* lir; - if (GetJitContext()->runtime->canUseSignalHandlers() && - !ExecutableAllocator::nonWritableJitCode) - { - lir = new(alloc()) LInterruptCheckImplicit(); - } else { - lir = new(alloc()) LInterruptCheck(); - } + LInstruction* lir = new(alloc()) LInterruptCheck(); add(lir, ins); assignSafepoint(lir, ins); } diff --git a/js/src/jit/shared/CodeGenerator-shared.cpp b/js/src/jit/shared/CodeGenerator-shared.cpp index 95ec41c5ca5c..7a5bc1762719 100644 --- a/js/src/jit/shared/CodeGenerator-shared.cpp +++ b/js/src/jit/shared/CodeGenerator-shared.cpp @@ -1559,12 +1559,12 @@ CodeGeneratorShared::labelForBackedgeWithImplicitCheck(MBasicBlock* mir) for (LInstructionIterator iter = mir->lir()->begin(); iter != mir->lir()->end(); iter++) { if (iter->isMoveGroup()) { // Continue searching for an interrupt check. - } else if (iter->isInterruptCheckImplicit()) { - return iter->toInterruptCheckImplicit()->oolEntry(); } else { // The interrupt check should be the first instruction in the - // loop header other than the initial label and move groups. + // loop header other than move groups. MOZ_ASSERT(iter->isInterruptCheck()); + if (iter->toInterruptCheck()->implicit()) + return iter->toInterruptCheck()->oolEntry(); return nullptr; } } diff --git a/js/src/jit/shared/LIR-shared.h b/js/src/jit/shared/LIR-shared.h index 2a0508bdcbd2..7f3f207cbfb2 100644 --- a/js/src/jit/shared/LIR-shared.h +++ b/js/src/jit/shared/LIR-shared.h @@ -1212,35 +1212,41 @@ class LAsmJSInterruptCheck : public LInstructionHelper<0, 0, 0> }; class LInterruptCheck : public LInstructionHelper<0, 0, 0> -{ - public: - LIR_HEADER(InterruptCheck) -}; - -// Alternative to LInterruptCheck which does not emit an explicit check of the -// interrupt flag but relies on the loop backedge being patched via a signal -// handler. -class LInterruptCheckImplicit : public LInstructionHelper<0, 0, 0> { Label* oolEntry_; - public: - LIR_HEADER(InterruptCheckImplicit) + // Whether this is an implicit interrupt check. Implicit interrupt checks + // use a patchable backedge and signal handlers instead of an explicit + // rt->interrupt check. + bool implicit_; - LInterruptCheckImplicit() - : oolEntry_(nullptr) + public: + LIR_HEADER(InterruptCheck) + + LInterruptCheck() + : oolEntry_(nullptr), + implicit_(false) {} Label* oolEntry() { + MOZ_ASSERT(implicit_); return oolEntry_; } void setOolEntry(Label* oolEntry) { + MOZ_ASSERT(implicit_); oolEntry_ = oolEntry; } MInterruptCheck* mir() const { return mir_->toInterruptCheck(); } + + void setImplicit() { + implicit_ = true; + } + bool implicit() const { + return implicit_; + } }; class LDefVar : public LCallInstructionHelper<0, 1, 0> diff --git a/js/src/jit/shared/LOpcodes-shared.h b/js/src/jit/shared/LOpcodes-shared.h index 8f9164f3df1b..6f546df8d9e9 100644 --- a/js/src/jit/shared/LOpcodes-shared.h +++ b/js/src/jit/shared/LOpcodes-shared.h @@ -328,7 +328,6 @@ _(CallInstanceOf) \ _(InterruptCheck) \ _(AsmJSInterruptCheck) \ - _(InterruptCheckImplicit) \ _(GetDOMProperty) \ _(GetDOMMemberV) \ _(GetDOMMemberT) \ diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index f9586c0c4076..756fa15ab966 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -3134,7 +3134,11 @@ GCRuntime::requestMajorGC(JS::gcreason::Reason reason) return; majorGCTriggerReason = reason; - rt->requestInterrupt(JSRuntime::RequestInterruptUrgent); + + // There's no need to use RequestInterruptUrgent here. It's slower because + // it has to interrupt (looping) Ion code, but loops in Ion code that + // affect GC will have an explicit interrupt check. + rt->requestInterrupt(JSRuntime::RequestInterruptCanWait); } void @@ -3145,7 +3149,9 @@ GCRuntime::requestMinorGC(JS::gcreason::Reason reason) return; minorGCTriggerReason = reason; - rt->requestInterrupt(JSRuntime::RequestInterruptUrgent); + + // See comment in requestMajorGC. + rt->requestInterrupt(JSRuntime::RequestInterruptCanWait); } bool