Skip to content

Commit 2b3c566

Browse files
pleathpleath
committed
[MERGE #6217 @pleath] Stop passing ActivationObject as 'this' in dynamic-binding cases, eliminate StrictLdThis
Merge pull request #6217 from pleath:actobjthis Detect non-with scopes in cases of dynamically bound functions and pass "undefined" as "this" instead of the owning instance, which is an ActivationObject. Make ScopedLdThis do the same. Got rid of some unnecessary virtual calls in the process. Since screening ActivationObject is the only thing StrictLdThis does, stop emitting it.
2 parents 3c2ce7f + ead295f commit 2b3c566

30 files changed

+21078
-21260
lines changed

‎lib/Backend/IRBuilder.cpp

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1755,7 +1755,7 @@ template <typename SizePolicy>
17551755
void
17561756
IRBuilder::BuildReg2(Js::OpCode newOpcode, uint32 offset)
17571757
{
1758-
Assert(!OpCodeAttr::IsProfiledOp(newOpcode) || newOpcode == Js::OpCode::ProfiledStrictLdThis);
1758+
Assert(!OpCodeAttr::IsProfiledOp(newOpcode));
17591759
Assert(OpCodeAttr::HasMultiSizeLayout(newOpcode));
17601760
auto layout = m_jnReader.GetLayout<Js::OpLayoutT_Reg2<SizePolicy>>();
17611761

@@ -1821,19 +1821,6 @@ IRBuilder::BuildReg2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot R0, Js::Re
18211821
}
18221822
break;
18231823

1824-
case Js::OpCode::ProfiledStrictLdThis:
1825-
newOpcode = Js::OpCode::StrictLdThis;
1826-
if (m_func->HasProfileInfo())
1827-
{
1828-
dstOpnd->SetValueType(m_func->GetReadOnlyProfileInfo()->GetThisInfo().valueType);
1829-
}
1830-
1831-
if (m_func->DoSimpleJitDynamicProfile())
1832-
{
1833-
IR::JitProfilingInstr* newInstr = IR::JitProfilingInstr::New(Js::OpCode::StrictLdThis, dstOpnd, src1Opnd, m_func);
1834-
instr = newInstr;
1835-
}
1836-
break;
18371824
case Js::OpCode::Delete_A:
18381825
dstOpnd->SetValueType(ValueType::Boolean);
18391826
break;

‎lib/Backend/Inline.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5453,7 +5453,6 @@ Inline::MapFormals(Func *inlinee,
54535453

54545454

54555455
case Js::OpCode::LdThis:
5456-
case Js::OpCode::StrictLdThis:
54575456
// Optimization of LdThis may be possible.
54585457
// Verify that this is a use of the "this" passed by the caller (not a nested function).
54595458
if (instr->GetSrc1()->AsRegOpnd()->m_sym == symThis)
@@ -5699,7 +5698,7 @@ Inline::DoCheckThisOpt(IR::Instr * instr)
56995698
// If the instr is an inlined LdThis, try to replace it with a CheckThis
57005699
// that will bail out if a helper call is required to get the real "this" pointer.
57015700

5702-
Assert(instr->m_opcode == Js::OpCode::LdThis || instr->m_opcode == Js::OpCode::StrictLdThis);
5701+
Assert(instr->m_opcode == Js::OpCode::LdThis);
57035702
Assert(instr->IsInlined());
57045703

57055704
// Create the CheckThis. The target is the original offset, i.e., the LdThis still has to be executed.
@@ -5708,7 +5707,7 @@ Inline::DoCheckThisOpt(IR::Instr * instr)
57085707
instr->FreeSrc2();
57095708
}
57105709
IR::Instr *newInstr =
5711-
IR::BailOutInstr::New( instr->m_opcode == Js::OpCode::LdThis ? Js::OpCode::CheckThis : Js::OpCode::StrictCheckThis, IR::BailOutCheckThis, instr, instr->m_func);
5710+
IR::BailOutInstr::New(Js::OpCode::CheckThis, IR::BailOutCheckThis, instr, instr->m_func);
57125711
// Just re-use the original src1 since the LdThis will usually be deleted.
57135712
newInstr->SetSrc1(instr->GetSrc1());
57145713
newInstr->SetByteCodeOffset(instr);

‎lib/Backend/JnHelperMethodList.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ HELPERCALLCHK(OP_CloneInnerScopeSlots, Js::JavascriptOperators::OP_CloneScopeSlo
6060
HELPERCALLCHK(OP_CloneBlockScope, Js::JavascriptOperators::OP_CloneBlockScope, AttrCanNotBeReentrant)
6161
HELPERCALLCHK(LdThis, Js::JavascriptOperators::OP_GetThis, AttrCanThrow)
6262
HELPERCALLCHK(LdThisNoFastPath, Js::JavascriptOperators::OP_GetThisNoFastPath, 0)
63-
HELPERCALLCHK(StrictLdThis, Js::JavascriptOperators::OP_StrictGetThis, AttrCanNotBeReentrant)
6463
HELPERCALLCHK(Op_LdElemUndef, Js::JavascriptOperators::OP_LoadUndefinedToElement, AttrCanNotBeReentrant)
6564
HELPERCALLCHK(Op_LdElemUndefDynamic, Js::JavascriptOperators::OP_LoadUndefinedToElementDynamic, AttrCanNotBeReentrant)
6665
HELPERCALLCHK(Op_LdElemUndefScoped, Js::JavascriptOperators::OP_LoadUndefinedToElementScoped, AttrCanNotBeReentrant)
@@ -424,7 +423,6 @@ HELPERCALLCHK(SimpleProfileCall_DefaultInlineCacheIndex, Js::SimpleJitHelpers::P
424423
HELPERCALLCHK(SimpleProfileCall, Js::SimpleJitHelpers::ProfileCall, AttrCanNotBeReentrant)
425424
HELPERCALLCHK(SimpleProfileReturnTypeCall, Js::SimpleJitHelpers::ProfileReturnTypeCall, AttrCanNotBeReentrant)
426425
//HELPERCALLCHK(SimpleProfiledLdLen, Js::SimpleJitHelpers::ProfiledLdLen_A, AttrCanThrow) //Can throw because it mirrors OP_GetProperty
427-
HELPERCALLCHK(SimpleProfiledStrictLdThis, Js::SimpleJitHelpers::ProfiledStrictLdThis, AttrCanNotBeReentrant)
428426
HELPERCALLCHK(SimpleProfiledLdThis, Js::SimpleJitHelpers::ProfiledLdThis, AttrCanNotBeReentrant)
429427
HELPERCALLCHK(SimpleProfiledSwitch, Js::SimpleJitHelpers::ProfiledSwitch, AttrCanNotBeReentrant)
430428
HELPERCALLCHK(SimpleProfiledDivide, Js::SimpleJitHelpers::ProfiledDivide, AttrCanThrow)

‎lib/Backend/Lower.cpp

Lines changed: 0 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,42 +1246,12 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
12461246
Assert(m_func->IsOOPJIT());
12471247
instrPrev = LowerLdNativeCodeData(instr);
12481248
break;
1249-
case Js::OpCode::StrictLdThis:
1250-
if (noFieldFastPath)
1251-
{
1252-
IR::JnHelperMethod meth;
1253-
if (instr->IsJitProfilingInstr())
1254-
{
1255-
Assert(instr->AsJitProfilingInstr()->profileId == Js::Constants::NoProfileId);
1256-
m_lowererMD.LoadHelperArgument(instr, CreateFunctionBodyOpnd(instr->m_func));
1257-
meth = IR::HelperSimpleProfiledStrictLdThis;
1258-
this->LowerUnaryHelper(instr, meth);
1259-
}
1260-
else
1261-
{
1262-
meth = IR::HelperStrictLdThis;
1263-
this->LowerUnaryHelperMem(instr, meth);
1264-
}
1265-
}
1266-
else
1267-
{
1268-
this->GenerateLdThisStrict(instr);
1269-
instr->Remove();
1270-
}
1271-
break;
1272-
12731249
case Js::OpCode::CheckThis:
12741250
GenerateLdThisCheck(instr);
12751251
instr->FreeSrc1();
12761252
this->GenerateBailOut(instr);
12771253
break;
12781254

1279-
case Js::OpCode::StrictCheckThis:
1280-
this->GenerateLdThisStrict(instr);
1281-
instr->FreeSrc1();
1282-
this->GenerateBailOut(instr);
1283-
break;
1284-
12851255
case Js::OpCode::NewScArray:
12861256
instrPrev = this->LowerNewScArray(instr);
12871257
break;
@@ -23245,69 +23215,6 @@ Lowerer::GenerateLdThisCheck(IR::Instr * instr)
2324523215
return true;
2324623216
}
2324723217

23248-
//
23249-
// TEST src, Js::AtomTag
23250-
// JNE $done
23251-
// MOV typeReg, objectSrc + offsetof(RecyclableObject::type)
23252-
// CMP [typeReg + offsetof(Type::typeid)], TypeIds_ActivationObject
23253-
// JEQ $helper
23254-
// $done:
23255-
// MOV dst, src
23256-
// JMP $fallthru
23257-
// helper:
23258-
// MOV dst, undefined
23259-
// $fallthru:
23260-
bool
23261-
Lowerer::GenerateLdThisStrict(IR::Instr* instr)
23262-
{
23263-
IR::RegOpnd * src1 = instr->GetSrc1()->AsRegOpnd();
23264-
IR::RegOpnd * typeReg = IR::RegOpnd::New(TyMachReg, this->m_func);
23265-
IR::LabelInstr * done = IR::LabelInstr::New(Js::OpCode::Label, m_func);
23266-
IR::LabelInstr * fallthru = IR::LabelInstr::New(Js::OpCode::Label, m_func);
23267-
IR::LabelInstr * helper = IR::LabelInstr::New(Js::OpCode::Label, m_func, /*helper*/true);
23268-
23269-
bool assign = instr->GetDst() && !instr->GetDst()->IsEqual(src1);
23270-
if (!src1->IsNotTaggedValue())
23271-
{
23272-
// TEST src1, Js::AtomTag
23273-
// JNE $done
23274-
this->m_lowererMD.GenerateObjectTest(src1, instr, assign ? done : fallthru);
23275-
}
23276-
23277-
IR::IndirOpnd * indirOpnd = IR::IndirOpnd::New(src1, Js::RecyclableObject::GetOffsetOfType(), TyMachReg, this->m_func);
23278-
Lowerer::InsertMove(typeReg, indirOpnd, instr);
23279-
23280-
IR::IndirOpnd * typeID = IR::IndirOpnd::New(typeReg, Js::Type::GetOffsetOfTypeId(), TyInt32, this->m_func);
23281-
IR::Opnd * activationObject = IR::IntConstOpnd::New(Js::TypeIds_ActivationObject, TyMachReg, this->m_func);
23282-
Lowerer::InsertCompare(typeID, activationObject, instr);
23283-
23284-
// JEQ $helper
23285-
Lowerer::InsertBranch(Js::OpCode::BrEq_A, helper, instr);
23286-
23287-
if (assign)
23288-
{
23289-
// $done:
23290-
instr->InsertBefore(done);
23291-
23292-
// MOV dst, src
23293-
Lowerer::InsertMove(instr->GetDst(), src1, instr);
23294-
}
23295-
23296-
// JMP $fallthru
23297-
Lowerer::InsertBranch(Js::OpCode::Br, fallthru, instr);
23298-
23299-
instr->InsertBefore(helper);
23300-
if (instr->GetDst())
23301-
{
23302-
// MOV dst, undefined
23303-
Lowerer::InsertMove(instr->GetDst(), LoadLibraryValueOpnd(instr, LibraryValue::ValueUndefined), instr);
23304-
}
23305-
// $fallthru:
23306-
instr->InsertAfter(fallthru);
23307-
23308-
return true;
23309-
}
23310-
2331123218
// given object instanceof function, functionReg is a register with function,
2331223219
// objectReg is a register with instance and inlineCache is an InstIsInlineCache.
2331323220
// We want to generate:

‎lib/Backend/Lower.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,6 @@ class Lowerer
173173
void GenerateIsDynamicObject(IR::RegOpnd *regOpnd, IR::Instr *insertInstr, IR::LabelInstr *labelHelper, bool fContinueLabel = false);
174174
void GenerateIsRecyclableObject(IR::RegOpnd *regOpnd, IR::Instr *insertInstr, IR::LabelInstr *labelHelper, bool checkObjectAndDynamicObject = true);
175175
bool GenerateLdThisCheck(IR::Instr * instr);
176-
bool GenerateLdThisStrict(IR::Instr * instr);
177176
bool GenerateFastIsInst(IR::Instr * instr);
178177
void GenerateFastArrayIsIn(IR::Instr * instr);
179178
void GenerateFastObjectIsIn(IR::Instr * instr);

‎lib/Backend/SimpleJitProfilingHelpers.cpp

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -94,26 +94,6 @@ using namespace Js;
9494
JIT_HELPER_END(SimpleProfileReturnTypeCall);
9595
}
9696

97-
Var SimpleJitHelpers::ProfiledStrictLdThis(Var thisVar, FunctionBody* functionBody)
98-
{
99-
JIT_HELPER_NOT_REENTRANT_NOLOCK_HEADER(SimpleProfiledStrictLdThis);
100-
//Adapted from InterpreterStackFrame::OP_ProfiledStrictLdThis
101-
DynamicProfileInfo * dynamicProfileInfo = functionBody->GetDynamicProfileInfo();
102-
TypeId typeId = JavascriptOperators::GetTypeId(thisVar);
103-
104-
if (typeId == TypeIds_ActivationObject)
105-
{
106-
thisVar = functionBody->GetScriptContext()->GetLibrary()->GetUndefined();
107-
dynamicProfileInfo->RecordThisInfo(thisVar, ThisType_Mapped);
108-
return thisVar;
109-
}
110-
111-
dynamicProfileInfo->RecordThisInfo(thisVar, ThisType_Simple);
112-
return thisVar;
113-
JIT_HELPER_END(SimpleProfiledStrictLdThis);
114-
}
115-
116-
11797
Var SimpleJitHelpers::ProfiledLdThis(Var thisVar, int moduleID, FunctionBody* functionBody)
11898
{
11999
JIT_HELPER_NOT_REENTRANT_NOLOCK_HEADER(SimpleProfiledLdThis);

‎lib/Backend/SimpleJitProfilingHelpers.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ namespace Js
1616
void ProfileCall(void* framePtr, ProfileId profileId, InlineCacheIndex inlineCacheIndex, Var retval, Var callee, CallInfo info);
1717
void ProfileReturnTypeCall(void* framePtr, ProfileId profileId, Var retval, JavascriptFunction*callee, CallInfo info);
1818

19-
Var ProfiledStrictLdThis(Var thisVar, FunctionBody* functionBody);
2019
Var ProfiledLdThis(Var thisVar, int moduleID, FunctionBody* functionBody);
2120
Var ProfiledSwitch(FunctionBody* functionBody,ProfileId profileId, Var exp);
2221
Var ProfiledDivide(FunctionBody* functionBody, ProfileId profileId, Var aLeft, Var aRight);

‎lib/Runtime/ByteCode/ByteCodeCacheReleaseFileVersion.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//-------------------------------------------------------------------------------------------------------
55
// NOTE: If there is a merge conflict the correct fix is to make a new GUID.
66

7-
// {CA631EEC-2284-4F10-B25B-159819C4ECDB}
8-
const GUID byteCodeCacheReleaseFileVersion =
9-
{ 0xca631eec, 0x2284, 0x4f10, { 0xb2, 0x5b, 0x15, 0x98, 0x19, 0xc4, 0xec, 0xdb } };
7+
// {1CD5808A-718A-4F04-9D00-8E5097969BD1}
8+
const GUID byteCodeCacheReleaseFileVersion =
9+
{ 0x1CD5808A, 0x718A, 0x4F04, { 0x9D, 0x00, 0x8E, 0x50, 0x97, 0x96, 0x9B, 0xD1 } };
10+

‎lib/Runtime/ByteCode/ByteCodeEmitter.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2195,7 +2195,11 @@ void ByteCodeGenerator::LoadThisObject(FuncInfo *funcInfo, bool thisLoadedFromPa
21952195
// thisLoadedFromParams would be true for the event Handler case,
21962196
// "this" would have been loaded from parameters to put in the environment
21972197
//
2198-
if (!thisLoadedFromParams)
2198+
if (funcInfo->GetIsStrictMode())
2199+
{
2200+
m_writer.ArgIn0(thisSym->GetLocation());
2201+
}
2202+
else if (!thisLoadedFromParams)
21992203
{
22002204
Js::RegSlot tmpReg = funcInfo->AcquireTmpRegister();
22012205
m_writer.ArgIn0(tmpReg);
@@ -2368,7 +2372,7 @@ void ByteCodeGenerator::EmitSuperCall(FuncInfo* funcInfo, ParseNodeSuperCall * p
23682372
}
23692373

23702374
Symbol* thisSym = pnodeSuperCall->pnodeThis->sym;
2371-
this->Writer()->Reg2(Js::OpCode::StrictLdThis, pnodeSuperCall->pnodeThis->location, valueForThis);
2375+
this->Writer()->Reg2(Js::OpCode::Ld_A, pnodeSuperCall->pnodeThis->location, valueForThis);
23722376

23732377
EmitPropStoreForSpecialSymbol(pnodeSuperCall->pnodeThis->location, thisSym, pnodeSuperCall->pnodeThis->pid, funcInfo, false);
23742378

@@ -2396,7 +2400,10 @@ void ByteCodeGenerator::EmitThis(FuncInfo *funcInfo, Js::RegSlot lhsLocation, Js
23962400
{
23972401
if (funcInfo->byteCodeFunction->GetIsStrictMode() && !funcInfo->IsGlobalFunction() && !funcInfo->IsLambda())
23982402
{
2399-
m_writer.Reg2(Js::OpCode::StrictLdThis, lhsLocation, fromRegister);
2403+
if (lhsLocation != fromRegister)
2404+
{
2405+
m_writer.Reg2(Js::OpCode::Ld_A, lhsLocation, fromRegister);
2406+
}
24002407
}
24012408
else
24022409
{
@@ -4321,7 +4328,7 @@ void ByteCodeGenerator::EmitLoadInstance(Symbol *sym, IdentPtr pid, Js::RegSlot
43214328
this->m_writer.Reg1(Js::OpCode::LdLocalObj, instLocation);
43224329
if (thisLocation != Js::Constants::NoRegister && thisLocation != instLocation)
43234330
{
4324-
this->m_writer.Reg1(Js::OpCode::LdLocalObj, thisLocation);
4331+
this->m_writer.Reg2(Js::OpCode::Ld_A, thisLocation, funcInfo->undefinedConstantRegister);
43254332
}
43264333
}
43274334
else
@@ -4334,7 +4341,7 @@ void ByteCodeGenerator::EmitLoadInstance(Symbol *sym, IdentPtr pid, Js::RegSlot
43344341
this->m_writer.SlotI1(Js::OpCode::LdEnvObj, instLocation, frameDisplayIndex);
43354342
if (thisLocation != Js::Constants::NoRegister && thisLocation != instLocation)
43364343
{
4337-
this->m_writer.SlotI1(Js::OpCode::LdEnvObj, thisLocation, frameDisplayIndex);
4344+
this->m_writer.Reg2(Js::OpCode::Ld_A, thisLocation, funcInfo->undefinedConstantRegister);
43384345
}
43394346
}
43404347
}

‎lib/Runtime/ByteCode/ByteCodeWriter.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -382,20 +382,9 @@ namespace Js
382382
CheckOp(op, OpLayoutType::Reg2);
383383
Assert(OpCodeAttr::HasMultiSizeLayout(op));
384384

385-
if (DoDynamicProfileOpcode(CheckThisPhase) ||
386-
DoDynamicProfileOpcode(TypedArrayTypeSpecPhase) ||
387-
DoDynamicProfileOpcode(ArrayCheckHoistPhase))
388-
{
389-
if (op == OpCode::StrictLdThis)
390-
{
391-
op = OpCode::ProfiledStrictLdThis;
392-
}
393-
}
394-
395385
R0 = ConsumeReg(R0);
396386
R1 = ConsumeReg(R1);
397387

398-
399388
bool isProfiled = false;
400389
bool isProfiled2 = false;
401390
Js::ProfileId profileId = Js::Constants::NoProfileId;

0 commit comments

Comments
 (0)