some codegen tests & fixes #15632
some codegen tests & fixes #15632
Conversation
| private: | ||
| void NotifyGDB(OwningBinary<ObjectFile> &DebugObj) | ||
| { | ||
| const char *Buffer = DebugObj.getBinary()->getMemoryBufferRef().getBufferStart(); |
|
LGTM, modulo @tkelman's style complaint. |
without this, code_llvm was showing un-optimized functions which could be very misleading since it differed from the code that actually executed (this was a recent regression caused by jn/moduledecoalescing)
CompileLayer.findSymbol is O(n) in the number of modules that have been emitted but we can pre-compute the result of findSymbolIn when notified that an object has been emitted and store it in one hash table for the JIT fix #15619
closes #14113 (fixed previously)
|
Since this contains a fix for a very annoying issue, I'm merging. Style issues can be fixed up afterwards. |
|
i fixed the style issue. just forgot to check CI, so you're all set :) |
| // note: calling getAddress here eagerly finalizes H | ||
| // as an alternative, we could store the JITSymbol instead | ||
| // (which would present a lazy-initializer functor interface instead) | ||
| JIT.LocalSymbolTable[*Name] = (void*)(uintptr_t)Sym.getAddress(); |
There was a problem hiding this comment.
IIUC this is the only place we get the symbol address (eagerly) now? It seems that on llvm 39 this is not called before we get to findSymbol (maybe because llvm gets lazier?) and therefore findSymbol returns NULL.
There was a problem hiding this comment.
i assumed this would be late enough: http://llvm.org/docs/doxygen/html/ObjectLinkingLayer_8h_source.html#l00259
There was a problem hiding this comment.
That line is in another lambda?
There was a problem hiding this comment.
indeed. i think there are a few more templated lambdas in that call chain too.
minor fixes and improvements for codegen, and some tests