[PATCH] D51052: [WIP] LLJITTest
Stefan Gränitz via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 22 09:46:02 PDT 2018
sgraenitz added a comment.
Maybe it makes sense in oder to illustrate the usage of your API, to give people a very simple example (like `UnthreadedSingleDylib`) and extended it step by step to more advanced ones (like `ThreadedMultiDylib`).
I don't see how to avoid the `PassManagerRunMutex` in `IRCompileLayer2::emit()`:
- I tried to add mutexes in MCAssembler, TargetLoweringObjectFile, AtomicExpandPass, etc., but it got very complicated very quickly. Everything goes through `PassManager::run()`, but a mutex there would affect a lot of other code. `IRCompileLayer2` appeared to be a good point.
- I always thought that support for multithreaded codegen is implicitly given when using different `LLVMContext` instances. It doesn't look like that here. Maybe there's more requirements to it?
================
Comment at: unittests/ExecutionEngine/Orc/LLJITTest.cpp:55
}
);
----------------
Changed example functions: foo and bar are disjunct now, buz is the one that depends on both of them.
================
Comment at: unittests/ExecutionEngine/Orc/LLJITTest.cpp:161
assert(Ms.size() < MaxMaterializerThreads &&
"More slots used than preallocated");
----------------
The thread that runs `DispatchMaterialization` can't be blocked, right? So it must spawn a new thread, which must be joined back to this thread later. How do we know the correct one? Track it in `MaterializationUnit` and join in destructor? Also see my below comments.
================
Comment at: unittests/ExecutionEngine/Orc/LLJITTest.cpp:184
JITEvaluatedSymbol FooSym = cantFail(Jit->lookup("foo"));
EXPECT_EQ(1, Exec(FooSym)); // Automatically waits for materializer.
});
----------------
Can't join the `DispatchMaterialization` thread here, because I don't know whether it's `M[0]` or `M[1]`.
================
Comment at: unittests/ExecutionEngine/Orc/LLJITTest.cpp:190
EXPECT_EQ(1, Exec(BarSym)); // Automatically waits for materializer.
});
----------------
Concurrent lookup for `foo` and `bar` works due to the mutex in `IRCompileLayer2::emit()`.
================
Comment at: unittests/ExecutionEngine/Orc/LLJITTest.cpp:194
JoinAll(Ts);
- JoinAll(Ms);
+ JoinAll(Ms); // This should happen in the above runner threads.
----------------
This randomly causes failures. I think threads should always be joined in the thread that they were forked from. Not simple, even with a fixed-size thread pool.
================
Comment at: unittests/ExecutionEngine/Orc/LLJITTest.cpp:269
- // FIXME: Lookup for "buz" can't run in parallel.
- JoinAll(Ts);
- JoinAll(Ms);
-
- // Allow buz to resolve foo and bar from the other dylibs.
- Extra2.addToSearchOrder(Main);
- Extra2.addToSearchOrder(Extra1);
-
+ // Lookup for "buz" works in parallel here even without a barrier.
Ts.emplace_back([&]() {
----------------
No barrier with `JoinAll` necessary here. Maybe because `foo` and `bar` are in other dylibs?
Repository:
rL LLVM
https://reviews.llvm.org/D51052
More information about the llvm-commits
mailing list