Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix LLVM 3.9 build #15749

Merged
merged 1 commit into from
Apr 2, 2016
Merged

Fix LLVM 3.9 build #15749

merged 1 commit into from
Apr 2, 2016

Conversation

yuyichao
Copy link
Contributor

@yuyichao yuyichao commented Apr 2, 2016

Force LLVM to emit the code before we look for function addresses.

We call findSymbol right after we call addModule so this shouldn't make the JIT more eager than what it is now.

Fixes #15712

Force LLVM to emit the code before we look for function addresses.
@yuyichao yuyichao added the compiler:codegen Generation of LLVM IR and native code label Apr 2, 2016
@vtjnash
Copy link
Member

vtjnash commented Apr 2, 2016

does this mean the CompileLayer had become lazy? our goal was definitely for this API to be eager since we do the lazy work outside of the JIT and only add it here when we need the fptr

@yuyichao
Copy link
Contributor Author

yuyichao commented Apr 2, 2016

That seems to be the case. I'm not sure when/how it happened since we only had this problem when using switching to our own symbol table.

@vtjnash
Copy link
Member

vtjnash commented Apr 2, 2016

i was assuming LocalSymbolTable would get populated sufficiently eagerly. but it seems it now first requires a lookup up for a symbol that needed be found in the object file for llvm to finalize the linking and relocations and call our finalization callback

@yuyichao
Copy link
Contributor Author

yuyichao commented Apr 2, 2016

I think so. But actually doing a symbol lookup may fall back to the O(n) scaling....

@vtjnash
Copy link
Member

vtjnash commented Apr 2, 2016

yes, this is better.

i suppose another option we could contribute upstream is to store a StringMap<ObjectLinkingLayer::ObjSetHandleT> created by ObjectLinkingLayer::addObjectSet (from walking the object file symbols). we could also mimic that by walking the module symbols before addModuleSet and create the same map using the return value from that call.

@yuyichao
Copy link
Contributor Author

yuyichao commented Apr 2, 2016

I imagine that can be useful if we want to rely on the lazyness of the OrcJIT itself at some point. IMHO doing that right now will just add one more lookup overhead since what we want is directly emitting the object file anyway...

@yuyichao
Copy link
Contributor Author

yuyichao commented Apr 2, 2016

Although in any case, I think we should report the O(n) scaling of symbol lookup assuming it haven't been fixed yet.

@yuyichao yuyichao merged commit 1eb8455 into master Apr 2, 2016
@yuyichao yuyichao deleted the yyc/llvm39 branch April 2, 2016 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants