Skip to content

Commit 7f3a8f6

Browse files
rajatdrajatd
committed
[1.2>master] [MERGE #958] Fixing bug with shared bailouts
Merge pull request #958 from rajatd:bailoutArmBug Shared bailouts for InlineMathFloor/Ceil/Round were bypassing the code on the bailout path that sets up data specific to a bailout (bailout kind, polymorphic inline cache index etc.) on the bailout record and were jumping directly to the bailout label. Fixed that.
2 parents 5a79092 + 22d2fb0 commit 7f3a8f6

File tree

4 files changed

+32
-52
lines changed

4 files changed

+32
-52
lines changed

‎lib/Backend/Lower.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12162,11 +12162,10 @@ Lowerer::GenerateObjectTestAndTypeLoad(IR::Instr *instrLdSt, IR::RegOpnd *opndBa
1216212162
}
1216312163

1216412164
IR::LabelInstr *
12165-
Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::LabelInstr *bailOutLabel)
12165+
Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::LabelInstr *bailOutLabel, IR::LabelInstr * collectRuntimeStatsLabel)
1216612166
{
1216712167
BailOutInfo * bailOutInfo = instr->GetBailOutInfo();
1216812168
IR::Instr * bailOutInstr = bailOutInfo->bailOutInstr;
12169-
IR::LabelInstr *collectRuntimeStatsLabel = nullptr;
1217012169
if (instr->IsCloned())
1217112170
{
1217212171
Assert(bailOutInstr != instr);
@@ -12179,14 +12178,18 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L
1217912178
return bailOutLabel;
1218012179
}
1218112180

12181+
// Add helper label to trigger layout.
12182+
if (!collectRuntimeStatsLabel)
12183+
{
12184+
collectRuntimeStatsLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
12185+
}
12186+
Assert(!collectRuntimeStatsLabel->IsLinked());
12187+
instr->InsertBefore(collectRuntimeStatsLabel);
12188+
1218212189
if (bailOutInstr != instr)
1218312190
{
1218412191
// this bailOutInfo is shared, just jump to the bailout target
1218512192

12186-
// Add helper label to trigger layout.
12187-
collectRuntimeStatsLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
12188-
instr->InsertBefore(collectRuntimeStatsLabel);
12189-
1219012193
IR::MemRefOpnd *pIndexOpndForBailOutKind =
1219112194
IR::MemRefOpnd::New((BYTE*)bailOutInfo->bailOutRecord + BailOutRecord::GetOffsetOfBailOutKind(), TyUint32, this->m_func, IR::AddrOpndKindDynamicBailOutKindRef);
1219212195
m_lowererMD.CreateAssign(
@@ -12234,10 +12237,6 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L
1223412237
// The bailout hasn't be generated yet.
1223512238
Assert(!bailOutInstr->IsLabelInstr());
1223612239

12237-
// Add helper label to trigger layout.
12238-
collectRuntimeStatsLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
12239-
instr->InsertBefore(collectRuntimeStatsLabel);
12240-
1224112240
// capture the condition for this bailout
1224212241
if (bailOutLabel == nullptr)
1224312242
{

‎lib/Backend/Lower.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ class Lowerer
408408
void PreserveSourcesForBailOnResultCondition(IR::Instr *const instr, IR::LabelInstr *const skipBailOutLabel) const;
409409
void LowerInstrWithBailOnResultCondition(IR::Instr *const instr, const IR::BailOutKind bailOutKind, IR::LabelInstr *const bailOutLabel, IR::LabelInstr *const skipBailOutLabel) const;
410410
void GenerateObjectTestAndTypeLoad(IR::Instr *instrLdSt, IR::RegOpnd *opndBase, IR::RegOpnd *opndType, IR::LabelInstr *labelHelper);
411-
IR::LabelInstr *GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr = nullptr, IR::LabelInstr * labelBailOut = nullptr);
411+
IR::LabelInstr *GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr = nullptr, IR::LabelInstr * labelBailOut = nullptr, IR::LabelInstr * collectRuntimeStatsLabel = nullptr);
412412
void GenerateJumpToEpilogForBailOut(BailOutInfo * bailOutInfo, IR::Instr *instrAfter);
413413

414414
void LowerDivI4(IR::Instr * const instr);

‎lib/Backend/LowerMDShared.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8874,14 +8874,7 @@ void LowererMD::GenerateFastInlineBuiltInCall(IR::Instr* instr, IR::JnHelperMeth
88748874
if (instr->GetDst()->IsInt32())
88758875
{
88768876
sharedBailout = (instr->GetBailOutInfo()->bailOutInstr != instr) ? true : false;
8877-
if (sharedBailout)
8878-
{
8879-
bailoutLabel = instr->GetBailOutInfo()->bailOutInstr->AsLabelInstr();
8880-
}
8881-
else
8882-
{
8883-
bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, /*helperLabel*/true);
8884-
}
8877+
bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, /*helperLabel*/true);
88858878
}
88868879

88878880
IR::Opnd * zero;
@@ -9096,7 +9089,10 @@ void LowererMD::GenerateFastInlineBuiltInCall(IR::Instr* instr, IR::JnHelperMeth
90969089
{
90979090
instr->InsertBefore(bailoutLabel);
90989091
}
9099-
this->m_lowerer->GenerateBailOut(instr);
9092+
9093+
// In case of a shared bailout, we should jump to the code that sets some data on the bailout record which is specific
9094+
// to this bailout. Pass the bailoutLabel to GenerateFunction so that it may use the label as the collectRuntimeStatsLabel.
9095+
this->m_lowerer->GenerateBailOut(instr, nullptr, nullptr, sharedBailout ? bailoutLabel : nullptr);
91009096
}
91019097
else
91029098
{

‎lib/Backend/arm/LowerMD.cpp

Lines changed: 17 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8399,17 +8399,9 @@ LowererMD::GenerateFastInlineBuiltInMathFloor(IR::Instr* instr)
83998399
IR::RegOpnd* floatOpnd = IR::RegOpnd::New(TyFloat64, this->m_func);
84008400
this->m_lowerer->InsertMove(floatOpnd, src, instr);
84018401

8402-
IR::LabelInstr * bailoutLabel;
8402+
IR::LabelInstr * bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, /*helperLabel*/true);;
84038403
bool sharedBailout = (instr->GetBailOutInfo()->bailOutInstr != instr) ? true : false;
8404-
if(sharedBailout)
8405-
{
8406-
bailoutLabel = instr->GetBailOutInfo()->bailOutInstr->AsLabelInstr();
8407-
}
8408-
else
8409-
{
8410-
bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
8411-
}
8412-
8404+
84138405
// NaN check
84148406
IR::Instr *instrCmp = IR::Instr::New(Js::OpCode::VCMPF64, this->m_func);
84158407
instrCmp->SetSrc1(floatOpnd);
@@ -8481,7 +8473,10 @@ LowererMD::GenerateFastInlineBuiltInMathFloor(IR::Instr* instr)
84818473
{
84828474
instr->InsertBefore(bailoutLabel);
84838475
}
8484-
this->m_lowerer->GenerateBailOut(instr);
8476+
8477+
// In case of a shared bailout, we should jump to the code that sets some data on the bailout record which is specific
8478+
// to this bailout. Pass the bailoutLabel to GenerateFunction so that it may use the label as the collectRuntimeStatsLabel.
8479+
this->m_lowerer->GenerateBailOut(instr, nullptr, nullptr, sharedBailout ? bailoutLabel : nullptr);
84858480

84868481
// MOV dst, intOpnd
84878482
IR::Instr* movInstr = IR::Instr::New(Js::OpCode::MOV, dst, intOpnd, this->m_func);
@@ -8502,16 +8497,8 @@ LowererMD::GenerateFastInlineBuiltInMathCeil(IR::Instr* instr)
85028497
IR::RegOpnd* floatOpnd = IR::RegOpnd::New(TyFloat64, this->m_func);
85038498
this->m_lowerer->InsertMove(floatOpnd, src, instr);
85048499

8505-
IR::LabelInstr * bailoutLabel;
8500+
IR::LabelInstr * bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, /*helperLabel*/true);;
85068501
bool sharedBailout = (instr->GetBailOutInfo()->bailOutInstr != instr) ? true : false;
8507-
if(sharedBailout)
8508-
{
8509-
bailoutLabel = instr->GetBailOutInfo()->bailOutInstr->AsLabelInstr();
8510-
}
8511-
else
8512-
{
8513-
bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
8514-
}
85158502

85168503
// NaN check
85178504
IR::Instr *instrCmp = IR::Instr::New(Js::OpCode::VCMPF64, this->m_func);
@@ -8589,7 +8576,10 @@ LowererMD::GenerateFastInlineBuiltInMathCeil(IR::Instr* instr)
85898576
{
85908577
instr->InsertBefore(bailoutLabel);
85918578
}
8592-
this->m_lowerer->GenerateBailOut(instr);
8579+
8580+
// In case of a shared bailout, we should jump to the code that sets some data on the bailout record which is specific
8581+
// to this bailout. Pass the bailoutLabel to GenerateFunction so that it may use the label as the collectRuntimeStatsLabel.
8582+
this->m_lowerer->GenerateBailOut(instr, nullptr, nullptr, sharedBailout ? bailoutLabel : nullptr);
85938583

85948584
// MOV dst, intOpnd
85958585
IR::Instr* movInstr = IR::Instr::New(Js::OpCode::MOV, dst, intOpnd, this->m_func);
@@ -8610,17 +8600,9 @@ LowererMD::GenerateFastInlineBuiltInMathRound(IR::Instr* instr)
86108600
IR::RegOpnd* floatOpnd = IR::RegOpnd::New(TyFloat64, this->m_func);
86118601
this->m_lowerer->InsertMove(floatOpnd, src, instr);
86128602

8613-
IR::LabelInstr * bailoutLabel;
8603+
IR::LabelInstr * bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, /*helperLabel*/true);;
86148604
bool sharedBailout = (instr->GetBailOutInfo()->bailOutInstr != instr) ? true : false;
8615-
if(sharedBailout)
8616-
{
8617-
bailoutLabel = instr->GetBailOutInfo()->bailOutInstr->AsLabelInstr();
8618-
}
8619-
else
8620-
{
8621-
bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
8622-
}
8623-
8605+
86248606
// NaN check
86258607
IR::Instr *instrCmp = IR::Instr::New(Js::OpCode::VCMPF64, this->m_func);
86268608
instrCmp->SetSrc1(floatOpnd);
@@ -8701,7 +8683,10 @@ LowererMD::GenerateFastInlineBuiltInMathRound(IR::Instr* instr)
87018683
{
87028684
instr->InsertBefore(bailoutLabel);
87038685
}
8704-
this->m_lowerer->GenerateBailOut(instr);
8686+
8687+
// In case of a shared bailout, we should jump to the code that sets some data on the bailout record which is specific
8688+
// to this bailout. Pass the bailoutLabel to GenerateFunction so that it may use the label as the collectRuntimeStatsLabel.
8689+
this->m_lowerer->GenerateBailOut(instr, nullptr, nullptr, sharedBailout ? bailoutLabel : nullptr);
87058690

87068691
// MOV dst, intOpnd
87078692
IR::Instr* movInstr = IR::Instr::New(Js::OpCode::MOV, dst, intOpnd, this->m_func);

0 commit comments

Comments
 (0)