[Mesa-stable] [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Kai Wasserbäch]

Hey Jan,
Jan Vesely wrote on 01.05.2018 23:59:
> On Tue, 2018-05-01 at 18:23 +0200, Kai Wasserbäch wrote:
>> Jan Vesely wrote on 01.05.2018 17:19:
>>> On Tue, 2018-05-01 at 14:14 +0200, Kai Wasserbäch wrote:
>>>> [...]
>>>>
>>>>  src/gallium/targets/opencl/Makefile.am | 3 +--
>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/gallium/targets/opencl/Makefile.am b/src/gallium/targets/opencl/Makefile.am
>>>> index de68a93ad5..f0e1de7797 100644
>>>> --- a/src/gallium/targets/opencl/Makefile.am
>>>> +++ b/src/gallium/targets/opencl/Makefile.am
>>>> @@ -23,11 +23,10 @@ lib at OPENCL_LIBNAME@_la_LIBADD = \
>>>>  	$(LIBELF_LIBS) \
>>>>  	$(DLOPEN_LIBS) \
>>>>  	-lclangCodeGen \
>>>> -	-lclangFrontendTool \
>>>>  	-lclangFrontend \
>>>> +	-lclangFrontendTool \
>>>
>>> This is strange. Why does reordering help here? Do we use -Wl,--as-
>>> needed anywhere?
>>
>> No, not that I can see.
>>
>>> Should we use -Wl,--start-group/-Wl,--end-group for all clang libraries
>>> instead?
>>
>> Maybe? This was the simplest fix I could come up with, but if there's a
>> preference for a link group, I can give that a try as well.
> 
> So the fix is to change ordering?

yes.

> Does using groups fix the issue as well? I think that would be
> preferable, but I use split .so files, so I don't hit this issue.

I tried convincing autotools to work with those flags but failed. The only
option I see to solve this, is very messy IMHO (and would still need the
ordering fix): putting -Wl,--{start,end}-group directly into the right places in
lib at OPENCL_LIBNAME@_la_LIBADD is forbidden by automake ("error: linker flags
such as '-Wl,--start-group' belong in 'lib at OPENCL_LIBNAME@_la_LDFLAGS'") and
adding them to lib at OPENCL_LIBNAME@_la_LDFLAGS like automake is suggesting won't
work for obvious reasons. The only solution I can see is to work with
substitution because automake seems to "not see" the flags then. I could do an
unconditional replacement, but there are probably linkers with no support for
these flags, which would mean I'd have to do the ordering fix in any case and
then conditionally set "-Wl,--{start,end}-group" just for the GNU toolchain with
no immediate benefit beyond future-proofing this section.
But maybe people who are deeper into the whole autotools stuff (Emil?
Francisco?) can point me to a solution? Otherwise I'd like to return to my
original patch which fixes the FTBFS and works for now. Or maybe the library
could be linked against libclang.so (at least when --enable-llvm-shared-libs is set?

>>>>  	-lclangDriver \
>>>>  	-lclangSerialization \
>>>> -	-lclangCodeGen \
>>>
>>> Is this change related?
>>
>> Not really, just a minor clean-up while I was busy a few lines above.
>> "clangCodeGen" is already named on the first Clang library line.
> 
> ah, all right, maybe mention it in the commit message?

Do I need to resend the patch for that or can you just add a line like "This
change also removes the duplicate clangCodeGen line (trivial change)." before
pushing, considering, that there are two T-b tags to be added anyway?

Cheers,
Kai

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20180502/73917241/attachment.sig>