From e39bb485ea1c53980c11b37cd86e1d8f930a4506 Mon Sep 17 00:00:00 2001 From: Andrey Guskov Date: Tue, 19 May 2015 22:01:19 +0300 Subject: [PATCH 1/2] Fixed global variable handling --- module.cpp | 160 ++++++++++++++++++----------------------------------- 1 file changed, 54 insertions(+), 106 deletions(-) diff --git a/module.cpp b/module.cpp index 5ed3c4d0..1487ddd3 100644 --- a/module.cpp +++ b/module.cpp @@ -2679,95 +2679,6 @@ struct RewriteGlobalInfo { SourcePos pos; }; -// Grab all of the global value definitions from the module and change them -// to be declarations; we'll emit a single definition of each global in the -// final module used with the dispatch functions, so that we don't have -// multiple definitions of them, one in each of the target-specific output -// files. -static void -lExtractAndRewriteGlobals(llvm::Module *module, - std::vector *globals) { - llvm::Module::global_iterator iter; - for (iter = module->global_begin(); iter != module->global_end(); ++iter) { - llvm::GlobalVariable *gv = iter; - // Is it a global definition? - if (gv->getLinkage() == llvm::GlobalValue::ExternalLinkage && - gv->hasInitializer()) { - // Turn this into an 'extern 'declaration by clearing its - // initializer. - llvm::Constant *init = gv->getInitializer(); - gv->setInitializer(NULL); - - Symbol *sym = - m->symbolTable->LookupVariable(gv->getName().str().c_str()); - Assert(sym != NULL); - globals->push_back(RewriteGlobalInfo(gv, init, sym->pos)); - } - } -} - - -// This function emits a global variable definition for each global that -// was turned into a declaration in the target-specific output file. -static void -lAddExtractedGlobals(llvm::Module *module, - std::vector globals[Target::NUM_ISAS]) { - // Find the first element in the globals[] array that has values stored - // in it. All elements of this array should either have empty vectors - // (if we didn't compile to the corresponding ISA or if there are no - // globals), or should have the same number of vector elements as the - // other non-empty vectors. - int firstActive = -1; - for (int i = 0; i < Target::NUM_ISAS; ++i) - if (globals[i].size() > 0) { - firstActive = i; - break; - } - - if (firstActive == -1) - // no globals - return; - - for (unsigned int i = 0; i < globals[firstActive].size(); ++i) { - RewriteGlobalInfo &rgi = globals[firstActive][i]; - llvm::GlobalVariable *gv = rgi.gv; - llvm::Type *type = gv->getType()->getElementType(); - llvm::Constant *initializer = rgi.init; - - // Create a new global in the given model that matches the original - // global - llvm::GlobalVariable *newGlobal = - new llvm::GlobalVariable(*module, type, gv->isConstant(), - llvm::GlobalValue::ExternalLinkage, - initializer, gv->getName()); - newGlobal->copyAttributesFrom(gv); - - // For all of the other targets that we actually generated code - // for, make sure the global we just created is compatible with the - // global from the module for that target. - for (int j = firstActive + 1; j < Target::NUM_ISAS; ++j) { - if (globals[j].size() > 0) { - // There should be the same number of globals in the other - // vectors, in the same order. - Assert(globals[firstActive].size() == globals[j].size()); - llvm::GlobalVariable *gv2 = globals[j][i].gv; - Assert(gv2->getName() == gv->getName()); - - // It is possible that the types may not match, though--for - // example, this happens with varying globals if we compile - // to different vector widths. - if (gv2->getType() != gv->getType()) - Warning(rgi.pos, "Mismatch in size/layout of global " - "variable \"%s\" with different targets. " - "Globals must not include \"varying\" types or arrays " - "with size based on programCount when compiling to " - "targets with differing vector widths.", - gv->getName().str().c_str()); - } - } - } -} - static llvm::FunctionType * lGetVaryingDispatchType(FunctionTargetVariants &funcs) { llvm::Type *ptrToInt8Ty = llvm::Type::getInt8PtrTy(*g->ctx); @@ -2969,13 +2880,8 @@ lCreateDispatchFunction(llvm::Module *module, llvm::Function *setISAFunc, } } -// Given a map that holds the mapping from each of the 'export'ed functions -// in the ispc program to the target-specific variants of the function, -// create a llvm::Module that has a dispatch function for each exported -// function that checks the system's capabilities and picks the most -// appropriate compiled variant of the function. -static llvm::Module * -lCreateDispatchModule(std::map &functions) { +// Initialize a dispatch module +static llvm::Module *lInitDispatchModule() { llvm::Module *module = new llvm::Module("dispatch_module", *g->ctx); // First, link in the definitions from the builtins-dispatch.ll file. @@ -2983,7 +2889,17 @@ lCreateDispatchModule(std::map &functions) extern int builtins_bitcode_dispatch_length; AddBitcodeToModule(builtins_bitcode_dispatch, builtins_bitcode_dispatch_length, module); + return module; +} +// Complete the creation of a dispatch module. +// Given a map that holds the mapping from each of the 'export'ed functions +// in the ispc program to the target-specific variants of the function, +// create a llvm::Module that has a dispatch function for each exported +// function that checks the system's capabilities and picks the most +// appropriate compiled variant of the function. +static void lEmitDispatchModule(llvm::Module *module, + std::map &functions) { // Get pointers to things we need below llvm::Function *setFunc = module->getFunction("__set_system_isa"); Assert(setFunc != NULL); @@ -2999,7 +2915,6 @@ lCreateDispatchModule(std::map &functions) // Do some rudimentary cleanup of the final result and make sure that // the module is all ok. - #if defined(LLVM_3_2) || defined(LLVM_3_3) || defined(LLVM_3_4) || defined(LLVM_3_5) || defined(LLVM_3_6) llvm::PassManager optPM; #else // LLVM 3.7+ @@ -3008,10 +2923,46 @@ lCreateDispatchModule(std::map &functions) optPM.add(llvm::createGlobalDCEPass()); optPM.add(llvm::createVerifierPass()); optPM.run(*module); - - return module; } +// Grab all of the global value definitions from the module and change them +// to be declarations; we'll emit a single definition of each global in the +// final module used with the dispatch functions, so that we don't have +// multiple definitions of them, one in each of the target-specific output +// files. +static void +lExtractGlobals(llvm::Module *module, llvm::Module **mdisp) { + llvm::Module::global_iterator iter; + + if (*mdisp) + return; + + *mdisp = lInitDispatchModule(); + for (iter = module->global_begin(); iter != module->global_end(); ++iter) { + llvm::GlobalVariable *gv = iter; + // Is it a global definition? + if (gv->getLinkage() == llvm::GlobalValue::ExternalLinkage && + gv->hasInitializer()) { + // Turn this into an 'extern 'declaration by clearing its + // initializer. + llvm::Constant *init = gv->getInitializer(); + gv->setInitializer(NULL); + + llvm::Type *type = gv->getType()->getElementType(); + Symbol *sym = + m->symbolTable->LookupVariable(gv->getName().str().c_str()); + Assert(sym != NULL); + + // Create a new global in the given model that matches the original + // global + llvm::GlobalVariable *newGlobal = + new llvm::GlobalVariable(**mdisp, type, gv->isConstant(), + llvm::GlobalValue::ExternalLinkage, + init, gv->getName()); + newGlobal->copyAttributesFrom(gv); + } + } +} #ifdef ISPC_NVPTX_ENABLED static std::string lCBEMangle(const std::string &S) @@ -3169,8 +3120,9 @@ Module::CompileAndOutput(const char *srcFile, for (int i = 0; i < Target::NUM_ISAS; ++i) targetMachines[i] = NULL; + llvm::Module *dispatchModule = NULL; + std::map exportedFunctions; - std::vector globals[Target::NUM_ISAS]; int errorCount = 0; // Handle creating a "generic" header file for multiple targets @@ -3221,8 +3173,7 @@ Module::CompileAndOutput(const char *srcFile, // just compiled, for use in generating the dispatch function // later. lGetExportedFunctions(m->symbolTable, exportedFunctions); - - lExtractAndRewriteGlobals(m->module, &globals[i]); + lExtractGlobals(m->module, &dispatchModule); if (outFileName != NULL) { std::string targetOutFileName; @@ -3298,10 +3249,7 @@ Module::CompileAndOutput(const char *srcFile, return 1; } - llvm::Module *dispatchModule = - lCreateDispatchModule(exportedFunctions); - - lAddExtractedGlobals(dispatchModule, globals); + lEmitDispatchModule(dispatchModule, exportedFunctions); if (outFileName != NULL) { if (outputType == Bitcode) From 50d84e0884f10e7b2c3c235f4322707180ad6308 Mon Sep 17 00:00:00 2001 From: Andrey Guskov Date: Thu, 21 May 2015 15:58:20 +0300 Subject: [PATCH 2/2] Re-added name/type checking for globals --- module.cpp | 92 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 63 insertions(+), 29 deletions(-) diff --git a/module.cpp b/module.cpp index 1487ddd3..7c81a2f2 100644 --- a/module.cpp +++ b/module.cpp @@ -2665,20 +2665,6 @@ lGetExportedFunctions(SymbolTable *symbolTable, } } - -struct RewriteGlobalInfo { - RewriteGlobalInfo(llvm::GlobalVariable *g = NULL, llvm::Constant *i = NULL, - SourcePos p = SourcePos()) { - gv = g; - init = i; - pos = p; - } - - llvm::GlobalVariable *gv; - llvm::Constant *init; - SourcePos pos; -}; - static llvm::FunctionType * lGetVaryingDispatchType(FunctionTargetVariants &funcs) { llvm::Type *ptrToInt8Ty = llvm::Type::getInt8PtrTy(*g->ctx); @@ -2925,25 +2911,48 @@ static void lEmitDispatchModule(llvm::Module *module, optPM.run(*module); } +// Determines if two types are compatible +static bool lCompatibleTypes(llvm::Type *Ty1, llvm::Type *Ty2) { + while (Ty1->getTypeID() == Ty1->getTypeID()) + switch (Ty1->getTypeID()) { + case llvm::ArrayType::ArrayTyID: + if (Ty1->getArrayNumElements() != + Ty2->getArrayNumElements()) + return false; + Ty1 = Ty1->getArrayElementType(); + Ty2 = Ty2->getArrayElementType(); + break; + + case llvm::ArrayType::PointerTyID: + Ty1 = Ty1->getPointerElementType(); + Ty2 = Ty2->getPointerElementType(); + break; + + case llvm::ArrayType::StructTyID: + return llvm::dyn_cast(Ty1)->isLayoutIdentical(llvm::dyn_cast(Ty2)); + + default: + // Pointers for compatible simple types are assumed equal + return Ty1 == Ty2; + } + return false; +} + // Grab all of the global value definitions from the module and change them // to be declarations; we'll emit a single definition of each global in the // final module used with the dispatch functions, so that we don't have // multiple definitions of them, one in each of the target-specific output // files. static void -lExtractGlobals(llvm::Module *module, llvm::Module **mdisp) { +lExtractOrCheckGlobals(llvm::Module *msrc, llvm::Module *mdst, bool check) { llvm::Module::global_iterator iter; - if (*mdisp) - return; - - *mdisp = lInitDispatchModule(); - for (iter = module->global_begin(); iter != module->global_end(); ++iter) { + for (iter = msrc->global_begin(); iter != msrc->global_end(); ++iter) { llvm::GlobalVariable *gv = iter; // Is it a global definition? if (gv->getLinkage() == llvm::GlobalValue::ExternalLinkage && gv->hasInitializer()) { - // Turn this into an 'extern 'declaration by clearing its + // Turn this into an 'extern' declaration by clearing its // initializer. llvm::Constant *init = gv->getInitializer(); gv->setInitializer(NULL); @@ -2953,13 +2962,32 @@ lExtractGlobals(llvm::Module *module, llvm::Module **mdisp) { m->symbolTable->LookupVariable(gv->getName().str().c_str()); Assert(sym != NULL); - // Create a new global in the given model that matches the original - // global - llvm::GlobalVariable *newGlobal = - new llvm::GlobalVariable(**mdisp, type, gv->isConstant(), - llvm::GlobalValue::ExternalLinkage, - init, gv->getName()); - newGlobal->copyAttributesFrom(gv); + // Check presence and compatibility for the current global + if (check) { + llvm::GlobalVariable *exist = + mdst->getGlobalVariable(gv->getName()); + Assert(exist != NULL); + + // It is possible that the types may not match: for + // example, this happens with varying globals if we + // compile to different vector widths + if (!lCompatibleTypes(exist->getType(), gv->getType())) { + Warning(sym->pos, "Mismatch in size/layout of global " + "variable \"%s\" with different targets. " + "Globals must not include \"varying\" types or arrays " + "with size based on programCount when compiling to " + "targets with differing vector widths.", + gv->getName().str().c_str()); + } + } + // Alternatively, create it anew and make it match the original + else { + llvm::GlobalVariable *newGlobal = + new llvm::GlobalVariable(*mdst, type, gv->isConstant(), + llvm::GlobalValue::ExternalLinkage, + init, gv->getName()); + newGlobal->copyAttributesFrom(gv); + } } } } @@ -3169,11 +3197,17 @@ Module::CompileAndOutput(const char *srcFile, m = new Module(srcFile); if (m->CompileFile() == 0) { + // Create the dispatch module, unless already created; + // in the latter case, just do the checking + bool check = (dispatchModule != NULL); + if (!check) + dispatchModule = lInitDispatchModule(); + lExtractOrCheckGlobals(m->module, dispatchModule, check); + // Grab pointers to the exported functions from the module we // just compiled, for use in generating the dispatch function // later. lGetExportedFunctions(m->symbolTable, exportedFunctions); - lExtractGlobals(m->module, &dispatchModule); if (outFileName != NULL) { std::string targetOutFileName;