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

Prepare to use LLVM in a threadsafe fashion #6570

Closed

Conversation

alexcrichton
Copy link
Member

Work towards #6511.

To isolate LLVM among threads, the main need to to use different LLVMContextRef structures. This is easy enough to do by storing a task-local context while translation is happening.

There aren't that many points in the LLVM api which actually need a context, so this wasn't too bad of a change in that respect. All of the bindings to functions which implicitly take a global context have been removed to prevent them from being used again.

Furthermore, this fixes memory leaks in the JIT by properly destroying the ExecutionEngine created after the JIT code finishes running.

One consequence of these commits is that LLVM needs to be re-built because the disable-threads and disable-pthreads options were previously passed to LLVM. By default these are no longer passed in.

@brson
Copy link
Contributor

brson commented May 18, 2013

\o/

@alexcrichton
Copy link
Member Author

@brson, this is going to need LLVM on the buildbots to be re-built, is there a special flag I should set somewhere or some file I should modify to trigger this?

@sanxiyn
Copy link
Member

sanxiyn commented May 20, 2013

CC #6367.

@z0w0
Copy link
Contributor

z0w0 commented May 21, 2013

Awesome work re: fixing rusti and JIT issues.

@brson
Copy link
Contributor

brson commented May 21, 2013

@alexcrichton set llvm-clean as property name 1 with a value of 1 to try to get the bots to rebuild llvm

@alexcrichton
Copy link
Member Author

So today I pushed this to try, and linux passed, but windows has problems after brson's patch.

cmr on irc pointed out that the file in question for llvm may not be getting included correctly? I took a glance at LLVM, and it didn't look like it was possible for it to not be included. It also appeared that the same guard for these functions is in both the header and source file.

I'm not exactly sure how to debug this. I don't think this could be a problem with llvm not having cleaned itself entirely or something like that. There's also an ominous warning printed out during configuration of llvm: LLVM will be built thread-unsafe because atomic builtins are missing, although when I looked into that, it didn't appear as if it would cause this kind of build failure.

I found some previous issue about where --disable-threads fixes some issues on mingw/msys. I know very little about building on windows. @brson, do you have any ideas about perhaps whether this is just a build problem with the currently LLVM? Otherwise, if you have more info about the windows build setup, I can look some more into that.

@brson
Copy link
Contributor

brson commented May 22, 2013

It looks like an LLVM bug. If you want to dig into it instructions for building on windows are on the wiki, but I'll poke at it on the bots tomorrow and see if there's an obvious fix.

@alexcrichton
Copy link
Member Author

Hmm... I investigated this a bit more, and I think that the clean-llvm flag isn't actually cleaning everything. I just re-ran the build again to make sure, but the relevant file is located in lib/Support, but the logs say:

make[2]: Entering directory `/c/bot/slave/try-win/build/obj/llvm/i686-pc-mingw32/lib/Support'
make[2]: Nothing to be done for `all'.
make[2]: Leaving directory `/c/bot/slave/try-win/build/obj/llvm/i686-pc-mingw32/lib/Support'

So this appears that the relevant file just isn't being recompiled. If there's another flag to force removal of everything, I could try that as well. If the llvm build directory were cleaned that may also help.

@graydon
Copy link
Contributor

graydon commented May 22, 2013

property: wipe, value: 1 will throw out the whole build dir and re-clone everything.

@alexcrichton
Copy link
Member Author

This is going to be blocked on #6713 to get LLVM building with --enable-threads

@alexcrichton
Copy link
Member Author

On second thought, it looks like it's going to take awhile to upgrade LLVM/get it building with --enable-threads, so I'll go ahead and land these changes (which work just fine in the meantime).

@alexcrichton
Copy link
Member Author

r? @brson

This allows parallel usage of the rustc library
Also stop leaking the ExecutionEngine created for jit code by forcibly disposing
of it after the JIT code has finished executing
@alexcrichton
Copy link
Member Author

Going to reopen a new request, this one's getting quite old and looks misleading.

bors added a commit that referenced this pull request Jun 11, 2013
…=graydon

This is a reopening of #6570, and almost fixes #6511.

Note that this doesn't actually enable building a threadsafe LLVM, because that will require an LLVM rebuild which will be bundled with the upgrades in #6713.

What this does do, however, is removes all thread-unsafe usage of LLVM from the compiler.
bors added a commit that referenced this pull request Jun 11, 2013
…=graydon

This is a reopening of #6570, and almost fixes #6511.

Note that this doesn't actually enable building a threadsafe LLVM, because that will require an LLVM rebuild which will be bundled with the upgrades in #6713.

What this does do, however, is removes all thread-unsafe usage of LLVM from the compiler.
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.

5 participants