From 99641aa4446dc9df04dcfeede8b49ff03abcac42 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Thu, 28 May 2015 10:16:24 +0200 Subject: [PATCH] Bug 1160884 - Add KeepAlive instructions after elements/slots uses. r=nbp, a=abillings --- js/src/jit/CodeGenerator.cpp | 7 ++ js/src/jit/CodeGenerator.h | 1 + js/src/jit/Ion.cpp | 7 ++ js/src/jit/IonAnalysis.cpp | 112 ++++++++++++++++++++++++++++++++ js/src/jit/IonAnalysis.h | 3 + js/src/jit/LIR-Common.h | 14 ++++ js/src/jit/LOpcodes.h | 1 + js/src/jit/Lowering.cpp | 9 +++ js/src/jit/Lowering.h | 1 + js/src/jit/MIR.h | 26 ++++++++ js/src/jit/MOpcodes.h | 1 + js/src/jit/ParallelSafetyAnalysis.cpp | 1 + js/src/jit/shared/Lowering-shared-inl.h | 8 ++- js/src/jit/shared/Lowering-shared.h | 1 + js/src/vm/TraceLogging.h | 3 +- 15 files changed, 193 insertions(+), 2 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 5dff9df..7364178 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -1476,6 +1476,13 @@ CodeGenerator::visitPointer(LPointer* lir) } bool +CodeGenerator::visitKeepAliveObject(LKeepAliveObject* lir) +{ + // No-op. + return true; +} + +bool CodeGenerator::visitSlots(LSlots* lir) { Address slots(ToRegister(lir->object()), JSObject::offsetOfSlots()); diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h index 95fb33b..e3b4fd7 100644 --- a/js/src/jit/CodeGenerator.h +++ b/js/src/jit/CodeGenerator.h @@ -106,6 +106,7 @@ class CodeGenerator : public CodeGeneratorSpecific bool visitLambdaForSingleton(LLambdaForSingleton* lir); bool visitLambdaPar(LLambdaPar* lir); bool visitPointer(LPointer* lir); + bool visitKeepAliveObject(LKeepAliveObject* lir); bool visitSlots(LSlots* lir); bool visitStoreSlotV(LStoreSlotV* store); bool visitElements(LElements* lir); diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp index 015d387..1551a80 100644 --- a/js/src/jit/Ion.cpp +++ b/js/src/jit/Ion.cpp @@ -1536,6 +1536,13 @@ OptimizeMIR(MIRGenerator* mir) AssertGraphCoherency(graph); } + if (!mir->compilingAsmJS()) { + AutoTraceLog log(logger, TraceLogger::AddKeepAliveInstructions); + AddKeepAliveInstructions(graph); + IonSpewPass("Add KeepAlive Instructions"); + AssertGraphCoherency(graph); + } + return true; } diff --git a/js/src/jit/IonAnalysis.cpp b/js/src/jit/IonAnalysis.cpp index 8965724..af58aae 100644 --- a/js/src/jit/IonAnalysis.cpp +++ b/js/src/jit/IonAnalysis.cpp @@ -1971,6 +1971,118 @@ jit::UnsplitEdges(LIRGraph* lir) return true; } +static bool +NeedsKeepAlive(MInstruction* slotsOrElements, MInstruction* use) +{ + MOZ_ASSERT(slotsOrElements->type() == MIRType_Elements || + slotsOrElements->type() == MIRType_Slots); + + if (slotsOrElements->block() != use->block()) + return true; + + MBasicBlock* block = use->block(); + MInstructionIterator iter(block->begin(slotsOrElements)); + MOZ_ASSERT(*iter == slotsOrElements); + ++iter; + + while (true) { + if (*iter == use) + return false; + + switch (iter->op()) { + case MDefinition::Op_Nop: + case MDefinition::Op_Constant: + case MDefinition::Op_KeepAliveObject: + case MDefinition::Op_Unbox: + case MDefinition::Op_LoadSlot: + case MDefinition::Op_StoreSlot: + case MDefinition::Op_LoadFixedSlot: + case MDefinition::Op_StoreFixedSlot: + case MDefinition::Op_LoadElement: + case MDefinition::Op_StoreElement: + case MDefinition::Op_InitializedLength: + case MDefinition::Op_ArrayLength: + case MDefinition::Op_BoundsCheck: + iter++; + break; + default: + return true; + } + } + + MOZ_CRASH("Unreachable"); +} + +void +jit::AddKeepAliveInstructions(MIRGraph& graph) +{ + for (MBasicBlockIterator i(graph.begin()); i != graph.end(); i++) { + MBasicBlock* block = *i; + + for (MInstructionIterator insIter(block->begin()); insIter != block->end(); insIter++) { + MInstruction* ins = *insIter; + if (ins->type() != MIRType_Elements && ins->type() != MIRType_Slots) + continue; + + MDefinition* ownerObject; + switch (ins->op()) { + case MDefinition::Op_ConstantElements: + case MDefinition::Op_NewSlots: + continue; + case MDefinition::Op_ConvertElementsToDoubles: + // EliminateRedundantChecks should have replaced all uses. + MOZ_ASSERT(!ins->hasUses()); + continue; + case MDefinition::Op_Elements: + case MDefinition::Op_TypedArrayElements: + case MDefinition::Op_TypedObjectElements: + MOZ_ASSERT(ins->numOperands() == 1); + ownerObject = ins->getOperand(0); + break; + case MDefinition::Op_Slots: + ownerObject = ins->toSlots()->object(); + break; + default: + MOZ_CRASH("Unexpected op"); + } + + MOZ_ASSERT(ownerObject->type() == MIRType_Object); + + if (ownerObject->isConstant()) { + // Constants are kept alive by other pointers, for instance + // ImmGCPtr in JIT code. + continue; + } + + for (MUseDefIterator uses(ins); uses; uses++) { + MInstruction* use = uses.def()->toInstruction(); + + if (use->isStoreElementHole()) { + // StoreElementHole has an explicit object operand. If GVN + // is disabled, we can get different unbox instructions with + // the same object as input, so we check for that case. + MOZ_ASSERT_IF(!use->toStoreElementHole()->object()->isUnbox() && !ownerObject->isUnbox(), + use->toStoreElementHole()->object() == ownerObject); + continue; + } + + if (use->isInArray()) { + // See StoreElementHole case above. + MOZ_ASSERT_IF(!use->toInArray()->object()->isUnbox() && !ownerObject->isUnbox(), + use->toInArray()->object() == ownerObject); + continue; + } + + if (!NeedsKeepAlive(ins, use)) + continue; + + MKeepAliveObject* keepAlive = MKeepAliveObject::New(graph.alloc(), ownerObject); + use->block()->insertAfter(use, keepAlive); + } + } + } +} + bool LinearSum::multiply(int32_t scale) { diff --git a/js/src/jit/IonAnalysis.h b/js/src/jit/IonAnalysis.h index aabf835..a320418 100644 --- a/js/src/jit/IonAnalysis.h +++ b/js/src/jit/IonAnalysis.h @@ -64,6 +64,9 @@ AssertExtendedGraphCoherency(MIRGraph& graph); bool EliminateRedundantChecks(MIRGraph& graph); +void +AddKeepAliveInstructions(MIRGraph& graph); + bool UnsplitEdges(LIRGraph* lir); diff --git a/js/src/jit/LIR-Common.h b/js/src/jit/LIR-Common.h index 5fe0ee9..6b03a42 100644 --- a/js/src/jit/LIR-Common.h +++ b/js/src/jit/LIR-Common.h @@ -3591,6 +3591,20 @@ class LImplicitThis : public LInstructionHelper } }; +class LKeepAliveObject : public LInstructionHelper<0, 1, 0> +{ + public: + LIR_HEADER(KeepAliveObject) + + explicit LKeepAliveObject(const LAllocation& object) { + setOperand(0, object); + } + + const LAllocation* object() { + return getOperand(0); + } +}; + // Load the "slots" member out of a JSObject. // Input: JSObject pointer // Output: slots pointer diff --git a/js/src/jit/LOpcodes.h b/js/src/jit/LOpcodes.h index cd7eef8..424b22c 100644 --- a/js/src/jit/LOpcodes.h +++ b/js/src/jit/LOpcodes.h @@ -166,6 +166,7 @@ _(LambdaForSingleton) \ _(LambdaPar) \ _(ImplicitThis) \ + _(KeepAliveObject) \ _(Slots) \ _(Elements) \ _(ConvertElementsToDoubles) \ diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index d671fd4..c0d434e 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -2110,6 +2110,15 @@ LIRGenerator::visitImplicitThis(MImplicitThis* ins) } bool +LIRGenerator::visitKeepAliveObject(MKeepAliveObject* ins) +{ + MDefinition* obj = ins->object(); + MOZ_ASSERT(obj->type() == MIRType_Object); + + return add(new(alloc()) LKeepAliveObject(useKeepalive(obj)), ins); +} + +bool LIRGenerator::visitSlots(MSlots* ins) { return define(new(alloc()) LSlots(useRegisterAtStart(ins->object())), ins); diff --git a/js/src/jit/Lowering.h b/js/src/jit/Lowering.h index ea50cab..a60dc30 100644 --- a/js/src/jit/Lowering.h +++ b/js/src/jit/Lowering.h @@ -160,6 +160,7 @@ class LIRGenerator : public LIRGeneratorSpecific bool visitLambdaArrow(MLambdaArrow* ins); bool visitLambdaPar(MLambdaPar* ins); bool visitImplicitThis(MImplicitThis* ins); + bool visitKeepAliveObject(MKeepAliveObject* ins); bool visitSlots(MSlots* ins); bool visitElements(MElements* ins); bool visitConstantElements(MConstantElements* ins); diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index 48e1dfb..a6060a2 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -5790,6 +5790,32 @@ class MSetTypedObjectOffset } }; +class MKeepAliveObject + : public MUnaryInstruction, + public SingleObjectPolicy +{ + explicit MKeepAliveObject(MDefinition* object) + : MUnaryInstruction(object) + { + setResultType(MIRType_None); + setGuard(); + } + + public: + INSTRUCTION_HEADER(KeepAliveObject) + + static MKeepAliveObject* New(TempAllocator& alloc, MDefinition* object) { + return new(alloc) MKeepAliveObject(object); + } + + MDefinition* object() const { + return getOperand(0); + } + TypePolicy* typePolicy() { + return this; + } +}; + // Perform !-operation class MNot : public MUnaryInstruction, diff --git a/js/src/jit/MOpcodes.h b/js/src/jit/MOpcodes.h index 83b9e63..cfc3895 100644 --- a/js/src/jit/MOpcodes.h +++ b/js/src/jit/MOpcodes.h @@ -110,6 +110,7 @@ namespace jit { _(Lambda) \ _(LambdaArrow) \ _(ImplicitThis) \ + _(KeepAliveObject) \ _(Slots) \ _(Elements) \ _(ConstantElements) \ diff --git a/js/src/jit/ParallelSafetyAnalysis.cpp b/js/src/jit/ParallelSafetyAnalysis.cpp index a6a1202..13c577b 100644 --- a/js/src/jit/ParallelSafetyAnalysis.cpp +++ b/js/src/jit/ParallelSafetyAnalysis.cpp @@ -199,6 +199,7 @@ class ParallelSafetyVisitor : public MInstructionVisitor CUSTOM_OP(Lambda) UNSAFE_OP(LambdaArrow) UNSAFE_OP(ImplicitThis) + SAFE_OP(KeepAliveObject) SAFE_OP(Slots) SAFE_OP(Elements) SAFE_OP(ConstantElements) diff --git a/js/src/jit/shared/Lowering-shared-inl.h b/js/src/jit/shared/Lowering-shared-inl.h index 17bb74a..832cc61 100644 --- a/js/src/jit/shared/Lowering-shared-inl.h +++ b/js/src/jit/shared/Lowering-shared-inl.h @@ -372,11 +372,17 @@ LIRGeneratorShared::useStorableAtStart(MDefinition* mir) #endif LAllocation +LIRGeneratorShared::useKeepalive(MDefinition* mir) +{ + return use(mir, LUse(LUse::KEEPALIVE)); +} + +LAllocation LIRGeneratorShared::useKeepaliveOrConstant(MDefinition* mir) { if (mir->isConstant()) return LAllocation(mir->toConstant()->vp()); - return use(mir, LUse(LUse::KEEPALIVE)); + return useKeepalive(mir); } LUse diff --git a/js/src/jit/shared/Lowering-shared.h b/js/src/jit/shared/Lowering-shared.h index 4bd13b0..b23d20e 100644 --- a/js/src/jit/shared/Lowering-shared.h +++ b/js/src/jit/shared/Lowering-shared.h @@ -85,6 +85,7 @@ class LIRGeneratorShared : public MInstructionVisitorWithDefaults // this is a generic "things we can expect to write into memory in 1 instruction" inline LAllocation useStorable(MDefinition* mir); inline LAllocation useStorableAtStart(MDefinition* mir); + inline LAllocation useKeepalive(MDefinition* mir); inline LAllocation useKeepaliveOrConstant(MDefinition* mir); inline LAllocation useRegisterOrConstant(MDefinition* mir); inline LAllocation useRegisterOrConstantAtStart(MDefinition* mir); diff --git a/js/src/vm/TraceLogging.h b/js/src/vm/TraceLogging.h index 4c2ebfe..8447679 100644 --- a/js/src/vm/TraceLogging.h +++ b/js/src/vm/TraceLogging.h @@ -145,7 +145,8 @@ namespace jit { _(EffectiveAddressAnalysis) \ _(EliminateDeadCode) \ _(EdgeCaseAnalysis) \ - _(EliminateRedundantChecks) + _(EliminateRedundantChecks) \ + _(AddKeepAliveInstructions) class AutoTraceLog; -- 2.4.3