[Mesa-dev] [PATCH] automake: r600: radeonsi: correctly manage libamd_common.la linking

[Emil Velikov]

On 1 March 2017 at 03:49, Michel Dänzer <michel at daenzer.net> wrote:
> On 28/02/17 08:08 PM, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> Since both r600 and radeonsi use code from libamd_common they need to
>> static link it. At the same time, adding a common library to LIB_DEPS is
>> fragile [can lean to multiple symbol definitions] and non-obvious - I
>> had to do a double-take how things work atm.
>>
>> So follow the libradeon.la approach and put common libraries in
>> TARGET_RADEON_COMMON
>>
>> Fixes: 936f5407a7d ("gallium/radeon: Add libamd_common.a to TARGET_LIB_DEPS also for r600")
>> Cc: Michel Dänzer <michel.daenzer at amd.com>
>> Cc: Timothy Arceri <tarceri at itsqueeze.com>
>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>> It's been a while since I looked in the area, yet I really should have
>> seen it coming.
>> ---
>>  src/gallium/drivers/r600/Automake.inc     | 6 +++---
>>  src/gallium/drivers/radeonsi/Automake.inc | 3 ++-
>>  2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/gallium/drivers/r600/Automake.inc b/src/gallium/drivers/r600/Automake.inc
>> index 5995433ce9..3ff5501d6c 100644
>> --- a/src/gallium/drivers/r600/Automake.inc
>> +++ b/src/gallium/drivers/r600/Automake.inc
>> @@ -13,9 +13,9 @@ TARGET_RADEON_WINSYS = \
>>  TARGET_RADEON_COMMON = \
>>       $(top_builddir)/src/gallium/drivers/radeon/libradeon.la
>>
>> +if NEED_RADEON_LLVM
>> +TARGET_RADEON_COMMON += \
>> +     $(top_builddir)/src/amd/common/libamd_common.la
>>  endif
>>
>> -if NEED_RADEON_LLVM
>> -TARGET_LIB_DEPS += \
>> -        $(top_builddir)/src/amd/common/libamd_common.la
>>  endif
>> diff --git a/src/gallium/drivers/radeonsi/Automake.inc b/src/gallium/drivers/radeonsi/Automake.inc
>> index 5a9dcfd9fd..1bc7b93f8c 100644
>> --- a/src/gallium/drivers/radeonsi/Automake.inc
>> +++ b/src/gallium/drivers/radeonsi/Automake.inc
>> @@ -13,6 +13,7 @@ TARGET_RADEON_WINSYS = \
>>       $(top_builddir)/src/gallium/winsys/amdgpu/drm/libamdgpuwinsys.la
>>
>>  TARGET_RADEON_COMMON = \
>> -     $(top_builddir)/src/gallium/drivers/radeon/libradeon.la
>> +     $(top_builddir)/src/gallium/drivers/radeon/libradeon.la \
>> +     $(top_builddir)/src/amd/common/libamd_common.la
>>
>>  endif
>>
>
> Hmm, TARGET_LIB_DEPS gets used in all the same places as
> TARGET_RADEON_COMMON AFAICT, so I guess TARGET_LIB_DEPS is otherwise
> used by automake or special somehow? Anyway,
>
The gist is that TARGET_LIB_DEPS is initialised in the
targets/foo/Makefile.am, then each driver's Automake.inc appends to
it.
While TARGET_RADEON_COMMON is defined in the Automake.inc, since we
want the code linked only as needed.

It's a bit clunky I admit, but makes adding/removing drivers dead trivial ;-)

Thanks for testing !
Emil