[Mesa-dev] radv vs vulkan loader

[Emil Velikov]

On 5 October 2016 at 23:12, Gustaw Smolarczyk <wielkiegie at gmail.com> wrote:
> 2016-10-06 0:05 GMT+02:00 Emil Velikov <emil.l.velikov at gmail.com>:
>> On 5 October 2016 at 21:45, Gustaw Smolarczyk <wielkiegie at gmail.com> wrote:
>>> Hello,
>>>
>>> I have encountered a following problem while trying to use radv
>>> through LunarG's vulkan loader.
>>>
>>> It seems that the loader dlopens() the ICD library twice. First, it
>>> looks up "vk_icdNegotiateLoaderICDInterfaceVersion" symbol, which
>>> seems to be the new mechanism used to determine the version of ICD
>>> interface. Since radv doesn't provide it, it fall backs to the older
>>> scheme. Unfortunately, it seems that the loader first unloads the ICD
>>> and then loads it again. That causes issues in LLVM libraries' command
>>> line registration which happens while initializing global objects with
>>> constructors. To be more specific, "asan-instrument-assembly"
>>> registered in libLLVMX86AsmPrinter.so registers for the second time
>>> and causes the following message:
>>>
>>>
>>> $ vulkaninfo
>>> ===========
>>> VULKAN INFO
>>> ===========
>>>
>>> Vulkan API Version: 1.0.26
>>>
>>>
>>> : CommandLine Error: Option 'asan-instrument-assembly' registered more
>>> than once!
>>> LLVM ERROR: inconsistency in registered CommandLine options
>>>
>>> I have "fixed" the problem by manually removing
>>> libLLVMX86AsmPrinter.so from the list of llvm dependencies to radv. It
>>> seems that it was the only library registering any command line
>>> option.
>>>
>>> I am not sure who is to blame for this situation. It's possible that
>>> advertising the new ICD entry point would fix it. LLVM is really
>>> fragile about its command line registration framework. Last, the
>>> vulkan loader could try not to unnecessarily dlclose and dlopen the
>>> ICD library.
>>>
>> From a quick read it sounds like something that should be fixed in
>> LLVM. Namely: if a library sets up a state it should cleanup after
>> itself.
>>
>> That aside, does the radv vulkan driver have unresolved/undefined
>> symbols (check via $ldd -r libvulkan_foo.so) with your workaround ? If
>> not we {c,sh}ould drop the library from the link chain. Alternatively
>> you can try static linking LLVM by using --disable-llvm-shared-libs at
>> mesa configure time.
>
> I see no relocation errors after doing ldd -r with my workaround.
>
> I think the problem lays with how llvm-config is called. We enable
> AMDGPU target and want the AsmPrinter module for it, so we enable
> asmprinter component. However, this enables asmprinter for all enabled
> targets. Since X86 target is enabled by default, this brings
> X86AsmPrinter into the list of libraries.
>
> llvmpipe/gallium need the X86 target, but radv could probably be built
> without it.
>
Pardon for missing your reply here.

In general I agree that one shouldn't link with libraries which they don't need.

At the same time:
 - a library is should tear down all the state that it sets up.
Afaict the LLVM module sets it up "asan-instrument-assembly" thus it
is the one responsible to unregister it.

 - Split shared LLVM wasn't a supported/recommended//good idea, last
time I've heard.
Please use single LLVM shared lib or [separate] static LLVM libs.

Thanks
Emil