I just located a concurrency error in my LLVM patch to add locks to the JIT. I did not add any locks to the JITResolver class in JITEmitter.cpp, which clearly is a problem since This particular issue causes an assertion failure on occasion when running a task that spawns threads on a parallel machine. Any recommendations about if I am doing something that seems horribly wrong are welcome. I'll update my patch that has been sitting in Bugzilla for a while as soon as I come up with a good fix. My understanding of how code is JITed: 1. The program calls a stub. 2. The stub does some magic to call into the JIT and pass the stub address as a parameter. 3. Eventually, it ends up in JITResolver::JITCompilerFn which looks up the function. 4. This ends up calling into the JIT eventually.>From what I understand, the JIT *never* calls into the JITResolver.However, the JITResolver calls into the JIT. Hence, it is okay to have two lock objects, one for the JIT and one for the JITResolver. However, these two classes work very closely together, so I think there should only be a single lock object. Does this seem reasonable? Additionally, it only seems to make sense that there is a single JITResolver instance. Thus, I am going to force code to hold the lock before calling "getJITResolver." Does that seem reasonable? Thanks, Evan Jones
Chris Lattner
2005-Apr-21 16:12 UTC
[LLVMdev] Control Flow and Locks when JITing Functions
On Thu, 21 Apr 2005, Evan Jones wrote:> I just located a concurrency error in my LLVM patch to add locks to the > JIT. I did not add any locks to the JITResolver class in JITEmitter.cpp, > which clearly is a problem since This particular issue causes an > assertion failure on occasion when running a task that spawns threads on > a parallel machine. Any recommendations about if I am doing something > that seems horribly wrong are welcome. I'll update my patch that has > been sitting in Bugzilla for a while as soon as I come up with a good > fix.Ok. Thanks, for forgot about that patch :(> My understanding of how code is JITed: > > 1. The program calls a stub. > 2. The stub does some magic to call into the JIT and pass the stub > address as a parameter. > 3. Eventually, it ends up in JITResolver::JITCompilerFn which looks up > the function. > 4. This ends up calling into the JIT eventually.yes.> From what I understand, the JIT *never* calls into the JITResolver. > However, the JITResolver calls into the JIT. Hence, it is okay to have > two lock objects, one for the JIT and one for the JITResolver. However, > these two classes work very closely together, so I think there should > only be a single lock object. Does this seem reasonable?I think it would be more straight-forward to have a single lock that the jit resolver can aquire. Makes sense to me at least :)> Additionally, it only seems to make sense that there is a single > JITResolver instance. Thus, I am going to force code to hold the lock > before calling "getJITResolver." Does that seem reasonable?Yes, there can be only one JITResolver. -Chris -- http://nondot.org/sabre/ http://llvm.cs.uiuc.edu/
On Apr 21, 2005, at 12:12, Chris Lattner wrote:> Ok. Thanks, for forgot about that patch :(No problem, since I still don't know how to integrate it cleanly into the build system. Platform dependencies are very annoying.> I think it would be more straight-forward to have a single lock that > the jit resolver can aquire. Makes sense to me at leastThis is actually what I ended up doing. So now, the lock in "ExecutionEngine" is public, and everything involved in the JIT locks it. However, it turns out that the issue isn't quite what I thought it was. I thought it was a concurrent modification issue, where two threads were trying to update the FunctionStubMap at the same time. Well, it sort of is, but for a different reason. I'm not sure how I should fix this, so suggestions are welcome. My application is spawning multiple threads that are calling in to the same function. They all call the stub, and they all end up calling into JITResolve::JITCompilerFn. One of them gets the lock and compiles the function. Then, when it releases the lock, another thread tries to continue and immediately gets this exception: lli: JITEmitter.cpp:236: static void* <unnamed>::JITResolver::JITCompilerFn(void*): Assertion `I != JR.state.getStubToFunctionMap(locked).begin() && "This is not a known stub!"' failed. The problem is that after a function is compiled, the information about the Stub -> Function mapping is erased because it is no longer needed. The stub has been rewritten to point to the native function. However, in this case, these threads have already called the stub. So the fix is one of: A) Hang on to stub -> function mapping indefinitely. (I'm going to try and implement this to get my project that is due tomorrow working) B) Have the threads wait on a condition in that particular spot. When another thread finishes rewriting the stub, it wakes the threads up. These threads then go and do some low level hack, like calling the stub again, and immediately call the compiled function. This is probably a better solution, but it certainly is more complex. Any other ideas? Evan Jones