Bug 1160884 - Add KeepAlive instructions after elements/slots uses. r=nbp, a=abillings
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -2191,16 +2191,22 @@ CodeGenerator::visitNurseryObject(LNurse
// Store a dummy JSObject pointer. We will fix it up on the main thread,
// in JitCode::fixupNurseryObjects. The low bit is set to distinguish
// it from a real JSObject pointer.
JSObject* ptr = reinterpret_cast<JSObject*>((uintptr_t(index) << 1) | 1);
masm.movePtr(ImmGCPtr(IonNurseryPtr(ptr)), output);
}
void
+CodeGenerator::visitKeepAliveObject(LKeepAliveObject* lir)
+{
+ // No-op.
+}
+
+void
CodeGenerator::visitSlots(LSlots* lir)
{
Address slots(ToRegister(lir->object()), NativeObject::offsetOfSlots());
masm.loadPtr(slots, ToRegister(lir->output()));
}
void
CodeGenerator::visitLoadSlotT(LLoadSlotT* lir)
--- a/js/src/jit/CodeGenerator.h
+++ b/js/src/jit/CodeGenerator.h
@@ -107,16 +107,17 @@ class CodeGenerator : public CodeGenerat
void visitOutOfLineRegExpTest(OutOfLineRegExpTest* ool);
void visitRegExpReplace(LRegExpReplace* lir);
void visitStringReplace(LStringReplace* lir);
void visitLambda(LLambda* lir);
void visitLambdaArrow(LLambdaArrow* lir);
void visitLambdaForSingleton(LLambdaForSingleton* lir);
void visitPointer(LPointer* lir);
void visitNurseryObject(LNurseryObject* lir);
+ void visitKeepAliveObject(LKeepAliveObject* lir);
void visitSlots(LSlots* lir);
void visitLoadSlotT(LLoadSlotT* lir);
void visitLoadSlotV(LLoadSlotV* lir);
void visitStoreSlotT(LStoreSlotT* lir);
void visitStoreSlotV(LStoreSlotV* lir);
void visitElements(LElements* lir);
void visitConvertElementsToDoubles(LConvertElementsToDoubles* lir);
void visitMaybeToDoubleElement(LMaybeToDoubleElement* lir);
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -1481,16 +1481,23 @@ OptimizeMIR(MIRGenerator* mir)
// code motion after this pass could incorrectly move a load or store
// before its bounds check.
if (!EliminateRedundantChecks(graph))
return false;
IonSpewPass("Bounds Check Elimination");
AssertGraphCoherency(graph);
}
+ if (!mir->compilingAsmJS()) {
+ AutoTraceLog log(logger, TraceLogger_AddKeepAliveInstructions);
+ AddKeepAliveInstructions(graph);
+ IonSpewPass("Add KeepAlive Instructions");
+ AssertGraphCoherency(graph);
+ }
+
return true;
}
LIRGraph*
GenerateLIR(MIRGenerator* mir)
{
MIRGraph& graph = mir->graph();
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -2726,16 +2726,127 @@ jit::EliminateRedundantChecks(MIRGraph&
for (size_t i = 0; i < eliminateList.length(); i++) {
MDefinition* def = eliminateList[i];
def->block()->discardDef(def);
}
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:
+ 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)
{
for (size_t i = 0; i < terms_.length(); i++) {
if (!SafeMul(scale, terms_[i].scale, &terms_[i].scale))
return false;
}
return SafeMul(scale, constant_, &constant_);
--- a/js/src/jit/IonAnalysis.h
+++ b/js/src/jit/IonAnalysis.h
@@ -78,16 +78,19 @@ void
AssertGraphCoherency(MIRGraph& graph);
void
AssertExtendedGraphCoherency(MIRGraph& graph);
bool
EliminateRedundantChecks(MIRGraph& graph);
+void
+AddKeepAliveInstructions(MIRGraph& graph);
+
class MDefinition;
// Simple linear sum of the form 'n' or 'x + n'.
struct SimpleLinearSum
{
MDefinition* term;
int32_t constant;
--- a/js/src/jit/LIR-Common.h
+++ b/js/src/jit/LIR-Common.h
@@ -4065,16 +4065,30 @@ class LLambdaArrow : public LInstruction
const LDefinition* temp() {
return getTemp(0);
}
const MLambdaArrow* mir() const {
return mir_->toLambdaArrow();
}
};
+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
class LSlots : public LInstructionHelper<1, 1, 0>
{
public:
LIR_HEADER(Slots)
--- a/js/src/jit/LOpcodes.h
+++ b/js/src/jit/LOpcodes.h
@@ -189,16 +189,17 @@
_(RegExpExec) \
_(RegExpTest) \
_(RegExpReplace) \
_(StringReplace) \
_(Substr) \
_(Lambda) \
_(LambdaArrow) \
_(LambdaForSingleton) \
+ _(KeepAliveObject) \
_(Slots) \
_(Elements) \
_(ConvertElementsToDoubles) \
_(MaybeToDoubleElement) \
_(MaybeCopyElementsForWrite) \
_(LoadSlotV) \
_(LoadSlotT) \
_(StoreSlotV) \
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -2188,16 +2188,25 @@ LIRGenerator::visitLambdaArrow(MLambdaAr
LLambdaArrow* lir = new(alloc()) LLambdaArrow(useRegister(ins->scopeChain()), temp());
useBox(lir, LLambdaArrow::ThisValue, ins->thisDef());
define(lir, ins);
assignSafepoint(lir, ins);
}
void
+LIRGenerator::visitKeepAliveObject(MKeepAliveObject* ins)
+{
+ MDefinition* obj = ins->object();
+ MOZ_ASSERT(obj->type() == MIRType_Object);
+
+ add(new(alloc()) LKeepAliveObject(useKeepalive(obj)), ins);
+}
+
+void
LIRGenerator::visitSlots(MSlots* ins)
{
define(new(alloc()) LSlots(useRegisterAtStart(ins->object())), ins);
}
void
LIRGenerator::visitElements(MElements* ins)
{
--- a/js/src/jit/Lowering.h
+++ b/js/src/jit/Lowering.h
@@ -157,16 +157,17 @@ class LIRGenerator : public LIRGenerator
void visitToObjectOrNull(MToObjectOrNull* convert);
void visitRegExp(MRegExp* ins);
void visitRegExpExec(MRegExpExec* ins);
void visitRegExpTest(MRegExpTest* ins);
void visitRegExpReplace(MRegExpReplace* ins);
void visitStringReplace(MStringReplace* ins);
void visitLambda(MLambda* ins);
void visitLambdaArrow(MLambdaArrow* ins);
+ void visitKeepAliveObject(MKeepAliveObject* ins);
void visitSlots(MSlots* ins);
void visitElements(MElements* ins);
void visitConstantElements(MConstantElements* ins);
void visitConvertElementsToDoubles(MConvertElementsToDoubles* ins);
void visitMaybeToDoubleElement(MMaybeToDoubleElement* ins);
void visitMaybeCopyElementsForWrite(MMaybeCopyElementsForWrite* ins);
void visitLoadSlot(MLoadSlot* ins);
void visitFunctionEnvironment(MFunctionEnvironment* ins);
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -8014,16 +8014,39 @@ class MSetTypedObjectOffset
AliasSet getAliasSet() const override {
// This affects the result of MTypedObjectElements,
// which is described as a load of ObjectFields.
return AliasSet::Store(AliasSet::ObjectFields);
}
};
+class MKeepAliveObject
+ : public MUnaryInstruction,
+ public SingleObjectPolicy::Data
+{
+ 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);
+ }
+};
+
// Perform !-operation
class MNot
: public MUnaryInstruction,
public TestPolicy::Data
{
bool operandMightEmulateUndefined_;
bool operandIsNeverNaN_;
--- a/js/src/jit/MOpcodes.h
+++ b/js/src/jit/MOpcodes.h
@@ -137,16 +137,17 @@ namespace jit {
_(LimitedTruncate) \
_(RegExp) \
_(RegExpExec) \
_(RegExpTest) \
_(RegExpReplace) \
_(StringReplace) \
_(Lambda) \
_(LambdaArrow) \
+ _(KeepAliveObject) \
_(Slots) \
_(Elements) \
_(ConstantElements) \
_(ConvertElementsToDoubles) \
_(MaybeToDoubleElement) \
_(MaybeCopyElementsForWrite) \
_(LoadSlot) \
_(StoreSlot) \
--- a/js/src/jit/shared/Lowering-shared-inl.h
+++ b/js/src/jit/shared/Lowering-shared-inl.h
@@ -362,21 +362,27 @@ LAllocation
LIRGeneratorShared::useStorableAtStart(MDefinition* mir)
{
return useRegisterOrConstantAtStart(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
LIRGeneratorShared::useFixed(MDefinition* mir, Register reg)
{
return use(mir, LUse(reg));
}
--- a/js/src/jit/shared/Lowering-shared.h
+++ b/js/src/jit/shared/Lowering-shared.h
@@ -100,16 +100,17 @@ class LIRGeneratorShared : public MDefin
// slots on X86, and only registers on ARM.
inline LAllocation useAny(MDefinition* mir);
inline LAllocation useAnyOrConstant(MDefinition* mir);
// "Storable" is architecture dependend, and will include registers and
// constants on X86 and only registers on ARM. 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);
inline LAllocation useRegisterOrZeroAtStart(MDefinition* mir);
inline LAllocation useRegisterOrNonDoubleConstant(MDefinition* mir);
inline LUse useRegisterForTypedLoad(MDefinition* mir, MIRType type);
--- a/js/src/vm/TraceLoggingTypes.h
+++ b/js/src/vm/TraceLoggingTypes.h
@@ -50,16 +50,17 @@
_(LICM) \
_(RangeAnalysis) \
_(LoopUnrolling) \
_(EffectiveAddressAnalysis) \
_(AlignmentMaskAnalysis) \
_(EliminateDeadCode) \
_(EdgeCaseAnalysis) \
_(EliminateRedundantChecks) \
+ _(AddKeepAliveInstructions) \
_(GenerateLIR) \
_(RegisterAllocation) \
_(GenerateCode)
#define TRACELOGGER_LOG_ITEMS(_) \
_(Bailout) \
_(Disable) \
_(Enable) \