Page MenuHomePhabricator

[mlir] Link mlir_runner_utils statically into cuda/rocm-runtime-wrappers.
ClosedPublic

Authored by csigg on Jan 11 2021, 4:04 AM.

Details

Summary

The runtime-wrappers depend on LLVMSupport, pulling in static initialization code (e.g. command line arguments). Dynamically loading multiple such libraries results in ODR violoations.

So far this has not been an issue, but in D94421, I would like to load both the async-runtime and the cuda-runtime-wrappers as part of a cuda-runner integration test. When doing this, code that asserts that an option category is only registered once fails (note that I've only experienced this in Google's bazel where the async-runtime depends on LLVMSupport, but a similar issue would happen in cmake if more than one runtime-wrapper starts to depend on LLVMSupport).

The underlying issue is that we have a mix of static and dynamic linking. If all dependencies were loaded as shared objects (i.e. if LLVMSupport was linked dynamically to the runtime wrappers), each dependency would only get loaded once. However, linking dependencies dynamically would require special attention to paths (one could dynamically load the dependencies first given explicit paths). The simpler approach seems to be to link all dependencies statically into a single shared object.

This change basically applies the same logic that we have in the c_runner_utils: we have a shared object target that can be loaded dynamically, and we have a static library target that can be linked to other runtime-wrapper shared object targets.

Diff Detail

Event Timeline

csigg created this revision.Jan 11 2021, 4:04 AM
csigg requested review of this revision.Jan 11 2021, 4:04 AM
csigg retitled this revision from Link mlir_runner_utils statically into cuda/rocm-runtime-wrappers. to [mlir] Link mlir_runner_utils statically into cuda/rocm-runtime-wrappers..Jan 11 2021, 4:54 AM

Can you add a description of the motivation to the commit message?

csigg edited the summary of this revision. (Show Details)Jan 11 2021, 12:24 PM

Thanks for adding the description: I still don't quite get how making these static is preventing an ODR issues?

csigg edited the summary of this revision. (Show Details)Jan 12 2021, 1:48 AM

If the runners depends on libSupport and we link them statically into the runtime wrapper that are dynamically loaded, aren't we having two copies of libSupport? One in the statically linked runner executable and one in the dynamically loaded shared objects?

I don't really understand how this works, but my guess is that the JITDylib loaded into the ExecutionEngine's module do not share any symbols with the main process.

I asked this in the other CL too, but I was just wondering if these tests are better candidates for mlir/integration_test rather than mlir/test?

herhut accepted this revision.Jan 19 2021, 1:24 AM

This still seems a bit like a hack to me but it is better than twiddling with runtime paths.

This revision is now accepted and ready to land.Jan 19 2021, 1:24 AM