<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 16, 2015 at 12:28 PM, Lang Hames <span dir="ltr"><<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Dave,<div><br><div>On your second point: Yes, it appears that this does need to be ported to later chapters.</div><div><br></div><div>As for the awkwardness of the code, there's some history that provides context for this. The Kaleidoscope tutorials were originally written for the legacy JIT and made use of several features that were only natively available on it. When the decision was made to deprecate the legacy JIT, Andy Kaylor wrote a series of blog posts about porting Kaleidoscope to MCJIT. He introduced this code to reconstruct the required features of the old JIT on top of MCJIT, but that involves some non-trivial work, which feels out-of-place in a tutorial that's aiming for simplicity.</div><div><br></div><div>There are a few ways we could proceed to improve Kaleidoscope. We could rewrite it to more neatly fit MCJIT, or we could try to tidy up the hacks to look less hack-y, but that would still leave non-trivial code in the tutorial.</div><div><br></div><div>Personally I'm inclined to revisit this once the Orc APIs are in-tree. The design of Orc was partially inspired by Andy's blog posts, and we could use it to remove all of the hacks from Kaleidoscope.</div></div></div></blockquote><div><br>Makes sense - thanks for the overview. (& happy to defer to your priorities/perspective here - don't feel strongly that it must be any different than what you've got in mind)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><br></div><div>In the mean time I'm very happy to review patches to fix the current implementations.</div><div><br></div><div><br></div><div>Cheers,</div><div>Lang.</div><div><div><br></div><div><br></div></div></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 16, 2015 at 11:54 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Fri, Jan 16, 2015 at 11:44 AM, Lang Hames <span dir="ltr"><<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: lhames<br>
Date: Fri Jan 16 13:44:46 2015<br>
New Revision: 226308<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=226308&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=226308&view=rev</a><br>
Log:<br>
[Kaleidoscope] Fix a bug in Chapter 4 of the Kaleidoscope tutorial where repeat<br>
calls to functions weren't evaluated correctly.<br>
<br>
Patch by Charlie Turner. Thanks Charlie!<br>
<br>
<br>
Modified:<br>
llvm/trunk/examples/Kaleidoscope/Chapter4/toy.cpp<br>
<br>
Modified: llvm/trunk/examples/Kaleidoscope/Chapter4/toy.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/examples/Kaleidoscope/Chapter4/toy.cpp?rev=226308&r1=226307&r2=226308&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/examples/Kaleidoscope/Chapter4/toy.cpp?rev=226308&r1=226307&r2=226308&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/examples/Kaleidoscope/Chapter4/toy.cpp (original)<br>
+++ llvm/trunk/examples/Kaleidoscope/Chapter4/toy.cpp Fri Jan 16 13:44:46 2015<br>
@@ -381,13 +381,253 @@ static PrototypeAST *ParseExtern() {<br>
}<br>
<br>
//===----------------------------------------------------------------------===//<br>
+// Quick and dirty hack<br>
+//===----------------------------------------------------------------------===//<br></blockquote></span><div><br>Seems like a fair bit of code to introduce to a relatively small program as a workaround/hack. But I haven't looked at it closely/understood the entire justification, etc.<br><br>I assume this needs to be ported into all the chapters beyond 4, too?<br> </div><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+// FIXME: Obviously we can do better than this<br>
+std::string GenerateUniqueName(const char *root)<br>
+{<br>
+ static int i = 0;<br>
+ char s[16];<br>
+ sprintf(s, "%s%d", root, i++);<br>
+ std::string S = s;<br>
+ return S;<br>
+}<br>
+<br>
+std::string MakeLegalFunctionName(std::string Name)<br>
+{<br>
+ std::string NewName;<br>
+ if (!Name.length())<br>
+ return GenerateUniqueName("anon_func_");<br>
+<br>
+ // Start with what we have<br>
+ NewName = Name;<br>
+<br>
+ // Look for a numberic first character<br>
+ if (NewName.find_first_of("0123456789") == 0) {<br>
+ NewName.insert(0, 1, 'n');<br>
+ }<br>
+<br>
+ // Replace illegal characters with their ASCII equivalent<br>
+ std::string legal_elements = "_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";<br>
+ size_t pos;<br>
+ while ((pos = NewName.find_first_not_of(legal_elements)) != std::string::npos) {<br>
+ char old_c = NewName.at(pos);<br>
+ char new_str[16];<br>
+ sprintf(new_str, "%d", (int)old_c);<br>
+ NewName = NewName.replace(pos, 1, new_str);<br>
+ }<br>
+<br>
+ return NewName;<br>
+}<br>
+<br>
+//===----------------------------------------------------------------------===//<br>
+// MCJIT helper class<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+class MCJITHelper<br>
+{<br>
+public:<br>
+ MCJITHelper(LLVMContext& C) : Context(C), OpenModule(NULL) {}<br>
+ ~MCJITHelper();<br>
+<br>
+ Function *getFunction(const std::string FnName);<br>
+ Module *getModuleForNewFunction();<br>
+ void *getPointerToFunction(Function* F);<br>
+ void *getSymbolAddress(const std::string &Name);<br>
+ void dump();<br>
+<br>
+private:<br>
+ typedef std::vector<Module*> ModuleVector;<br>
+ typedef std::vector<ExecutionEngine*> EngineVector;<br>
+<br>
+ LLVMContext &Context;<br>
+ Module *OpenModule;<br>
+ ModuleVector Modules;<br>
+ EngineVector Engines;<br>
+};<br>
+<br>
+class HelpingMemoryManager : public SectionMemoryManager<br>
+{<br>
+ HelpingMemoryManager(const HelpingMemoryManager&) LLVM_DELETED_FUNCTION;<br>
+ void operator=(const HelpingMemoryManager&) LLVM_DELETED_FUNCTION;<br>
+<br>
+public:<br>
+ HelpingMemoryManager(MCJITHelper *Helper) : MasterHelper(Helper) {}<br>
+ virtual ~HelpingMemoryManager() {}<br>
+<br>
+ /// This method returns the address of the specified symbol.<br>
+ /// Our implementation will attempt to find symbols in other<br>
+ /// modules associated with the MCJITHelper to cross link symbols<br>
+ /// from one generated module to another.<br>
+ virtual uint64_t getSymbolAddress(const std::string &Name) override;<br>
+private:<br>
+ MCJITHelper *MasterHelper;<br>
+};<br>
+<br>
+uint64_t HelpingMemoryManager::getSymbolAddress(const std::string &Name)<br>
+{<br>
+ uint64_t FnAddr = SectionMemoryManager::getSymbolAddress(Name);<br>
+ if (FnAddr)<br>
+ return FnAddr;<br>
+<br>
+ uint64_t HelperFun = (uint64_t) MasterHelper->getSymbolAddress(Name);<br>
+ if (!HelperFun)<br>
+ report_fatal_error("Program used extern function '" + Name +<br>
+ "' which could not be resolved!");<br>
+<br>
+ return HelperFun;<br>
+}<br>
+<br>
+MCJITHelper::~MCJITHelper()<br>
+{<br>
+ if (OpenModule)<br>
+ delete OpenModule;<br>
+ EngineVector::iterator begin = Engines.begin();<br>
+ EngineVector::iterator end = Engines.end();<br>
+ EngineVector::iterator it;<br>
+ for (it = begin; it != end; ++it)<br>
+ delete *it;<br>
+}<br>
+<br>
+Function *MCJITHelper::getFunction(const std::string FnName) {<br>
+ ModuleVector::iterator begin = Modules.begin();<br>
+ ModuleVector::iterator end = Modules.end();<br>
+ ModuleVector::iterator it;<br>
+ for (it = begin; it != end; ++it) {<br>
+ Function *F = (*it)->getFunction(FnName);<br>
+ if (F) {<br>
+ if (*it == OpenModule)<br>
+ return F;<br>
+<br>
+ assert(OpenModule != NULL);<br>
+<br>
+ // This function is in a module that has already been JITed.<br>
+ // We need to generate a new prototype for external linkage.<br>
+ Function *PF = OpenModule->getFunction(FnName);<br>
+ if (PF && !PF->empty()) {<br>
+ ErrorF("redefinition of function across modules");<br>
+ return 0;<br>
+ }<br>
+<br>
+ // If we don't have a prototype yet, create one.<br>
+ if (!PF)<br>
+ PF = Function::Create(F->getFunctionType(),<br>
+ Function::ExternalLinkage,<br>
+ FnName,<br>
+ OpenModule);<br>
+ return PF;<br>
+ }<br>
+ }<br>
+ return NULL;<br>
+}<br>
+<br>
+Module *MCJITHelper::getModuleForNewFunction() {<br>
+ // If we have a Module that hasn't been JITed, use that.<br>
+ if (OpenModule)<br>
+ return OpenModule;<br>
+<br>
+ // Otherwise create a new Module.<br>
+ std::string ModName = GenerateUniqueName("mcjit_module_");<br>
+ Module *M = new Module(ModName, Context);<br>
+ Modules.push_back(M);<br>
+ OpenModule = M;<br>
+ return M;<br>
+}<br>
+<br>
+void *MCJITHelper::getPointerToFunction(Function* F) {<br>
+ // See if an existing instance of MCJIT has this function.<br>
+ EngineVector::iterator begin = Engines.begin();<br>
+ EngineVector::iterator end = Engines.end();<br>
+ EngineVector::iterator it;<br>
+ for (it = begin; it != end; ++it) {<br>
+ void *P = (*it)->getPointerToFunction(F);<br>
+ if (P)<br>
+ return P;<br>
+ }<br>
+<br>
+ // If we didn't find the function, see if we can generate it.<br>
+ if (OpenModule) {<br>
+ std::string ErrStr;<br>
+ ExecutionEngine *NewEngine = EngineBuilder(std::unique_ptr<Module>(OpenModule))<br>
+ .setErrorStr(&ErrStr)<br>
+ .setMCJITMemoryManager(std::unique_ptr<HelpingMemoryManager>(new HelpingMemoryManager(this)))<br>
+ .create();<br>
+ if (!NewEngine) {<br>
+ fprintf(stderr, "Could not create ExecutionEngine: %s\n", ErrStr.c_str());<br>
+ exit(1);<br>
+ }<br>
+<br>
+ // Create a function pass manager for this engine<br>
+ FunctionPassManager *FPM = new FunctionPassManager(OpenModule);<br>
+<br>
+ // Set up the optimizer pipeline. Start with registering info about how the<br>
+ // target lays out data structures.<br>
+ OpenModule->setDataLayout(NewEngine->getDataLayout());<br>
+ FPM->add(new DataLayoutPass());<br>
+ // Provide basic AliasAnalysis support for GVN.<br>
+ FPM->add(createBasicAliasAnalysisPass());<br>
+ // Promote allocas to registers.<br>
+ FPM->add(createPromoteMemoryToRegisterPass());<br>
+ // Do simple "peephole" optimizations and bit-twiddling optzns.<br>
+ FPM->add(createInstructionCombiningPass());<br>
+ // Reassociate expressions.<br>
+ FPM->add(createReassociatePass());<br>
+ // Eliminate Common SubExpressions.<br>
+ FPM->add(createGVNPass());<br>
+ // Simplify the control flow graph (deleting unreachable blocks, etc).<br>
+ FPM->add(createCFGSimplificationPass());<br>
+ FPM->doInitialization();<br>
+<br>
+ // For each function in the module<br>
+ Module::iterator it;<br>
+ Module::iterator end = OpenModule->end();<br>
+ for (it = OpenModule->begin(); it != end; ++it) {<br>
+ // Run the FPM on this function<br>
+ FPM->run(*it);<br>
+ }<br>
+<br>
+ // We don't need this anymore<br>
+ delete FPM;<br>
+<br>
+ OpenModule = NULL;<br>
+ Engines.push_back(NewEngine);<br>
+ NewEngine->finalizeObject();<br>
+ return NewEngine->getPointerToFunction(F);<br>
+ }<br>
+ return NULL;<br>
+}<br>
+<br>
+void *MCJITHelper::getSymbolAddress(const std::string &Name)<br>
+{<br>
+ // Look for the symbol in each of our execution engines.<br>
+ EngineVector::iterator begin = Engines.begin();<br>
+ EngineVector::iterator end = Engines.end();<br>
+ EngineVector::iterator it;<br>
+ for (it = begin; it != end; ++it) {<br>
+ uint64_t FAddr = (*it)->getFunctionAddress(Name);<br>
+ if (FAddr) {<br>
+ return (void *)FAddr;<br>
+ }<br>
+ }<br>
+ return NULL;<br>
+}<br>
+<br>
+void MCJITHelper::dump()<br>
+{<br>
+ ModuleVector::iterator begin = Modules.begin();<br>
+ ModuleVector::iterator end = Modules.end();<br>
+ ModuleVector::iterator it;<br>
+ for (it = begin; it != end; ++it)<br>
+ (*it)->dump();<br>
+}<br>
+//===----------------------------------------------------------------------===//<br>
// Code Generation<br>
//===----------------------------------------------------------------------===//<br>
<br>
-static Module *TheModule;<br>
+static MCJITHelper *JITHelper;<br>
static IRBuilder<> Builder(getGlobalContext());<br>
static std::map<std::string, Value *> NamedValues;<br>
-static FunctionPassManager *TheFPM;<br>
<br>
Value *ErrorV(const char *Str) {<br>
Error(Str);<br>
@@ -429,7 +669,7 @@ Value *BinaryExprAST::Codegen() {<br>
<br>
Value *CallExprAST::Codegen() {<br>
// Look up the name in the global module table.<br>
- Function *CalleeF = TheModule->getFunction(Callee);<br>
+ Function *CalleeF = JITHelper->getFunction(Callee);<br>
if (CalleeF == 0)<br>
return ErrorV("Unknown function referenced");<br>
<br>
@@ -454,16 +694,19 @@ Function *PrototypeAST::Codegen() {<br>
FunctionType *FT =<br>
FunctionType::get(Type::getDoubleTy(getGlobalContext()), Doubles, false);<br>
<br>
+ std::string FnName = MakeLegalFunctionName(Name);<br>
+<br>
+ Module *M = JITHelper->getModuleForNewFunction();<br>
+<br>
Function *F =<br>
- Function::Create(FT, Function::ExternalLinkage, Name, TheModule);<br>
+ Function::Create(FT, Function::ExternalLinkage, FnName, M);<br>
<br>
// If F conflicted, there was already something named 'Name'. If it has a<br>
// body, don't allow redefinition or reextern.<br>
- if (F->getName() != Name) {<br>
+ if (F->getName() != FnName) {<br>
// Delete the one we just made and get the existing one.<br>
F->eraseFromParent();<br>
- F = TheModule->getFunction(Name);<br>
-<br>
+ F = JITHelper->getFunction(Name);<br>
// If F already has a body, reject this.<br>
if (!F->empty()) {<br>
ErrorF("redefinition of function");<br>
@@ -508,9 +751,6 @@ Function *FunctionAST::Codegen() {<br>
// Validate the generated code, checking for consistency.<br>
verifyFunction(*TheFunction);<br>
<br>
- // Optimize the function.<br>
- TheFPM->run(*TheFunction);<br>
-<br>
return TheFunction;<br>
}<br>
<br>
@@ -523,8 +763,6 @@ Function *FunctionAST::Codegen() {<br>
// Top-Level parsing and JIT Driver<br>
//===----------------------------------------------------------------------===//<br>
<br>
-static ExecutionEngine *TheExecutionEngine;<br>
-<br>
static void HandleDefinition() {<br>
if (FunctionAST *F = ParseDefinition()) {<br>
if (Function *LF = F->Codegen()) {<br>
@@ -553,9 +791,8 @@ static void HandleTopLevelExpression() {<br>
// Evaluate a top-level expression into an anonymous function.<br>
if (FunctionAST *F = ParseTopLevelExpr()) {<br>
if (Function *LF = F->Codegen()) {<br>
- TheExecutionEngine->finalizeObject();<br>
// JIT the function, returning a function pointer.<br>
- void *FPtr = TheExecutionEngine->getPointerToFunction(LF);<br>
+ void *FPtr = JITHelper->getPointerToFunction(LF);<br>
<br>
// Cast it to the right type (takes no arguments, returns a double) so we<br>
// can call it as a native function.<br>
@@ -610,6 +847,7 @@ int main() {<br>
InitializeNativeTargetAsmPrinter();<br>
InitializeNativeTargetAsmParser();<br>
LLVMContext &Context = getGlobalContext();<br>
+ JITHelper = new MCJITHelper(Context);<br>
<br>
// Install standard binary operators.<br>
// 1 is lowest precedence.<br>
@@ -622,51 +860,11 @@ int main() {<br>
fprintf(stderr, "ready> ");<br>
getNextToken();<br>
<br>
- // Make the module, which holds all the code.<br>
- std::unique_ptr<Module> Owner = make_unique<Module>("my cool jit", Context);<br>
- TheModule = Owner.get();<br>
-<br>
- // Create the JIT. This takes ownership of the module.<br>
- std::string ErrStr;<br>
- TheExecutionEngine =<br>
- EngineBuilder(std::move(Owner))<br>
- .setErrorStr(&ErrStr)<br>
- .setMCJITMemoryManager(llvm::make_unique<SectionMemoryManager>())<br>
- .create();<br>
- if (!TheExecutionEngine) {<br>
- fprintf(stderr, "Could not create ExecutionEngine: %s\n", ErrStr.c_str());<br>
- exit(1);<br>
- }<br>
-<br>
- FunctionPassManager OurFPM(TheModule);<br>
-<br>
- // Set up the optimizer pipeline. Start with registering info about how the<br>
- // target lays out data structures.<br>
- TheModule->setDataLayout(TheExecutionEngine->getDataLayout());<br>
- OurFPM.add(new DataLayoutPass());<br>
- // Provide basic AliasAnalysis support for GVN.<br>
- OurFPM.add(createBasicAliasAnalysisPass());<br>
- // Do simple "peephole" optimizations and bit-twiddling optzns.<br>
- OurFPM.add(createInstructionCombiningPass());<br>
- // Reassociate expressions.<br>
- OurFPM.add(createReassociatePass());<br>
- // Eliminate Common SubExpressions.<br>
- OurFPM.add(createGVNPass());<br>
- // Simplify the control flow graph (deleting unreachable blocks, etc).<br>
- OurFPM.add(createCFGSimplificationPass());<br>
-<br>
- OurFPM.doInitialization();<br>
-<br>
- // Set the global so the code gen can use this.<br>
- TheFPM = &OurFPM;<br>
-<br>
// Run the main "interpreter loop" now.<br>
MainLoop();<br>
<br>
- TheFPM = 0;<br>
-<br>
// Print out all of the generated code.<br>
- TheModule->dump();<br>
+ JITHelper->dump();<br>
<br>
return 0;<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>