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

Remove precompiled=no on windows. #15956

Closed
wants to merge 1 commit into from
Closed

Conversation

dhoegh
Copy link
Contributor

@dhoegh dhoegh commented Apr 20, 2016

As LLVM has been upgraded, is there any reason for not removing precompiled=no on windows? It reduces startup time and the test seem to pass on 64bit.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Apr 20, 2016

yes, we'll eventually do this. but there's a couple outstanding (upstream) bugs to deal with first (e.g. llvm3.5 didn't fix them as hoped). here's the quick list, if you're interesting in helping tackle them i'm happy to answer questions:

  • the chief bug is patch llvm win64 dwarf4 relocations #15859
  • we also need a custom SectionMemoryManager (that allocates all sections consecutively and near each other) and to patch llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFX86_64.h to use that image base from the memory manager (instead of hardcoding 0 to handle COFF::IMAGE_REL_AMD64_ADDR32NB) and a few other steps to hookup the .xdata section (basically, fix https://llvm.org/bugs/show_bug.cgi?id=24233). I think we might need to disable FORCE_ELF too (hopefully that doesn't break anything else, but I think we should be OK on that front).
  • then we also need to set our llvm::Function unwind personality to call _seh_exception_handler (defined in libjulia).

@vtjnash vtjnash closed this Apr 20, 2016
@tkelman
Copy link
Contributor

tkelman commented Apr 20, 2016

if it requires SEH, I'm guessing it'll require picking one of slower startup or worse backtraces on 32 bit?

@vtjnash
Copy link
Sponsor Member

vtjnash commented Apr 20, 2016

win32 needs the first one, but not the others

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants