Skip to content

Commit 5e72ccf

Browse files
rhuanjlrhuanjl
committed
Avoid using TryFinallyWithYield OpCode when there is no Yield.
Implement partial tracking of Yield within ByteCodeGenerator to enable this. The OpCode TryFinallyWithYield is not supported by the Jit. It was being used unnecessarily in various code patterns that should be safe to Jit, such patterns were hitting an Abort in the Jit because of the unimplemented OpCode. As the OpCode is unnecessary when there is no Yield (the cases we currently try to jit) fix logic so we do not use it in those cases.
1 parent c6a9549 commit 5e72ccf

File tree

5 files changed

+170
-18
lines changed

5 files changed

+170
-18
lines changed

‎lib/Backend/Lower.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2605,6 +2605,9 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
26052605
instrPrev = this->LowerTry(instr, true /*try-catch*/);
26062606
break;
26072607

2608+
case Js::OpCode::TryFinallyWithYield:
2609+
// TODO Implement TryFinallyWithYield before enabling Jit on full async functions
2610+
AssertOrFailFastMsg(false, "TryFinallyWithYield not implemented in Jit, should not be trying to Jit this function.");
26082611
case Js::OpCode::TryFinally:
26092612
instrPrev = this->LowerTry(instr, false /*try-finally*/);
26102613
break;

‎lib/Runtime/ByteCode/ByteCodeEmitter.cpp‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6777,9 +6777,9 @@ void EmitIteratorTopLevelFinally(
67776777
Js::RegSlot yieldOffsetLocation,
67786778
ByteCodeGenerator* byteCodeGenerator,
67796779
FuncInfo* funcInfo,
6780+
bool hasYield,
67806781
bool isAsync)
67816782
{
6782-
bool isCoroutine = funcInfo->byteCodeFunction->IsCoroutine();
67836783

67846784
Js::ByteCodeLabel afterFinallyBlockLabel = byteCodeGenerator->Writer()->DefineLabel();
67856785
byteCodeGenerator->Writer()->Empty(Js::OpCode::Leave);
@@ -6805,8 +6805,9 @@ void EmitIteratorTopLevelFinally(
68056805
byteCodeGenerator->Writer()->MarkLabel(skipCallCloseLabel);
68066806

68076807
byteCodeGenerator->PopJumpCleanup();
6808-
if (isCoroutine)
6808+
if (hasYield)
68096809
{
6810+
Assert(funcInfo->byteCodeFunction->IsCoroutine());
68106811
funcInfo->ReleaseTmpRegister(yieldOffsetLocation);
68116812
funcInfo->ReleaseTmpRegister(yieldExceptionLocation);
68126813
}
@@ -6826,6 +6827,7 @@ void EmitIteratorCatchAndFinally(
68266827
Js::RegSlot yieldOffsetLocation,
68276828
ByteCodeGenerator* byteCodeGenerator,
68286829
FuncInfo* funcInfo,
6830+
bool hasYield,
68296831
bool isAsync = false)
68306832
{
68316833
byteCodeGenerator->PopJumpCleanup();
@@ -6849,6 +6851,7 @@ void EmitIteratorCatchAndFinally(
68496851
yieldOffsetLocation,
68506852
byteCodeGenerator,
68516853
funcInfo,
6854+
hasYield,
68526855
isAsync);
68536856

68546857
funcInfo->ReleaseTmpRegister(shouldCallReturnFunctionLocationFinally);
@@ -6901,10 +6904,11 @@ void EmitDestructuredArray(
69016904

69026905
Js::RegSlot regException = Js::Constants::NoRegister;
69036906
Js::RegSlot regOffset = Js::Constants::NoRegister;
6904-
bool isCoroutine = funcInfo->byteCodeFunction->IsCoroutine();
6907+
bool hasYield = byteCodeGenerator->GetHasYield(lhs);
69056908

6906-
if (isCoroutine)
6909+
if (hasYield)
69076910
{
6911+
Assert(funcInfo->byteCodeFunction->IsCoroutine());
69086912
regException = funcInfo->AcquireTmpRegister();
69096913
regOffset = funcInfo->AcquireTmpRegister();
69106914
}
@@ -6914,7 +6918,7 @@ void EmitDestructuredArray(
69146918
Js::ByteCodeLabel catchLabel = byteCodeGenerator->Writer()->DefineLabel();
69156919
byteCodeGenerator->Writer()->RecordCrossFrameEntryExitRecord(true);
69166920

6917-
if (isCoroutine)
6921+
if (hasYield)
69186922
{
69196923
byteCodeGenerator->Writer()->BrReg2(Js::OpCode::TryFinallyWithYield, finallyLabel, regException, regOffset);
69206924
byteCodeGenerator->PushJumpCleanupForTry(
@@ -6950,7 +6954,8 @@ void EmitDestructuredArray(
69506954
regException,
69516955
regOffset,
69526956
byteCodeGenerator,
6953-
funcInfo);
6957+
funcInfo,
6958+
hasYield);
69546959

69556960
funcInfo->ReleaseTmpRegister(nextMethodReg);
69566961
funcInfo->ReleaseTmpRegister(iteratorLocation);
@@ -9836,6 +9841,7 @@ void EmitForInOrForOf(ParseNodeForInOrForOf *loopNode, ByteCodeGenerator *byteCo
98369841
{
98379842
bool isForIn = (loopNode->nop == knopForIn);
98389843
bool isForAwaitOf = (loopNode->nop == knopForAwaitOf);
9844+
bool hasYield = isForAwaitOf || byteCodeGenerator->GetHasYield(loopNode);
98399845
Assert(isForAwaitOf || isForIn || loopNode->nop == knopForOf);
98409846

98419847
BeginEmitBlock(loopNode->pnodeBlock, byteCodeGenerator, funcInfo);
@@ -9899,10 +9905,9 @@ void EmitForInOrForOf(ParseNodeForInOrForOf *loopNode, ByteCodeGenerator *byteCo
98999905
Js::RegSlot shouldCallReturnFunctionLocation = loopNode->shouldCallReturnFunctionLocation;
99009906
Js::RegSlot shouldCallReturnFunctionLocationFinally = loopNode->shouldCallReturnFunctionLocationFinally;
99019907

9902-
bool isCoroutine = funcInfo->byteCodeFunction->IsCoroutine();
9903-
9904-
if (isCoroutine)
9908+
if (hasYield)
99059909
{
9910+
Assert(funcInfo->byteCodeFunction->IsCoroutine());
99069911
regException = funcInfo->AcquireTmpRegister();
99079912
regOffset = funcInfo->AcquireTmpRegister();
99089913
}
@@ -9946,7 +9951,7 @@ void EmitForInOrForOf(ParseNodeForInOrForOf *loopNode, ByteCodeGenerator *byteCo
99469951
byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdFalse, shouldCallReturnFunctionLocation);
99479952
byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdFalse, shouldCallReturnFunctionLocationFinally);
99489953

9949-
if (isCoroutine)
9954+
if (hasYield)
99509955
{
99519956
byteCodeGenerator->Writer()->BrReg2(Js::OpCode::TryFinallyWithYield, finallyLabel, regException, regOffset);
99529957
byteCodeGenerator->PushJumpCleanupForTry(
@@ -10027,6 +10032,7 @@ void EmitForInOrForOf(ParseNodeForInOrForOf *loopNode, ByteCodeGenerator *byteCo
1002710032
regOffset,
1002810033
byteCodeGenerator,
1002910034
funcInfo,
10035+
hasYield,
1003010036
isForAwaitOf);
1003110037
}
1003210038

@@ -12545,11 +12551,11 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func
1254512551

1254612552
finallyLabel = byteCodeGenerator->Writer()->DefineLabel();
1254712553
byteCodeGenerator->Writer()->RecordCrossFrameEntryExitRecord(true);
12554+
bool hasYield = byteCodeGenerator->GetHasYield(pnodeTryFinally);
1254812555

12549-
// [CONSIDER][aneeshd] Ideally the TryFinallyWithYield opcode needs to be used only if there is a yield expression.
12550-
// For now, if the function is generator we are using the TryFinallyWithYield.
12551-
if (funcInfo->byteCodeFunction->IsCoroutine())
12556+
if (hasYield)
1255212557
{
12558+
Assert(funcInfo->byteCodeFunction->IsCoroutine());
1255312559
regException = funcInfo->AcquireTmpRegister();
1255412560
regOffset = funcInfo->AcquireTmpRegister();
1255512561
byteCodeGenerator->Writer()->BrReg2(Js::OpCode::TryFinallyWithYield, finallyLabel, regException, regOffset);
@@ -12593,7 +12599,7 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func
1259312599
Emit(pnodeFinally->pnodeBody, byteCodeGenerator, funcInfo, fReturnValue);
1259412600
funcInfo->ReleaseLoc(pnodeFinally->pnodeBody);
1259512601

12596-
if (funcInfo->byteCodeFunction->IsCoroutine())
12602+
if (hasYield)
1259712603
{
1259812604
funcInfo->ReleaseTmpRegister(regOffset);
1259912605
funcInfo->ReleaseTmpRegister(regException);

‎lib/Runtime/ByteCode/ByteCodeGenerator.cpp‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,12 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref
219219
prefix(pnode, byteCodeGenerator);
220220
switch (pnode->nop)
221221
{
222+
case knopYield:
223+
case knopYieldLeaf:
224+
case knopYieldStar:
225+
case knopAwait:
226+
byteCodeGenerator->SetHasYield();
227+
// fall through to default
222228
default:
223229
{
224230
uint flags = ParseNode::Grfnop(pnode->nop);
@@ -245,6 +251,7 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref
245251
break;
246252

247253
case knopArrayPattern:
254+
byteCodeGenerator->PushTrackForYield(pnode);
248255
if (!byteCodeGenerator->InDestructuredPattern())
249256
{
250257
byteCodeGenerator->SetInDestructuredPattern(true);
@@ -255,6 +262,7 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref
255262
{
256263
Visit(pnode->AsParseNodeUni()->pnode1, byteCodeGenerator, prefix, postfix);
257264
}
265+
byteCodeGenerator->PopTrackForYield(pnode);
258266
break;
259267

260268
case knopCall:
@@ -379,11 +387,13 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref
379387
case knopForOf:
380388
case knopForAwaitOf:
381389
BeginVisitBlock(pnode->AsParseNodeForInOrForOf()->pnodeBlock, byteCodeGenerator);
390+
byteCodeGenerator->PushTrackForYield(pnode);
382391
Visit(pnode->AsParseNodeForInOrForOf()->pnodeLval, byteCodeGenerator, prefix, postfix);
383392
Visit(pnode->AsParseNodeForInOrForOf()->pnodeObj, byteCodeGenerator, prefix, postfix);
384393
byteCodeGenerator->EnterLoop();
385394
Visit(pnode->AsParseNodeForInOrForOf()->pnodeBody, byteCodeGenerator, prefix, postfix, pnode);
386395
byteCodeGenerator->ExitLoop();
396+
byteCodeGenerator->PopTrackForYield(pnode);
387397
EndVisitBlock(pnode->AsParseNodeForInOrForOf()->pnodeBlock, byteCodeGenerator);
388398
break;
389399
// PTNODE(knopReturn , "return" ,None ,Uni ,fnopNone)
@@ -441,8 +451,10 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref
441451
break;
442452
// PTNODE(knopTryCatchFinally,"try-catch-finally",None,TryCatchFinally,fnopCleanup)
443453
case knopTryFinally:
454+
byteCodeGenerator->PushTrackForYield(pnode);
444455
Visit(pnode->AsParseNodeTryFinally()->pnodeTry, byteCodeGenerator, prefix, postfix, pnode);
445456
Visit(pnode->AsParseNodeTryFinally()->pnodeFinally, byteCodeGenerator, prefix, postfix, pnode);
457+
byteCodeGenerator->PopTrackForYield(pnode);
446458
break;
447459
// PTNODE(knopTryCatch , "try-catch" ,None ,TryCatch ,fnopCleanup)
448460
case knopTryCatch:
@@ -728,6 +740,8 @@ ByteCodeGenerator::ByteCodeGenerator(Js::ScriptContext* scriptContext, Js::Scope
728740
flags(0),
729741
funcInfoStack(nullptr),
730742
jumpCleanupList(nullptr),
743+
nodesToTrackForYield(nullptr),
744+
nodesWithYield(nullptr),
731745
pRootFunc(nullptr),
732746
pCurrentFunction(nullptr),
733747
globalScope(nullptr),
@@ -769,6 +783,52 @@ void ByteCodeGenerator::AddFuncInfoToFinalizationSet(FuncInfo * funcInfo)
769783
this->funcInfosToFinalize->Prepend(funcInfo);
770784
}
771785

786+
void ByteCodeGenerator::PushTrackForYield(ParseNode* node)
787+
{
788+
if (this->nodesToTrackForYield == nullptr)
789+
{
790+
this->nodesToTrackForYield = Anew(alloc, SList<ParseNode*>, alloc);
791+
}
792+
this->nodesToTrackForYield->Push(node);
793+
}
794+
795+
void ByteCodeGenerator::PopTrackForYield(ParseNode* node)
796+
{
797+
Assert (this->nodesToTrackForYield != nullptr);
798+
Assert (this->nodesToTrackForYield->Top() == node);
799+
this->nodesToTrackForYield->Pop();
800+
if (this->nodesToTrackForYield->Empty())
801+
{
802+
this->nodesToTrackForYield = nullptr;
803+
}
804+
else if (this->GetHasYield(node))
805+
{
806+
this->SetHasYield();
807+
}
808+
}
809+
810+
void ByteCodeGenerator::SetHasYield()
811+
{
812+
if (this->nodesToTrackForYield != nullptr)
813+
{
814+
Assert(!this->nodesToTrackForYield->Empty());
815+
if (this->nodesWithYield == nullptr)
816+
{
817+
this->nodesWithYield = Anew(alloc, SList<ParseNode*>, alloc);
818+
}
819+
this->nodesWithYield->Push(this->nodesToTrackForYield->Top());
820+
}
821+
}
822+
823+
bool ByteCodeGenerator::GetHasYield(ParseNode* pnode)
824+
{
825+
if (this->nodesWithYield != nullptr && this->nodesWithYield->Has(pnode))
826+
{
827+
return true;
828+
}
829+
return false;
830+
}
831+
772832
/* static */
773833
bool ByteCodeGenerator::IsFalse(ParseNode* node)
774834
{
@@ -2203,6 +2263,8 @@ void ByteCodeGenerator::Begin(
22032263
this->envDepth = 0;
22042264
this->trackEnvDepth = false;
22052265
this->funcInfosToFinalize = nullptr;
2266+
this->nodesToTrackForYield = nullptr;
2267+
this->nodesWithYield = nullptr;
22062268

22072269
this->funcInfoStack = Anew(alloc, SList<FuncInfo*>, alloc);
22082270
this->jumpCleanupList = Anew(alloc, JumpCleanupList, alloc);

‎lib/Runtime/ByteCode/ByteCodeGenerator.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ class ByteCodeGenerator
6262
Js::ParseableFunctionInfo * pRootFunc;
6363

6464
SList<FuncInfo*> * funcInfosToFinalize;
65+
SList<ParseNode*> * nodesToTrackForYield;
66+
SList<ParseNode*> * nodesWithYield;
6567

6668
using JumpCleanupList = DList<JumpCleanupInfo, ArenaAllocator>;
6769
JumpCleanupList* jumpCleanupList;
@@ -485,6 +487,11 @@ class ByteCodeGenerator
485487
bool HasJumpCleanup() { return !this->jumpCleanupList->Empty(); }
486488
void EmitJumpCleanup(ParseNode* target, FuncInfo* funcInfo);
487489

490+
void ByteCodeGenerator::SetHasYield();
491+
bool ByteCodeGenerator::GetHasYield(ParseNode* node);
492+
void ByteCodeGenerator::PopTrackForYield(ParseNode* node);
493+
void ByteCodeGenerator::PushTrackForYield(ParseNode* node);
494+
488495
private:
489496
bool NeedCheckBlockVar(Symbol* sym, Scope* scope, FuncInfo* funcInfo) const;
490497

‎test/es6GeneratorJit/async-jit-bugs.js‎

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
55
//-------------------------------------------------------------------------------------------------------
66

7-
function main() {
7+
function testOne() {
88
const v2 = [13.37,13.37,13.37,13.37,13.37];
99
async function v4(v5,v6,v7,v8) {
1010
const v10 = 0;
@@ -26,8 +26,82 @@ function main() {
2626
const v38 = --v33;
2727
}
2828
const v39 = 128;
29-
print("pass")
3029
}
31-
v4("vEBD7ei78q");
30+
return v4("vEBD7ei78q");
3231
}
33-
main();
32+
33+
// BugIssue #7034
34+
function testTwo() {
35+
let finallyCount = 0;
36+
let throwCount = 0;
37+
async function asyncFinally() {
38+
for (let i = 0; i < 1000; ++i){
39+
try {
40+
if (i > 170) {
41+
++throwCount;
42+
throw 1;
43+
}
44+
}
45+
finally {
46+
++finallyCount;
47+
}
48+
}
49+
}
50+
return asyncFinally ().catch((e) => {
51+
if (throwCount != 1) {
52+
throw new Error ("Wrong number of throws within async function expected 1 but received " + throwCount);
53+
}
54+
if (e != 1) {
55+
throw new Error ("Wrong value thrown from async function expected 1 but received " + e);
56+
}
57+
if (finallyCount != 172) {
58+
throw new Error ("Wrong number of finally calls from async function expected 172 but received " + finallyCount);
59+
}
60+
});
61+
}
62+
63+
function testThree() {
64+
let finallyCount = 0;
65+
let throwCount = 0;
66+
async function asyncFinallyAwait() {
67+
for (let i = 0; i < 1000; ++i){
68+
try {
69+
if (i > 170) {
70+
++throwCount;
71+
throw 1;
72+
}
73+
}
74+
finally {
75+
await 5;
76+
++finallyCount;
77+
}
78+
}
79+
}
80+
return asyncFinallyAwait().catch((e) => {
81+
if (throwCount != 1) {
82+
throw new Error ("Wrong number of throws within async function expected 1 but received " + throwCount);
83+
}
84+
if (e != 1) {
85+
throw new Error ("Wrong value thrown from async function expected 1 but received " + e);
86+
}
87+
if (finallyCount != 172) {
88+
throw new Error ("Wrong number of finally calls from async function expected 172 but received " + finallyCount);
89+
}
90+
});
91+
}
92+
93+
// BugIssue #7016
94+
function testFour()
95+
{
96+
async function test() {
97+
var i8 = new Int8Array(256);
98+
var IntArr0 = [];
99+
for (var _strvar1 of i8) {
100+
for (var _strvar1 of IntArr0) {}
101+
}
102+
}
103+
return test();
104+
}
105+
106+
107+
Promise.all([testOne(), testTwo(), testThree(), testFour()]).then(()=>{print("pass")}, (e)=>{print (e)});

0 commit comments

Comments
 (0)