From f8f83da51268e0556b4458b3ad9069eb850fc289 Mon Sep 17 00:00:00 2001 From: pchintalapudi <34727397+pchintalapudi@users.noreply.github.com> Date: Mon, 4 Apr 2022 19:32:45 -0400 Subject: [PATCH] Fix leftover indentation TODOs (#44854) --- src/codegen.cpp | 16 ++-- src/disasm.cpp | 10 +-- src/jitlayers.cpp | 217 +++++++++++++++++++++++----------------------- 3 files changed, 121 insertions(+), 122 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index 4cb72567df63b..fc4327564cddf 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -7885,10 +7885,10 @@ static jl_llvm_functions_t // so that they don't conflict when they show up in the execution engine. for (auto &TSMod : ctx.llvmcall_modules) { SmallVector Exports; - TSMod.withModuleDo([&](Module &Mod) { //TODO fix indentation after review - for (const auto &F: Mod.functions()) - if (!F.isDeclaration()) - Exports.push_back(F.getName().str()); + TSMod.withModuleDo([&](Module &Mod) { + for (const auto &F: Mod.functions()) + if (!F.isDeclaration()) + Exports.push_back(F.getName().str()); }); jl_merge_module(TSM, std::move(TSMod)); for (auto FN: Exports) @@ -7898,10 +7898,10 @@ static jl_llvm_functions_t // link in opaque closure modules for (auto &TSMod : ctx.oc_modules) { SmallVector Exports; - TSMod.withModuleDo([&](Module &Mod) { //TODO fix indentation after review - for (const auto &F: Mod.functions()) - if (!F.isDeclaration()) - Exports.push_back(F.getName().str()); + TSMod.withModuleDo([&](Module &Mod) { + for (const auto &F: Mod.functions()) + if (!F.isDeclaration()) + Exports.push_back(F.getName().str()); }); jl_merge_module(TSM, std::move(TSMod)); for (auto FN: Exports) diff --git a/src/disasm.cpp b/src/disasm.cpp index 0946b5b9734eb..f43dc095a78f4 100644 --- a/src/disasm.cpp +++ b/src/disasm.cpp @@ -1198,11 +1198,11 @@ jl_value_t *jl_dump_function_asm_impl(void *F, char raw_mc, const char* asm_vari Function *f = dump->F; llvm::raw_svector_ostream asmfile(ObjBufferSV); assert(!f->isDeclaration()); - dump->TSM.withModuleDo([&](Module &m) {//TODO fix indentation - for (auto &f2 : m.functions()) { - if (f != &f2 && !f->isDeclaration()) - f2.deleteBody(); - } + dump->TSM.withModuleDo([&](Module &m) { + for (auto &f2 : m.functions()) { + if (f != &f2 && !f->isDeclaration()) + f2.deleteBody(); + } }); LLVMTargetMachine *TM = static_cast(&jl_ExecutionEngine->getTargetMachine()); legacy::PassManager PM; diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp index ae4db5b36b67c..1df535d7d5346 100644 --- a/src/jitlayers.cpp +++ b/src/jitlayers.cpp @@ -1222,110 +1222,109 @@ void jl_merge_module(orc::ThreadSafeModule &destTSM, orc::ThreadSafeModule srcTS { destTSM.withModuleDo([&](Module &dest) { srcTSM.withModuleDo([&](Module &src) { - //TODO fix indentation after review - assert(&dest != &src && "Cannot merge module with itself!"); - assert(&dest.getContext() == &src.getContext() && "Cannot merge modules with different contexts!"); - assert(dest.getDataLayout() == src.getDataLayout() && "Cannot merge modules with different data layouts!"); - assert(dest.getTargetTriple() == src.getTargetTriple() && "Cannot merge modules with different target triples!"); - - for (Module::global_iterator I = src.global_begin(), E = src.global_end(); I != E;) { - GlobalVariable *sG = &*I; - GlobalVariable *dG = cast_or_null(dest.getNamedValue(sG->getName())); - ++I; - // Replace a declaration with the definition: - if (dG) { - if (sG->isDeclaration()) { - sG->replaceAllUsesWith(dG); - sG->eraseFromParent(); - continue; - } - //// If we start using llvm.used, we need to enable and test this - //else if (!dG->isDeclaration() && dG->hasAppendingLinkage() && sG->hasAppendingLinkage()) { - // auto *dCA = cast(dG->getInitializer()); - // auto *sCA = cast(sG->getInitializer()); - // SmallVector Init; - // for (auto &Op : dCA->operands()) - // Init.push_back(cast_or_null(Op)); - // for (auto &Op : sCA->operands()) - // Init.push_back(cast_or_null(Op)); - // Type *Int8PtrTy = Type::getInt8PtrTy(dest.getContext()); - // ArrayType *ATy = ArrayType::get(Int8PtrTy, Init.size()); - // GlobalVariable *GV = new GlobalVariable(dest, ATy, dG->isConstant(), - // GlobalValue::AppendingLinkage, ConstantArray::get(ATy, Init), "", - // dG->getThreadLocalMode(), dG->getType()->getAddressSpace()); - // GV->copyAttributesFrom(dG); - // sG->replaceAllUsesWith(GV); - // dG->replaceAllUsesWith(GV); - // GV->takeName(sG); - // sG->eraseFromParent(); - // dG->eraseFromParent(); - // continue; - //} - else { - assert(dG->isDeclaration() || dG->getInitializer() == sG->getInitializer()); - dG->replaceAllUsesWith(sG); - dG->eraseFromParent(); + assert(&dest != &src && "Cannot merge module with itself!"); + assert(&dest.getContext() == &src.getContext() && "Cannot merge modules with different contexts!"); + assert(dest.getDataLayout() == src.getDataLayout() && "Cannot merge modules with different data layouts!"); + assert(dest.getTargetTriple() == src.getTargetTriple() && "Cannot merge modules with different target triples!"); + + for (Module::global_iterator I = src.global_begin(), E = src.global_end(); I != E;) { + GlobalVariable *sG = &*I; + GlobalVariable *dG = cast_or_null(dest.getNamedValue(sG->getName())); + ++I; + // Replace a declaration with the definition: + if (dG) { + if (sG->isDeclaration()) { + sG->replaceAllUsesWith(dG); + sG->eraseFromParent(); + continue; + } + //// If we start using llvm.used, we need to enable and test this + //else if (!dG->isDeclaration() && dG->hasAppendingLinkage() && sG->hasAppendingLinkage()) { + // auto *dCA = cast(dG->getInitializer()); + // auto *sCA = cast(sG->getInitializer()); + // SmallVector Init; + // for (auto &Op : dCA->operands()) + // Init.push_back(cast_or_null(Op)); + // for (auto &Op : sCA->operands()) + // Init.push_back(cast_or_null(Op)); + // Type *Int8PtrTy = Type::getInt8PtrTy(dest.getContext()); + // ArrayType *ATy = ArrayType::get(Int8PtrTy, Init.size()); + // GlobalVariable *GV = new GlobalVariable(dest, ATy, dG->isConstant(), + // GlobalValue::AppendingLinkage, ConstantArray::get(ATy, Init), "", + // dG->getThreadLocalMode(), dG->getType()->getAddressSpace()); + // GV->copyAttributesFrom(dG); + // sG->replaceAllUsesWith(GV); + // dG->replaceAllUsesWith(GV); + // GV->takeName(sG); + // sG->eraseFromParent(); + // dG->eraseFromParent(); + // continue; + //} + else { + assert(dG->isDeclaration() || dG->getInitializer() == sG->getInitializer()); + dG->replaceAllUsesWith(sG); + dG->eraseFromParent(); + } + } + // Reparent the global variable: + sG->removeFromParent(); + dest.getGlobalList().push_back(sG); + // Comdat is owned by the Module + sG->setComdat(nullptr); } - } - // Reparent the global variable: - sG->removeFromParent(); - dest.getGlobalList().push_back(sG); - // Comdat is owned by the Module - sG->setComdat(nullptr); - } - for (Module::iterator I = src.begin(), E = src.end(); I != E;) { - Function *sG = &*I; - Function *dG = cast_or_null(dest.getNamedValue(sG->getName())); - ++I; - // Replace a declaration with the definition: - if (dG) { - if (sG->isDeclaration()) { - sG->replaceAllUsesWith(dG); - sG->eraseFromParent(); - continue; - } - else { - assert(dG->isDeclaration()); - dG->replaceAllUsesWith(sG); - dG->eraseFromParent(); + for (Module::iterator I = src.begin(), E = src.end(); I != E;) { + Function *sG = &*I; + Function *dG = cast_or_null(dest.getNamedValue(sG->getName())); + ++I; + // Replace a declaration with the definition: + if (dG) { + if (sG->isDeclaration()) { + sG->replaceAllUsesWith(dG); + sG->eraseFromParent(); + continue; + } + else { + assert(dG->isDeclaration()); + dG->replaceAllUsesWith(sG); + dG->eraseFromParent(); + } + } + // Reparent the global variable: + sG->removeFromParent(); + dest.getFunctionList().push_back(sG); + // Comdat is owned by the Module + sG->setComdat(nullptr); } - } - // Reparent the global variable: - sG->removeFromParent(); - dest.getFunctionList().push_back(sG); - // Comdat is owned by the Module - sG->setComdat(nullptr); - } - for (Module::alias_iterator I = src.alias_begin(), E = src.alias_end(); I != E;) { - GlobalAlias *sG = &*I; - GlobalAlias *dG = cast_or_null(dest.getNamedValue(sG->getName())); - ++I; - if (dG) { - if (!dG->isDeclaration()) { // aliases are always definitions, so this test is reversed from the above two - sG->replaceAllUsesWith(dG); - sG->eraseFromParent(); - continue; - } - else { - dG->replaceAllUsesWith(sG); - dG->eraseFromParent(); + for (Module::alias_iterator I = src.alias_begin(), E = src.alias_end(); I != E;) { + GlobalAlias *sG = &*I; + GlobalAlias *dG = cast_or_null(dest.getNamedValue(sG->getName())); + ++I; + if (dG) { + if (!dG->isDeclaration()) { // aliases are always definitions, so this test is reversed from the above two + sG->replaceAllUsesWith(dG); + sG->eraseFromParent(); + continue; + } + else { + dG->replaceAllUsesWith(sG); + dG->eraseFromParent(); + } + } + sG->removeFromParent(); + dest.getAliasList().push_back(sG); } - } - sG->removeFromParent(); - dest.getAliasList().push_back(sG); - } - // metadata nodes need to be explicitly merged not just copied - // so there are special passes here for each known type of metadata - NamedMDNode *sNMD = src.getNamedMetadata("llvm.dbg.cu"); - if (sNMD) { - NamedMDNode *dNMD = dest.getOrInsertNamedMetadata("llvm.dbg.cu"); - for (NamedMDNode::op_iterator I = sNMD->op_begin(), E = sNMD->op_end(); I != E; ++I) { - dNMD->addOperand(*I); - } - } + // metadata nodes need to be explicitly merged not just copied + // so there are special passes here for each known type of metadata + NamedMDNode *sNMD = src.getNamedMetadata("llvm.dbg.cu"); + if (sNMD) { + NamedMDNode *dNMD = dest.getOrInsertNamedMetadata("llvm.dbg.cu"); + for (NamedMDNode::op_iterator I = sNMD->op_begin(), E = sNMD->op_end(); I != E; ++I) { + dNMD->addOperand(*I); + } + } }); }); } @@ -1393,19 +1392,19 @@ static int jl_add_to_ee( } int MergeUp = depth; // Compute the cycle-id - M.withModuleDo([&](Module &m) {//TODO fix indentation - for (auto &F : m.global_objects()) { - if (F.isDeclaration() && F.getLinkage() == GlobalValue::ExternalLinkage) { - auto Callee = NewExports.find(F.getName()); - if (Callee != NewExports.end()) { - auto &CM = Callee->second; - int Down = jl_add_to_ee(*CM, NewExports, Queued, ToMerge, depth + 1); - assert(Down <= depth); - if (Down && Down < MergeUp) - MergeUp = Down; + M.withModuleDo([&](Module &m) { + for (auto &F : m.global_objects()) { + if (F.isDeclaration() && F.getLinkage() == GlobalValue::ExternalLinkage) { + auto Callee = NewExports.find(F.getName()); + if (Callee != NewExports.end()) { + auto &CM = Callee->second; + int Down = jl_add_to_ee(*CM, NewExports, Queued, ToMerge, depth + 1); + assert(Down <= depth); + if (Down && Down < MergeUp) + MergeUp = Down; + } } } - } }); if (MergeUp == depth) { // Not in a cycle (or at the top of it)