Skip to content

Commit

Permalink
[1.10>master] [MERGE #5618 @rajatd] Copy-prop ScopedLdFld. OS#18622681
Browse files Browse the repository at this point in the history
Merge pull request #5618 from rajatd:scopedLdFld-2

Enable copy-prop of ScopedLdFld and ScopedLdFldForTypeOf.
  • Loading branch information
rajatd committed Aug 21, 2018
2 parents 5d23077 + 70aa49a commit d58fa97
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 18 deletions.
10 changes: 9 additions & 1 deletion lib/Backend/GlobOpt.cpp
Expand Up @@ -3289,6 +3289,7 @@ GlobOpt::OptSrc(IR::Opnd *opnd, IR::Instr * *pInstr, Value **indirIndexValRef, I
case Js::OpCode::BrOnHasProperty:
case Js::OpCode::LdMethodFldPolyInlineMiss:
case Js::OpCode::StSlotChkUndecl:
case Js::OpCode::ScopedLdInst:
return nullptr;
};

Expand Down Expand Up @@ -3718,6 +3719,8 @@ GlobOpt::CopyProp(IR::Opnd *opnd, IR::Instr *instr, Value *val, IR::IndirOpnd *p
case Js::OpCode::LdRootMethodFld:
case Js::OpCode::LdMethodFromFlags:
case Js::OpCode::ScopedLdMethodFld:
case Js::OpCode::ScopedLdFld:
case Js::OpCode::ScopedLdFldForTypeOf:
instr->m_opcode = Js::OpCode::Ld_A;
case Js::OpCode::Ld_A:
{
Expand Down Expand Up @@ -3956,6 +3959,8 @@ GlobOpt::CopyPropReplaceOpnd(IR::Instr * instr, IR::Opnd * opnd, StackSym * copy
case Js::OpCode::LdMethodFld:
case Js::OpCode::LdRootMethodFld:
case Js::OpCode::ScopedLdMethodFld:
case Js::OpCode::ScopedLdFld:
case Js::OpCode::ScopedLdFldForTypeOf:
instr->m_opcode = Js::OpCode::Ld_A;
break;

Expand Down Expand Up @@ -4682,13 +4687,16 @@ GlobOpt::ValueNumberDst(IR::Instr **pInstr, Value *src1Val, Value *src2Val)
case Js::OpCode::LdFld:
case Js::OpCode::LdFldForTypeOf:
case Js::OpCode::LdFldForCallApplyTarget:
// Do not transfer value type on ldFldForTypeOf to prevent copy-prop to LdRootFld in case the field doesn't exist since LdRootFldForTypeOf does not throw
// Do not transfer value type on LdRootFldForTypeOf to prevent copy-prop to LdRootFld in case the field doesn't exist since LdRootFldForTypeOf does not throw.
// Same goes for ScopedLdFldForTypeOf as we'll end up loading the property from the root object if the property is not in the scope chain.
//case Js::OpCode::LdRootFldForTypeOf:
//case Js::OpCode::ScopedLdFldForTypeOf:
case Js::OpCode::LdRootFld:
case Js::OpCode::LdMethodFld:
case Js::OpCode::LdRootMethodFld:
case Js::OpCode::ScopedLdMethodFld:
case Js::OpCode::LdMethodFromFlags:
case Js::OpCode::ScopedLdFld:
if (instr->IsProfiledInstr())
{
ValueType profiledValueType(instr->AsProfiledInstr()->u.FldInfo().valueType);
Expand Down
6 changes: 3 additions & 3 deletions lib/Backend/GlobOptFields.cpp
Expand Up @@ -566,8 +566,6 @@ GlobOpt::AssertCanCopyPropOrCSEFieldLoad(IR::Instr * instr)
// Consider: Hoisting LdRootFld may have complication with exception if the field doesn't exist.
// We need to have another opcode for the hoisted version to avoid the exception and bailout.

// Consider: Theoretically, we can copy prop/field hoist ScopedLdFld/ScopedStFld
// but Instr::TransferSrcValue blocks that now, and copy prop into that instruction is not supported yet.
Assert(instr->m_opcode == Js::OpCode::LdSlot || instr->m_opcode == Js::OpCode::LdSlotArr
|| instr->m_opcode == Js::OpCode::LdFld || instr->m_opcode == Js::OpCode::LdFldForCallApplyTarget
|| instr->m_opcode == Js::OpCode::LdLen_A
Expand All @@ -578,7 +576,9 @@ GlobOpt::AssertCanCopyPropOrCSEFieldLoad(IR::Instr * instr)
|| instr->m_opcode == Js::OpCode::LdMethodFromFlags
|| instr->m_opcode == Js::OpCode::ScopedLdMethodFld
|| instr->m_opcode == Js::OpCode::CheckFixedFld
|| instr->m_opcode == Js::OpCode::CheckPropertyGuardAndLoadType);
|| instr->m_opcode == Js::OpCode::CheckPropertyGuardAndLoadType
|| instr->m_opcode == Js::OpCode::ScopedLdFld
|| instr->m_opcode == Js::OpCode::ScopedLdFldForTypeOf);

Assert(instr->m_opcode == Js::OpCode::CheckFixedFld || instr->GetDst()->GetType() == TyVar || instr->m_func->GetJITFunctionBody()->IsAsmJsMode());
Assert(instr->GetSrc1()->GetType() == TyVar || instr->m_func->GetJITFunctionBody()->IsAsmJsMode());
Expand Down
1 change: 0 additions & 1 deletion lib/Backend/IR.cpp
Expand Up @@ -3305,7 +3305,6 @@ bool Instr::TransfersSrcValue()
// No point creating an unknown value for the src of a binary instr, as the dst will just be a different
// Don't create value for instruction without dst as well. The value doesn't go anywhere.

// if (src2 == nullptr) Disable copy prop for ScopedLdFld/ScopeStFld, etc., consider enabling that in the future
// Consider: Add opcode attribute to indicate whether the opcode would use the value or not

return this->GetDst() != nullptr && this->GetSrc2() == nullptr && !OpCodeAttr::DoNotTransfer(this->m_opcode) && !this->CallsAccessor();
Expand Down
10 changes: 4 additions & 6 deletions lib/Backend/IRBuilder.cpp
Expand Up @@ -3150,10 +3150,9 @@ IRBuilder::BuildElementScopedC(Js::OpCode newOpcode, uint32 offset, Js::RegSlot
case Js::OpCode::ScopedDeleteFld:
case Js::OpCode::ScopedDeleteFldStrict:
{
// Implicit root object as default instance
IR::Opnd * instance2Opnd = this->BuildSrcOpnd(Js::FunctionBody::RootObjectRegSlot);
Assert(this->m_func->GetScriptContextInfo()->GetAddr() == this->m_func->GetTopFunc()->GetScriptContextInfo()->GetAddr());
regOpnd = this->BuildDstOpnd(regSlot);
instr = IR::Instr::New(newOpcode, regOpnd, fieldSymOpnd, instance2Opnd, m_func);
instr = IR::Instr::New(newOpcode, regOpnd, fieldSymOpnd, m_func);
break;
}

Expand Down Expand Up @@ -4412,13 +4411,12 @@ IRBuilder::BuildElementP(Js::OpCode newOpcode, uint32 offset, Js::RegSlot regSlo
case Js::OpCode::ScopedLdFldForTypeOf:
{
Assert(!isProfiled);
Assert(this->m_func->GetScriptContextInfo()->GetAddr() == this->m_func->GetTopFunc()->GetScriptContextInfo()->GetAddr());

fieldSymOpnd = this->BuildFieldOpnd(newOpcode, instance, propertyId, (Js::PropertyIdIndexType)-1, PropertyKindData, inlineCacheIndex);

// Implicit root object as default instance
IR::Opnd * instance2Opnd = this->BuildSrcOpnd(Js::FunctionBody::RootObjectRegSlot);
regOpnd = this->BuildDstOpnd(regSlot);
instr = IR::Instr::New(newOpcode, regOpnd, fieldSymOpnd, instance2Opnd, m_func);
instr = IR::Instr::New(newOpcode, regOpnd, fieldSymOpnd, m_func);
break;
}

Expand Down
4 changes: 2 additions & 2 deletions lib/Backend/Lower.cpp
Expand Up @@ -6927,8 +6927,8 @@ Lowerer::LowerScopedLdFld(IR::Instr * ldFldInstr, IR::JnHelperMethod helperMetho
LoadScriptContext(ldFldInstr);
}

src = ldFldInstr->UnlinkSrc2();
AssertMsg(src->IsRegOpnd(), "Expected reg opnd as src2");
intptr_t rootObject = m_func->GetJITFunctionBody()->GetRootObject();
src = IR::AddrOpnd::New(rootObject, IR::AddrOpndKindDynamicVar, this->m_func, true);
instrPrev = m_lowererMD.LoadHelperArgument(ldFldInstr, src);

src = ldFldInstr->UnlinkSrc1();
Expand Down
5 changes: 0 additions & 5 deletions lib/Backend/LowerMDShared.cpp
Expand Up @@ -4110,11 +4110,6 @@ LowererMD::GenerateFastScopedLdFld(IR::Instr * instrLdScopedFld)

opndBase = propertySymOpnd->CreatePropertyOwnerOpnd(m_func);

IR::Opnd *srcBase = instrLdScopedFld->GetSrc2();
AssertMsg(srcBase->IsRegOpnd(), "Expected reg opnd as src2");
//opndBase = srcBase;

//IR::IndirOpnd * indirOpnd = src->AsIndirOpnd();
labelHelper = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);

AssertMsg(opndBase->m_sym->m_isSingleDef, "We assume this isn't redefined");
Expand Down

0 comments on commit d58fa97

Please sign in to comment.