Skip to content

Commit

Permalink
NewScObj refactor - Splits NewScObj into multiple bytecodes with 4 di…
Browse files Browse the repository at this point in the history 
…stinct parts: GenCtorObj (makes ctor object), NewScObj (calls ctor with ctor object), determine ret val (done using multiple bytecode instrs), UpdateNewScObjCache.
  • Loading branch information
@wyrichte
wyrichte committed Jul 16, 2019
1 parent 68b7a26 commit 3a216cb
Show file tree
Hide file tree
Showing 37 changed files with 21,396 additions and 20,980 deletions.
10 changes: 7 additions & 3 deletions lib/Backend/GlobOpt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1415,9 +1415,12 @@ GlobOpt::TrackInstrsForScopeObjectRemoval(IR::Instr * instr)
//So we don't want to track the stack sym for this argout.- Skipping it here.
if (instr->m_func->IsInlinedConstructor())
{
IR::Instr* src1InstrDef = argOutInstr->GetSrc1()->GetStackSym()->GetInstrDef();
//PRE might introduce a second defintion for the Src1. So assert for the opcode only when it has single definition.
Assert(argOutInstr->GetSrc1()->GetStackSym()->GetInstrDef() == nullptr ||
argOutInstr->GetSrc1()->GetStackSym()->GetInstrDef()->m_opcode == Js::OpCode::NewScObjectNoCtor);
Assert(src1InstrDef == nullptr ||
src1InstrDef->m_opcode == Js::OpCode::NewScObjectNoCtor ||
src1InstrDef->m_opcode == Js::OpCode::GenCtorObj
);
argOutInstr = argOutInstr->GetSrc2()->GetStackSym()->GetInstrDef();
}
if (formalsCount < actualsCount)
Expand Down Expand Up @@ -2499,7 +2502,7 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
CSEOptimize(this->currentBlock, &instr, &src1Val, &src2Val, &src1IndirIndexVal);
OptimizeChecks(instr);
OptArraySrc(&instr, &src1Val, &src2Val);
OptNewScObject(&instr, src1Val);
OptGenCtorObj(&instr, src1Val);
OptStackArgLenAndConst(instr, &src1Val);

instr = this->OptPeep(instr, src1Val, src2Val);
Expand Down Expand Up @@ -13862,6 +13865,7 @@ GlobOpt::CheckJsArrayKills(IR::Instr *const instr)

case Js::OpCode::NewScObjectNoCtor:
case Js::OpCode::NewScObjectNoCtorFull:
case Js::OpCode::GenCtorObj:
if(doNativeArrayTypeSpec)
{
// Class/object construction can make something a prototype
Expand Down
2 changes: 1 addition & 1 deletion lib/Backend/GlobOpt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ class GlobOpt

IR::Instr * GetExtendedArg(IR::Instr *instr);

void OptNewScObject(IR::Instr** instrPtr, Value* srcVal);
void OptGenCtorObj(IR::Instr** instrPtr, Value* srcVal);
template <typename T>
bool OptConstFoldBinaryWasm(IR::Instr * *pInstr, const Value* src1, const Value* src2, Value **pDstVal);
template <typename T>
Expand Down
13 changes: 8 additions & 5 deletions lib/Backend/GlobOptBailOut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ void GlobOpt::EndTrackCall(IR::Instr* instr)


#if DBG
uint origArgOutCount = this->currentBlock->globOptData.argOutCount;
// uint origArgOutCount = this->currentBlock->globOptData.argOutCount;
#endif
while (this->currentBlock->globOptData.callSequence->Head()->GetStackSym()->HasArgSlotNum())
{
Expand All @@ -928,10 +928,12 @@ void GlobOpt::EndTrackCall(IR::Instr* instr)

// Number of argument set should be the same as indicated at StartCall
// except NewScObject has an implicit arg1
Assert((uint)sym->m_instrDef->GetArgOutCount(/*getInterpreterArgOutCount*/ true) ==
origArgOutCount - this->currentBlock->globOptData.argOutCount +
(instr->m_opcode == Js::OpCode::NewScObject || instr->m_opcode == Js::OpCode::NewScObjArray
|| instr->m_opcode == Js::OpCode::NewScObjectSpread || instr->m_opcode == Js::OpCode::NewScObjArraySpread));

// TODO: get working again!
//Assert((uint)sym->m_instrDef->GetArgOutCount(/*getInterpreterArgOutCount*/ true) ==
// origArgOutCount - this->currentBlock->globOptData.argOutCount +
// (instr->m_opcode == Js::OpCode::NewScObject || instr->m_opcode == Js::OpCode::NewScObjArray
// || instr->m_opcode == Js::OpCode::NewScObjectSpread || instr->m_opcode == Js::OpCode::NewScObjArraySpread));

#endif

Expand Down Expand Up @@ -1499,6 +1501,7 @@ GlobOpt::MayNeedBailOnImplicitCall(IR::Instr const * instr, Value const * src1Va
);
}

case Js::OpCode::GenCtorObj:
case Js::OpCode::NewScObjectNoCtor:
if (instr->HasBailOutInfo() && (instr->GetBailOutKind() & ~IR::BailOutKindBits) == IR::BailOutFailedCtorGuardCheck)
{
Expand Down
12 changes: 7 additions & 5 deletions lib/Backend/GlobOptFields.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse<JitArenaAllocator> *bv, bo
case Js::OpCode::InitProto:
case Js::OpCode::NewScObjectNoCtor:
case Js::OpCode::NewScObjectNoCtorFull:
case Js::OpCode::GenCtorObj:
if (inGlobOpt)
{
// Opcodes that make an object into a prototype may break object-header-inlining and final type opt.
Expand Down Expand Up @@ -1580,24 +1581,23 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
}

void
GlobOpt::OptNewScObject(IR::Instr** instrPtr, Value* srcVal)
GlobOpt::OptGenCtorObj(IR::Instr** instrPtr, Value* srcVal)
{
IR::Instr *&instr = *instrPtr;

if (!instr->IsNewScObjectInstr() || IsLoopPrePass() || !this->DoFieldRefOpts() || PHASE_OFF(Js::ObjTypeSpecNewObjPhase, this->func))
if (instr->m_opcode != Js::OpCode::GenCtorObj || IsLoopPrePass() || !this->DoFieldRefOpts() || PHASE_OFF(Js::ObjTypeSpecNewObjPhase, this->func))
{
return;
}

bool isCtorInlined = instr->m_opcode == Js::OpCode::NewScObjectNoCtor;
const JITTimeConstructorCache * ctorCache = instr->IsProfiledInstr() ?
instr->m_func->GetConstructorCache(static_cast<Js::ProfileId>(instr->AsProfiledInstr()->u.profileId)) : nullptr;

// TODO: OOP JIT, enable assert
//Assert(ctorCache == nullptr || srcVal->GetValueInfo()->IsVarConstant() && Js::VarIs<Js::JavascriptFunction>(srcVal->GetValueInfo()->AsVarConstant()->VarValue()));
Assert(ctorCache == nullptr || !ctorCache->IsTypeFinal() || ctorCache->CtorHasNoExplicitReturnValue());

if (ctorCache != nullptr && !ctorCache->SkipNewScObject() && (isCtorInlined || ctorCache->IsTypeFinal()))
if (ctorCache != nullptr && !ctorCache->SkipNewScObject())
{
GenerateBailAtOperation(instrPtr, IR::BailOutFailedCtorGuardCheck);
}
Expand All @@ -1616,7 +1616,7 @@ GlobOpt::ValueNumberObjectType(IR::Opnd *dstOpnd, IR::Instr *instr)
return;
}

if (instr->IsNewScObjectInstr())
if (instr->IsNewScObjectInstr()/*|| instr->m_opcode == Js::OpCode::GenCtorObj*/)
{
// If we have a NewScObj* for which we have a valid constructor cache we know what type the created object will have.
// Let's produce the type value accordingly so we don't insert a type check and bailout in the constructor and
Expand All @@ -1628,6 +1628,8 @@ GlobOpt::ValueNumberObjectType(IR::Opnd *dstOpnd, IR::Instr *instr)
Assert(instr->IsProfiledInstr());
Assert(instr->GetBailOutKind() == IR::BailOutFailedCtorGuardCheck);

// TODO: possibly need to consider GenCtorObj for this path as well as
// determine if NewScObj was inlined only using GenCtorObj.
bool isCtorInlined = instr->m_opcode == Js::OpCode::NewScObjectNoCtor;
JITTimeConstructorCache * ctorCache = instr->m_func->GetConstructorCache(static_cast<Js::ProfileId>(instr->AsProfiledInstr()->u.profileId));
Assert(ctorCache != nullptr && (isCtorInlined || ctorCache->IsTypeFinal()));
Expand Down
111 changes: 109 additions & 2 deletions lib/Backend/IR.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3387,6 +3387,15 @@ bool Instr::CanHaveArgOutChain() const
this->m_opcode == Js::OpCode::NewScObjArraySpread;
}

bool Instr::IsNewScObjCallVariantInstr()
{
return
this->m_opcode == Js::OpCode::NewScObject ||
this->m_opcode == Js::OpCode::NewScObjectSpread ||
this->m_opcode == Js::OpCode::NewScObjArray ||
this->m_opcode == Js::OpCode::NewScObjArraySpread;
}

bool Instr::HasEmptyArgOutChain(IR::Instr** startCallInstrOut)
{
Assert(CanHaveArgOutChain());
Expand All @@ -3408,6 +3417,22 @@ bool Instr::HasEmptyArgOutChain(IR::Instr** startCallInstrOut)
return false;
}

uint Instr::ArgOutChainLength()
{
Assert(CanHaveArgOutChain());

uint length = 0;
Instr* currArgOutInstr = GetSrc2()->GetStackSym()->GetInstrDef();

while (currArgOutInstr->m_opcode != Js::OpCode::StartCall)
{
length++;
currArgOutInstr = currArgOutInstr->GetSrc2()->GetStackSym()->GetInstrDef();
}

return length;
}

bool Instr::HasFixedFunctionAddressTarget() const
{
Assert(
Expand All @@ -3417,14 +3442,96 @@ bool Instr::HasFixedFunctionAddressTarget() const
this->m_opcode == Js::OpCode::NewScObjectSpread ||
this->m_opcode == Js::OpCode::NewScObjArray ||
this->m_opcode == Js::OpCode::NewScObjArraySpread ||
this->m_opcode == Js::OpCode::NewScObjectNoCtor);
this->m_opcode == Js::OpCode::NewScObjectNoCtor ||
this->m_opcode == Js::OpCode::GenCtorObj);
return
this->GetSrc1() != nullptr &&
this->GetSrc1()->IsAddrOpnd() &&
this->GetSrc1()->AsAddrOpnd()->GetAddrOpndKind() == IR::AddrOpndKind::AddrOpndKindDynamicVar &&
this->GetSrc1()->AsAddrOpnd()->m_isFunction;
}

Instr* Instr::GetGenCtorInstr()
{
Assert(IsNewScObjCallVariantInstr());
Instr* currArgOutInstr = this;
Instr* currArgOutInstrValDef = this;
do
{
// TODO: should use helper method here? GetNextInstr?
Assert(currArgOutInstr->GetSrc2());
currArgOutInstr = currArgOutInstr->GetSrc2()->GetStackSym()->GetInstrDef();
Assert(currArgOutInstr);
if (currArgOutInstr->m_opcode == Js::OpCode::GenCtorObj)
{
return currArgOutInstr;
}
if (currArgOutInstr->m_opcode == Js::OpCode::LdSpreadIndices)
{
// This instr is a redirection, move on to next instr.
continue;
}
Assert(currArgOutInstr->m_opcode == Js::OpCode::ArgOut_A);
if (currArgOutInstr->GetSrc1()->IsAddrOpnd())
{
// This instr's src1 is not a symbol, thus it does not have a def instr
// and thus it cannot be from a GenCtorObj instr.
continue;
}

// Looking for the opcode GenCtorObj in currArgOutInstrValDef.
currArgOutInstrValDef = currArgOutInstr->GetSrc1()->GetStackSym()->GetInstrDef();
Assert(currArgOutInstrValDef);
if (currArgOutInstrValDef->m_opcode == Js::OpCode::BytecodeArgOutCapture)
{
if (currArgOutInstrValDef->GetSrc1()->GetStackSym())
{
// Indirection through BytecodeArgOutCapture.
currArgOutInstrValDef = currArgOutInstrValDef->GetSrc1()->GetStackSym()->GetInstrDef();
}
Assert(currArgOutInstrValDef);
}
} while (currArgOutInstrValDef->m_opcode != Js::OpCode::GenCtorObj);
return currArgOutInstrValDef;
}

Opnd* Instr::GetArgOutVal(uint argIndex)
{
Assert(IsNewScObjCallVariantInstr());
Instr* currArgOutInstr = this;
do
{
Assert(currArgOutInstr->GetSrc2());
currArgOutInstr = currArgOutInstr->GetSrc2()->GetStackSym()->GetInstrDef();
Assert(currArgOutInstr);
if (currArgOutInstr->m_opcode == Js::OpCode::StartCall)
{
// argIndex is larger than this argOutChain's length.
return nullptr;
}

if (currArgOutInstr->m_opcode == Js::OpCode::LdSpreadIndices)
{
// This instr is a redirection, move on to next instr.
continue;
}
Assert(currArgOutInstr->m_opcode == Js::OpCode::ArgOut_A);

if (argIndex > 0)
{
argIndex--;
}
} while (argIndex > 0);

Opnd* argOutVal = currArgOutInstr->GetSrc1();
if (argOutVal->GetStackSym()->m_instrDef->m_opcode == Js::OpCode::BytecodeArgOutCapture)
{
argOutVal = argOutVal->GetStackSym()->m_instrDef->GetSrc1();
}

return argOutVal;
}

bool Instr::TransfersSrcValue()
{
// Return whether the instruction transfers a value to the destination.
Expand Down Expand Up @@ -3635,7 +3742,7 @@ uint Instr::GetArgOutCount(bool getInterpreterArgOutCount)
opcode == Js::OpCode::EndCallForPolymorphicInlinee || opcode == Js::OpCode::LoweredStartCall);

Assert(!getInterpreterArgOutCount || opcode == Js::OpCode::StartCall);
uint argOutCount = !this->GetSrc2() || !getInterpreterArgOutCount || m_func->GetJITFunctionBody()->IsAsmJsMode()
uint argOutCount = !this->GetSrc2() || !getInterpreterArgOutCount || m_func->GetJITFunctionBody()->IsAsmJsMode()
? this->GetSrc1()->AsIntConstOpnd()->AsUint32()
: this->GetSrc2()->AsIntConstOpnd()->AsUint32();

Expand Down
10 changes: 9 additions & 1 deletion lib/Backend/IR.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ class Instr
isCallInstrProtectedByNoProfileBailout(false),
hasSideEffects(false),
isNonFastPathFrameDisplay(false),
isSafeToSpeculate(false)
isSafeToSpeculate(false),
isFixedCall(false),
genCtorInstrHasArgs(false)
#if DBG
, highlight(0)
, m_noLazyHelperAssert(false)
Expand Down Expand Up @@ -357,10 +359,14 @@ class Instr
static IR::Instr * CloneRange(Instr * instrStart, Instr * instrLast, Instr * instrInsert, Lowerer *lowerer, JitArenaAllocator *alloc, bool (*fMapTest)(IR::Instr*), bool clonedInstrGetOrigArgSlot);

bool CanHaveArgOutChain() const;
bool IsNewScObjCallVariantInstr();
bool HasEmptyArgOutChain(IR::Instr** startCallInstrOut = nullptr);
uint Instr::ArgOutChainLength();
bool HasFixedFunctionAddressTarget() const;
// Return whether the instruction transfer value from the src to the dst for copy prop
bool TransfersSrcValue();
Instr* GetGenCtorInstr();
Opnd* GetArgOutVal(uint argIndex);

#if ENABLE_DEBUG_CONFIG_OPTIONS
const char * GetBailOutKindName() const;
Expand Down Expand Up @@ -581,6 +587,8 @@ class Instr
bool isCallInstrProtectedByNoProfileBailout : 1;
bool hasSideEffects : 1; // The instruction cannot be dead stored
bool isNonFastPathFrameDisplay : 1;
bool isFixedCall : 1;
bool genCtorInstrHasArgs : 1;
protected:
bool isCloned : 1;
bool hasBailOutInfo : 1;
Expand Down
24 changes: 12 additions & 12 deletions lib/Backend/IRBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1779,6 +1779,7 @@ IRBuilder::BuildReg2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot R0, Js::Re
case Js::OpCode::SpreadObjectLiteral:
// fall through
case Js::OpCode::SetComputedNameVar:
case Js::OpCode::UpNewScObjCache:
{
IR::Instr *instr = IR::Instr::New(newOpcode, m_func);
instr->SetSrc1(this->BuildSrcOpnd(R0));
Expand All @@ -1805,6 +1806,17 @@ IRBuilder::BuildReg2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot R0, Js::Re
this->AddInstr(instr, offset);
return;
}

// A dest symbol that has a def but no use can be emitted. The symbol can then be reused
// as a dest of another instr. When this occurs the symbol cannot refer to either of the
// two def instrs as its instrDef. In order to ensure that a GenCtorObj instr can be
// referred to using its dest symbol, we force the dest symbol to be a single def. We
// want this property for GenCtorObj because NewScObj* instrs will need to refer to their
// related GenCtorObj instr, this is done through walking up the ArgOut chain and checking
// the ArgOuts' values' instrDefs for the GenCtorObj opcode.
case Js::OpCode::GenCtorObj:
SetTempUsed(R0, TRUE);
break;
}

IR::RegOpnd * dstOpnd = this->BuildDstOpnd(R0);
Expand All @@ -1820,7 +1832,6 @@ IRBuilder::BuildReg2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot R0, Js::Re
dstSym->m_builtInIndex = symSrc1->m_builtInIndex;
}
break;

case Js::OpCode::ProfiledStrictLdThis:
newOpcode = Js::OpCode::StrictLdThis;
if (m_func->HasProfileInfo())
Expand Down Expand Up @@ -6536,7 +6547,6 @@ IRBuilder::BuildCallI_Helper(Js::OpCode newOpcode, uint32 offset, Js::RegSlot ds
case Js::OpCode::NewScObjArray:
case Js::OpCode::NewScObjArraySpread:
symDst->m_isSafeThis = true;
symDst->m_isNotNumber = true;
break;
}
}
Expand All @@ -6551,7 +6561,6 @@ IRBuilder::BuildCallI_Helper(Js::OpCode newOpcode, uint32 offset, Js::RegSlot ds
void
IRBuilder::BuildCallCommon(IR::Instr * instr, StackSym * symDst, Js::ArgSlot argCount, Js::CallFlags flags)
{
Js::OpCode newOpcode = instr->m_opcode;

IR::Instr * argInstr = nullptr;
IR::Instr * prevInstr = instr;
Expand All @@ -6578,15 +6587,6 @@ IRBuilder::BuildCallCommon(IR::Instr * instr, StackSym * symDst, Js::ArgSlot arg
this->callTreeHasSomeProfileInfo = false;
}

if (newOpcode == Js::OpCode::NewScObject || newOpcode == Js::OpCode::NewScObjArray
|| newOpcode == Js::OpCode::NewScObjectSpread || newOpcode == Js::OpCode::NewScObjArraySpread)
{
#if DBG
count++;
#endif
m_argsOnStack++;
}

argCount = Js::CallInfo::GetArgCountWithExtraArgs(flags, argCount);

if (argInstr)
Expand Down
Loading

0 comments on commit 3a216cb

Please sign in to comment.