[Mesa-dev] [PATCH] c11/threads: resolve link issues with -O0

[Rob Clark]

On Tue, May 24, 2016 at 9:17 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Tue, May 24, 2016 at 9:15 AM, Rob Herring <robh at kernel.org> wrote:
>> On Tue, May 24, 2016 at 8:09 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>> On Tue, May 24, 2016 at 9:06 AM, Rob Herring <robh at kernel.org> wrote:
>>>> On Tue, May 24, 2016 at 4:57 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>>>
>>>>> Add weak symbol notation for the pthread_mutexattr* symbols, thus making
>>>>> the linker happy. When building with -O1 or greater the optimiser will
>>>>> kick in and remove the said functions as they are dead/unreachable code.
>>>>>
>>>>> Ideally we'll enable the optimisations locally, yet that does not seem
>>>>> to work atm.
>>>>>
>>>>> Cc: Alejandro Piñeiro <apinheiro at igalia.com>
>>>>> Cc: Ben Widawsky <ben at bwidawsk.net>
>>>>> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
>>>>> Cc: Mark Janes <mark.a.janes at intel.com>
>>>>> Cc: Rob Clark <robdclark at gmail.com>
>>>>> Cc: Rob Herring <robh at kernel.org>
>>>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>>>> ---
>>>>> Building with -Wall -Wextra -pedantic and it does not cause any
>>>>> additional warnings/errors.
>>>>> ---
>>>>>  include/c11/threads_posix.h | 26 ++++++++++++++++++++++++++
>>>>>  1 file changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/include/c11/threads_posix.h b/include/c11/threads_posix.h
>>>>> index 11d36e4..61b7fab 100644
>>>>> --- a/include/c11/threads_posix.h
>>>>> +++ b/include/c11/threads_posix.h
>>>>> @@ -169,6 +169,32 @@ mtx_destroy(mtx_t *mtx)
>>>>>      pthread_mutex_destroy(mtx);
>>>>>  }
>>>>>
>>>>> +/*
>>>>> + * XXX: Workaround when building with -O0 and without pthreads link.
>>>>> + *
>>>>> + * In such cases constant folding and dead code elimination won't be
>>>>> + * available, thus the compiler will always add the pthread_mutexattr*
>>>>> + * functions into the binary. As we try to link, we'll fail as the
>>>>> + * symbols are unresolved.
>>>>> + *
>>>>> + * Ideally we'll enable the optimisations locally, yet that does not
>>>>> + * seem to work.
>>>>> + *
>>>>> + * So the alternative workaround is to annotate the symbols as weak.
>>>>> + * Thus the linker will be happy and things don't clash when building
>>>>> + * with -O1 or greater.
>>>>> + */
>>>>> +#ifdef HAVE_FUNC_ATTRIBUTE_WEAK
>>>>
>>>> Doesn't this need to get defined somewhere?
>>>
>>> See m4/ax_gcc_func_attribute.m4.
>>
>> Right, but then don't we need "AX_GCC_FUNC_ATTRIBUTE([weak])" added in
>> configure.ac? Only the following are checked:
>>
>> configure.ac:AX_GCC_FUNC_ATTRIBUTE([const])
>> configure.ac:AX_GCC_FUNC_ATTRIBUTE([flatten])
>> configure.ac:AX_GCC_FUNC_ATTRIBUTE([format])
>> configure.ac:AX_GCC_FUNC_ATTRIBUTE([malloc])
>> configure.ac:AX_GCC_FUNC_ATTRIBUTE([packed])
>> configure.ac:AX_GCC_FUNC_ATTRIBUTE([pure])
>> configure.ac:AX_GCC_FUNC_ATTRIBUTE([returns_nonnull])
>> configure.ac:AX_GCC_FUNC_ATTRIBUTE([unused])
>> configure.ac:AX_GCC_FUNC_ATTRIBUTE([warn_unused_result])
>
> Right :) Since Emil says he tested, must be some sort of error in his
> testing procedure. Or he forgot to commit that file.

hmm, even with the below hunk added, it isn't working for me..  I
suspect Emil might need to double check his generated Makefiles to be
sure they have -O0..

----
diff --git a/configure.ac b/configure.ac
index 47c138a..fc0b1db 100644
--- a/configure.ac
+++ b/configure.ac
@@ -233,6 +233,7 @@ AX_GCC_FUNC_ATTRIBUTE([pure])
 AX_GCC_FUNC_ATTRIBUTE([returns_nonnull])
 AX_GCC_FUNC_ATTRIBUTE([unused])
 AX_GCC_FUNC_ATTRIBUTE([warn_unused_result])
+AX_GCC_FUNC_ATTRIBUTE([weak])

 AM_CONDITIONAL([GEN_ASM_OFFSETS], test "x$GEN_ASM_OFFSETS" = xyes)
----

>   -ilia