This change makes the ModuleTranslation threadsafe by locking on the
LLVMContext. Furthermore, we now clone the llvm module into a new
context when compiling to PTX similar to what the OrcJit does.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h | ||
---|---|---|
29 | Is this needed? |
mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h | ||
---|---|---|
29 | No, a leftover from using std::mutex in an earlier version. Thanks! |
I don't suppose there is a way to make this method only visible to ModuleTranslation...
mlir/lib/Conversion/GPUToCUDA/ConvertKernelFuncToCubin.cpp | ||
---|---|---|
114 | Why don't you use the high level API the same way this is done in the ExecutionEngine? // TODO(zinenko): Reevaluate model of ownership of LLVMContext in LLVMDialect. SmallVector<char, 1> buffer; { llvm::raw_svector_ostream os(buffer); WriteBitcodeToFile(*llvmModule, os); } llvm::MemoryBufferRef bufferRef(StringRef(buffer.data(), buffer.size()), "cloned module buffer"); auto expectedModule = parseBitcodeFile(bufferRef, *ctx); if (!expectedModule) return expectedModule.takeError(); std::unique_ptr<Module> deserModule = std::move(*expectedModule); auto dataLayout = deserModule->getDataLayout(); I'd also like a TODO also here for Alex to actually fix: the fact that we have a LLVMContext tied to the LLVM dialect is really something we need to fix. | |
114 | (It may be even worth extracting this logic in a helper by the way) |
mlir/lib/Conversion/GPUToCUDA/ConvertKernelFuncToCubin.cpp | ||
---|---|---|
114 |
Yeah, this is the next big thing on my todo list. Let's see how many discussion we can have in parallel :) |
This is causing TSAN failures. Looks like ConvertKernelFuncToCubin isn't thread safe. More specifically the call to LLVMInitializeNVPTXTargetInfo.
mlir/lib/Conversion/GPUToCUDA/ConvertKernelFuncToCubin.cpp:62:5
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | ||
---|---|---|
500 | Can we just lock once at beginning? |
Is this needed?