From: Duncan P. N. Exon Smith Date: Fri, 1 Aug 2014 21:51:52 +0000 (+0000) Subject: BitcodeReader: Change mechanics of BlockAddress forward references, NFC X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=147851ef9d11173c012217511be257ebefe695c4;p=oota-llvm.git BitcodeReader: Change mechanics of BlockAddress forward references, NFC Now that we can reliably handle forward references to `BlockAddress` (r214563), change the mechanics to simplify predicting use-list order. Previously, we created dummy `GlobalVariable`s to represent block addresses. After every function was materialized, we'd go through any forward references to its blocks and RAUW them with a proper `BlockAddress` constant. This causes some (potentially a lot of) unnecessary use-list churn, since any constant expression that it's a part of will need to be rematerialized as well. Instead, pre-construct a `BasicBlock` immediately -- without attaching it to its (empty) `Function` -- and use that to construct a `BlockAddress`. This constant will not have to be regenerated. When the function body is parsed, hook this pre-constructed basic block up in the right place using `BasicBlock::insertInto()`. Both before and after this change, the IR is temporarily in an invalid state that gets resolved when `materializeForwardReferencedFunctions()` gets called. This is a prep commit that's part of PR5680, but the only functionality change is the reduction of churn in the constant pool. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@214570 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 8537354a5b3..48fa9390412 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -38,8 +38,8 @@ std::error_code BitcodeReader::materializeForwardReferencedFunctions() { // Prevent recursion. WillMaterializeAllForwardRefs = true; - while (!BlockAddrFwdRefs.empty()) { - Function *F = BlockAddrFwdRefs.begin()->first; + while (!BasicBlockFwdRefs.empty()) { + Function *F = BasicBlockFwdRefs.begin()->first; assert(F && "Expected valid function"); // Check for a function that isn't materializable to prevent an infinite // loop. When parsing a blockaddress stored in a global variable, there @@ -71,7 +71,7 @@ void BitcodeReader::FreeState() { DeferredFunctionInfo.clear(); MDKindMap.clear(); - assert(BlockAddrFwdRefs.empty() && "Unresolved blockaddress fwd references"); + assert(BasicBlockFwdRefs.empty() && "Unresolved blockaddress fwd references"); } //===----------------------------------------------------------------------===// @@ -1612,24 +1612,26 @@ std::error_code BitcodeReader::ParseConstants() { // If the function is already parsed we can insert the block address right // away. + BasicBlock *BB; + unsigned BBID = Record[2]; + if (!BBID) + // Invalid reference to entry block. + return Error(BitcodeError::InvalidID); if (!Fn->empty()) { Function::iterator BBI = Fn->begin(), BBE = Fn->end(); - for (size_t I = 0, E = Record[2]; I != E; ++I) { + for (size_t I = 0, E = BBID; I != E; ++I) { if (BBI == BBE) return Error(BitcodeError::InvalidID); ++BBI; } - V = BlockAddress::get(Fn, BBI); + BB = BBI; } else { // Otherwise insert a placeholder and remember it so it can be inserted // when the function is parsed. - GlobalVariable *FwdRef = new GlobalVariable(*Fn->getParent(), - Type::getInt8Ty(Context), - false, GlobalValue::InternalLinkage, - nullptr, ""); - BlockAddrFwdRefs[Fn].push_back(std::make_pair(Record[2], FwdRef)); - V = FwdRef; + BB = BasicBlock::Create(Context); + BasicBlockFwdRefs[Fn].emplace_back(BBID, BB); } + V = BlockAddress::get(Fn, BB); break; } } @@ -2367,15 +2369,45 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) { switch (BitCode) { default: // Default behavior: reject return Error(BitcodeError::InvalidValue); - case bitc::FUNC_CODE_DECLAREBLOCKS: // DECLAREBLOCKS: [nblocks] + case bitc::FUNC_CODE_DECLAREBLOCKS: { // DECLAREBLOCKS: [nblocks] if (Record.size() < 1 || Record[0] == 0) return Error(BitcodeError::InvalidRecord); // Create all the basic blocks for the function. FunctionBBs.resize(Record[0]); - for (unsigned i = 0, e = FunctionBBs.size(); i != e; ++i) - FunctionBBs[i] = BasicBlock::Create(Context, "", F); + + // See if anything took the address of blocks in this function. + auto BBFRI = BasicBlockFwdRefs.find(F); + if (BBFRI == BasicBlockFwdRefs.end()) { + for (unsigned i = 0, e = FunctionBBs.size(); i != e; ++i) + FunctionBBs[i] = BasicBlock::Create(Context, "", F); + } else { + auto &BBRefs = BBFRI->second; + std::sort(BBRefs.begin(), BBRefs.end(), + [](const std::pair &LHS, + const std::pair &RHS) { + return LHS.first < RHS.first; + }); + unsigned R = 0, RE = BBRefs.size(); + for (unsigned I = 0, E = FunctionBBs.size(); I != E; ++I) + if (R != RE && BBRefs[R].first == I) { + assert(I != 0 && "Invalid reference to entry block"); + BasicBlock *BB = BBRefs[R++].second; + BB->insertInto(F); + FunctionBBs[I] = BB; + } else { + FunctionBBs[I] = BasicBlock::Create(Context, "", F); + } + // Check for invalid basic block references. + if (R != RE) + return Error(BitcodeError::InvalidID); + + // Erase from the table. + BasicBlockFwdRefs.erase(BBFRI); + } + CurBB = FunctionBBs[0]; continue; + } case bitc::FUNC_CODE_DEBUG_LOC_AGAIN: // DEBUG_LOC_AGAIN // This record indicates that the last instruction is at the same @@ -3210,25 +3242,6 @@ OutOfRecordLoop: // FIXME: Check for unresolved forward-declared metadata references // and clean up leaks. - // See if anything took the address of blocks in this function. If so, - // resolve them now. - DenseMap >::iterator BAFRI = - BlockAddrFwdRefs.find(F); - if (BAFRI != BlockAddrFwdRefs.end()) { - std::vector &RefList = BAFRI->second; - for (unsigned i = 0, e = RefList.size(); i != e; ++i) { - unsigned BlockIdx = RefList[i].first; - if (BlockIdx >= FunctionBBs.size()) - return Error(BitcodeError::InvalidID); - - GlobalVariable *FwdRef = RefList[i].second; - FwdRef->replaceAllUsesWith(BlockAddress::get(F, FunctionBBs[BlockIdx])); - FwdRef->eraseFromParent(); - } - - BlockAddrFwdRefs.erase(BAFRI); - } - // Trim the value list down to the size it was before we parsed this function. ValueList.shrinkTo(ModuleValueListSize); MDValueList.shrinkTo(ModuleMDValueListSize); @@ -3351,7 +3364,7 @@ std::error_code BitcodeReader::MaterializeModule(Module *M) { // Check that all block address forward references got resolved (as we // promised above). - if (!BlockAddrFwdRefs.empty()) + if (!BasicBlockFwdRefs.empty()) return Error(BitcodeError::NeverResolvedFunctionFromBlockAddress); // Upgrade any intrinsic calls that slipped through (should not happen!) and diff --git a/lib/Bitcode/Reader/BitcodeReader.h b/lib/Bitcode/Reader/BitcodeReader.h index 341e134302d..a25ab37a4e3 100644 --- a/lib/Bitcode/Reader/BitcodeReader.h +++ b/lib/Bitcode/Reader/BitcodeReader.h @@ -179,10 +179,10 @@ class BitcodeReader : public GVMaterializer { /// stream. DenseMap DeferredFunctionInfo; - /// BlockAddrFwdRefs - These are blockaddr references to basic blocks. These - /// are resolved lazily when functions are loaded. - typedef std::pair BlockAddrRefTy; - DenseMap > BlockAddrFwdRefs; + /// These are basic blocks forward-referenced by block addresses. They are + /// inserted lazily into functions when they're loaded. + typedef std::pair BasicBlockRefTy; + DenseMap> BasicBlockFwdRefs; /// UseRelativeIDs - Indicates that we are using a new encoding for /// instruction operands where most operands in the current