[Mesa-dev] radv vs vulkan loader

[Emil Velikov]

On 16 December 2016 at 17:25, Gustaw Smolarczyk <wielkiegie at gmail.com> wrote:
> 2016-12-16 18:12 GMT+01:00 Gustaw Smolarczyk <wielkiegie at gmail.com>:
>> 2016-12-16 17:57 GMT+01:00 Emil Velikov <emil.l.velikov at gmail.com>:
>>> On 16 December 2016 at 15:27, Gustaw Smolarczyk <wielkiegie at gmail.com> wrote:
>>>> 2016-12-16 14:50 GMT+01:00 Emil Velikov <emil.l.velikov at gmail.com>:
>>>>>
>>>>> 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.
>>>>
>>>>
>>>> Yes, I also think this should be really fixed in LLVM. There is however an
>>>> easy work-around for mesa that I have posted a few days ago [1].
>>>>
>>>>>
>>>>>
>>>>>  - Split shared LLVM wasn't a supported/recommended//good idea, last
>>>>> time I've heard.
>>>>
>>>>
>>>> This is how llvm is built on gentoo by default [2]. Because of that it
>>>> possibly affects all gentoo users.
>>>>
>>>>>
>>>>> Please use single LLVM shared lib or [separate] static LLVM libs.
>>>>
>>>>
>>>> I will check what happens when I dlopen/dlclose/dlopen both separate and
>>>> monolithic shared LLVM libraries.
>>>>
>>> Based of git log, I cannot see any justification/information why would
>>> anyone want to enable SHARED_LIBS.
>>> To take this even more fun with ~3.7 series gentoo carries patch
>>> (backport?) which adds the functionality in the first place.
>>>
>>> Seems like devs have missed the warning/notice message [1] that the
>>> option is for LLVM developers ?
>
> FYI, I have filed a gentoo bug report about the usage of an
> unsupported llvm cmake flag [1].
>
> Regards,
> Gustaw
>
> [1] https://bugs.gentoo.org/show_bug.cgi?id=602850

Skimming through the maintainer (?) reply makes me wonder:
 - Projects have managed to iron out the issues integrating with LLVM
(one big shared lib or multiple small static ones).
Thus adding another way to manage things is nice, yet gives greater
change of things being not [so] thoroughly untested and/or broken ?

 - If the documentation about BUILD_SHARED_LIBS is wrong, should one
update that one first ?
Esp. if the developer/maintainer of said codebase is known
upstream/has commit access (?).

If you do come across LLVM related issues with Mesa I would still
suggest sticking either of the two known working and supported
methods.
Might send a patch adding a fat warning at configure time should one
opt for anything else.

Thanks
Emil