Skip to content

Commit 22d2fb0

Browse files
rajatdrajatd
committed
[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 2bf9cf4 + dc20aef commit 22d2fb0

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
@@ -12157,11 +12157,10 @@ Lowerer::GenerateObjectTestAndTypeLoad(IR::Instr *instrLdSt, IR::RegOpnd *opndBa
1215712157
}
1215812158

1215912159
IR::LabelInstr *
12160-
Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::LabelInstr *bailOutLabel)
12160+
Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::LabelInstr *bailOutLabel, IR::LabelInstr * collectRuntimeStatsLabel)
1216112161
{
1216212162
BailOutInfo * bailOutInfo = instr->GetBailOutInfo();
1216312163
IR::Instr * bailOutInstr = bailOutInfo->bailOutInstr;
12164-
IR::LabelInstr *collectRuntimeStatsLabel = nullptr;
1216512164
if (instr->IsCloned())
1216612165
{
1216712166
Assert(bailOutInstr != instr);
@@ -12174,14 +12173,18 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L
1217412173
return bailOutLabel;
1217512174
}
1217612175

12176+
// Add helper label to trigger layout.
12177+
if (!collectRuntimeStatsLabel)
12178+
{
12179+
collectRuntimeStatsLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
12180+
}
12181+
Assert(!collectRuntimeStatsLabel->IsLinked());
12182+
instr->InsertBefore(collectRuntimeStatsLabel);
12183+
1217712184
if (bailOutInstr != instr)
1217812185
{
1217912186
// this bailOutInfo is shared, just jump to the bailout target
1218012187

12181-
// Add helper label to trigger layout.
12182-
collectRuntimeStatsLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
12183-
instr->InsertBefore(collectRuntimeStatsLabel);
12184-
1218512188
IR::MemRefOpnd *pIndexOpndForBailOutKind =
1218612189
IR::MemRefOpnd::New((BYTE*)bailOutInfo->bailOutRecord + BailOutRecord::GetOffsetOfBailOutKind(), TyUint32, this->m_func, IR::AddrOpndKindDynamicBailOutKindRef);
1218712190
m_lowererMD.CreateAssign(
@@ -12229,10 +12232,6 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L
1222912232
// The bailout hasn't be generated yet.
1223012233
Assert(!bailOutInstr->IsLabelInstr());
1223112234

12232-
// Add helper label to trigger layout.
12233-
collectRuntimeStatsLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
12234-
instr->InsertBefore(collectRuntimeStatsLabel);
12235-
1223612235
// capture the condition for this bailout
1223712236
if (bailOutLabel == nullptr)
1223812237
{

‎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
@@ -8858,14 +8858,7 @@ void LowererMD::GenerateFastInlineBuiltInCall(IR::Instr* instr, IR::JnHelperMeth
88588858
if (instr->GetDst()->IsInt32())
88598859
{
88608860
sharedBailout = (instr->GetBailOutInfo()->bailOutInstr != instr) ? true : false;
8861-
if (sharedBailout)
8862-
{
8863-
bailoutLabel = instr->GetBailOutInfo()->bailOutInstr->AsLabelInstr();
8864-
}
8865-
else
8866-
{
8867-
bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, /*helperLabel*/true);
8868-
}
8861+
bailoutLabel = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, /*helperLabel*/true);
88698862
}
88708863

88718864
IR::Opnd * zero;
@@ -9080,7 +9073,10 @@ void LowererMD::GenerateFastInlineBuiltInCall(IR::Instr* instr, IR::JnHelperMeth
90809073
{
90819074
instr->InsertBefore(bailoutLabel);
90829075
}
9083-
this->m_lowerer->GenerateBailOut(instr);
9076+
9077+
// In case of a shared bailout, we should jump to the code that sets some data on the bailout record which is specific
9078+
// to this bailout. Pass the bailoutLabel to GenerateFunction so that it may use the label as the collectRuntimeStatsLabel.
9079+
this->m_lowerer->GenerateBailOut(instr, nullptr, nullptr, sharedBailout ? bailoutLabel : nullptr);
90849080
}
90859081
else
90869082
{

‎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)