From 9f1ea28953cc8521e72229a5c7e19cd3f158347e Mon Sep 17 00:00:00 2001 From: Brian Fiete Date: Wed, 18 Sep 2019 13:00:44 -0700 Subject: [PATCH] Fixed bug with stack saving In certain cases when we need to remove the StackSave (because we crossed the save threshold with an allocation), there may already be restores using that stack save which need to be removed as well. --- IDEHelper/Backend/BeIRCodeGen.cpp | 8 ++++++-- IDEHelper/Backend/BeModule.cpp | 10 ++++++---- IDEHelper/Backend/BeModule.h | 2 ++ IDEHelper/Compiler/BfIRBuilder.cpp | 3 ++- IDEHelper/Compiler/BfIRBuilder.h | 2 +- IDEHelper/Compiler/BfIRCodeGen.cpp | 1 + IDEHelper/Compiler/BfModule.cpp | 8 ++++++-- IDEHelper/Compiler/BfModule.h | 1 + 8 files changed, 25 insertions(+), 10 deletions(-) diff --git a/IDEHelper/Backend/BeIRCodeGen.cpp b/IDEHelper/Backend/BeIRCodeGen.cpp index ab3091cf..d0f8edc6 100644 --- a/IDEHelper/Backend/BeIRCodeGen.cpp +++ b/IDEHelper/Backend/BeIRCodeGen.cpp @@ -1767,8 +1767,11 @@ void BeIRCodeGen::HandleNextCmd() CMD_PARAM(BeValue*, instVal); BeInst* inst = (BeInst*)instVal; - auto itr = std::find(inst->mParentBlock->mInstructions.begin(), inst->mParentBlock->mInstructions.end(), inst); - inst->mParentBlock->mInstructions.erase(itr); + bool wasRemoved = inst->mParentBlock->mInstructions.Remove(inst); + BF_ASSERT(wasRemoved); +#ifdef _DEBUG + inst->mWasRemoved = true; +#endif } break; case BfIRCmd_CreateBr: @@ -3050,6 +3053,7 @@ BeValue* BeIRCodeGen::GetBeValue(int id) BF_ASSERT(result.mKind == BeIRCodeGenEntryKind_Value); #ifdef _DEBUG BF_ASSERT(!result.mBeValue->mLifetimeEnded); + BF_ASSERT(!result.mBeValue->mWasRemoved); #endif return result.mBeValue; } diff --git a/IDEHelper/Backend/BeModule.cpp b/IDEHelper/Backend/BeModule.cpp index ecb035b8..c4d8f962 100644 --- a/IDEHelper/Backend/BeModule.cpp +++ b/IDEHelper/Backend/BeModule.cpp @@ -2940,10 +2940,12 @@ void BeModule::AddBlock(BeFunction* function, BeBlock* block) void BeModule::RemoveBlock(BeFunction* function, BeBlock* block) { - auto itr = std::find(function->mBlocks.begin(), function->mBlocks.end(), block); - BF_ASSERT(itr != function->mBlocks.end()); - if (itr != function->mBlocks.end()) - function->mBlocks.erase(itr); + bool didRemove = function->mBlocks.Remove(block); + BF_ASSERT(didRemove); +#ifdef _DEBUG + for (auto inst : block->mInstructions) + inst->mWasRemoved = true; +#endif } BeBlock* BeModule::GetInsertBlock() diff --git a/IDEHelper/Backend/BeModule.h b/IDEHelper/Backend/BeModule.h index 4c858725..b7e3b4be 100644 --- a/IDEHelper/Backend/BeModule.h +++ b/IDEHelper/Backend/BeModule.h @@ -207,9 +207,11 @@ class BeValue : public BeHashble public: #ifdef _DEBUG bool mLifetimeEnded; + bool mWasRemoved; BeValue() { mLifetimeEnded = false; + mWasRemoved = false; } #endif diff --git a/IDEHelper/Compiler/BfIRBuilder.cpp b/IDEHelper/Compiler/BfIRBuilder.cpp index 9a847e87..7840546d 100644 --- a/IDEHelper/Compiler/BfIRBuilder.cpp +++ b/IDEHelper/Compiler/BfIRBuilder.cpp @@ -3843,10 +3843,11 @@ BfIRValue BfIRBuilder::CreateStackSave() return retVal; } -void BfIRBuilder::CreateStackRestore(BfIRValue stackVal) +BfIRValue BfIRBuilder::CreateStackRestore(BfIRValue stackVal) { BfIRValue retVal = WriteCmd(BfIRCmd_StackRestore, stackVal); NEW_CMD_INSERTED; + return retVal; } BfIRValue BfIRBuilder::CreateGlobalVariable(BfIRType varType, bool isConstant, BfIRLinkageType linkageType, BfIRValue initializer, const StringImpl& name, bool isTLS) diff --git a/IDEHelper/Compiler/BfIRBuilder.h b/IDEHelper/Compiler/BfIRBuilder.h index 032efef7..3e6604f1 100644 --- a/IDEHelper/Compiler/BfIRBuilder.h +++ b/IDEHelper/Compiler/BfIRBuilder.h @@ -1099,7 +1099,7 @@ public: BfIRValue CreateMemSet(BfIRValue addr, BfIRValue val, BfIRValue size, int align); void CreateFence(BfIRFenceType fenceType); BfIRValue CreateStackSave(); - void CreateStackRestore(BfIRValue stackVal); + BfIRValue CreateStackRestore(BfIRValue stackVal); BfIRValue CreateGlobalVariable(BfIRType varType, bool isConstant, BfIRLinkageType linkageType, BfIRValue initializer, const StringImpl& name, bool isTLS = false); void GlobalVar_SetUnnamedAddr(BfIRValue val, bool unnamedAddr); diff --git a/IDEHelper/Compiler/BfIRCodeGen.cpp b/IDEHelper/Compiler/BfIRCodeGen.cpp index 953fb348..7880bd6a 100644 --- a/IDEHelper/Compiler/BfIRCodeGen.cpp +++ b/IDEHelper/Compiler/BfIRCodeGen.cpp @@ -1585,6 +1585,7 @@ void BfIRCodeGen::HandleNextCmd() CMD_PARAM(llvm::Value*, stackVal); auto intrin = llvm::Intrinsic::getDeclaration(mLLVMModule, llvm::Intrinsic::stackrestore); auto callInst = mIRBuilder->CreateCall(intrin, llvm::SmallVector {stackVal }); + SetResult(curId, callInst); } break; case BfIRCmd_GlobalVariable: diff --git a/IDEHelper/Compiler/BfModule.cpp b/IDEHelper/Compiler/BfModule.cpp index ffa8fab4..072551e7 100644 --- a/IDEHelper/Compiler/BfModule.cpp +++ b/IDEHelper/Compiler/BfModule.cpp @@ -2122,8 +2122,12 @@ void BfModule::SaveStackState(BfScopeData* scopeData) } if (checkScope->mSavedStack) { + for (auto usage : checkScope->mSavedStackUses) + mBfIRBuilder->EraseInstFromParent(usage); + checkScope->mSavedStackUses.Clear(); + mBfIRBuilder->EraseInstFromParent(checkScope->mSavedStack); - checkScope->mSavedStack = BfIRValue(); + checkScope->mSavedStack = BfIRValue(); } checkScope = checkScope->mPrevScope; } @@ -12394,7 +12398,7 @@ void BfModule::EmitDeferredScopeCalls(bool useSrcPositions, BfScopeData* scopeDa if (checkScope->mSavedStack) { - mBfIRBuilder->CreateStackRestore(checkScope->mSavedStack); + checkScope->mSavedStackUses.Add(mBfIRBuilder->CreateStackRestore(checkScope->mSavedStack)); if (mCurMethodState->mDynStackRevIdx) { diff --git a/IDEHelper/Compiler/BfModule.h b/IDEHelper/Compiler/BfModule.h index 4d723428..0df72c59 100644 --- a/IDEHelper/Compiler/BfModule.h +++ b/IDEHelper/Compiler/BfModule.h @@ -315,6 +315,7 @@ public: BfIRValue mBlock; BfIRValue mValueScopeStart; BfIRValue mSavedStack; + Array mSavedStackUses; Array mDeferredHandlers; // These get cleared when us our a parent gets new entries added into mDeferredCallEntries Array mAtEndBlocks; // Move these to the end after we close scope Array mDeferredLifetimeEnds;