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

[Emil Velikov]

On 24 May 2016 at 15:12, Rob Clark <robdclark at gmail.com> wrote:
> On Tue, May 24, 2016 at 10:03 AM, Rob Clark <robdclark at gmail.com> wrote:
>> On Tue, May 24, 2016 at 9:47 AM, Rob Herring <robh at kernel.org> wrote:
>>> On Tue, May 24, 2016 at 8:40 AM, Rob Clark <robdclark at gmail.com> wrote:
>>>> 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..
>>>
>>> Strange. It fails for me so I have -O0, and I can add the weak attribs
>>> w/o the ifdef and it works.
>>
>> and indeed, if I drop the #ifdef guard, it works..
>>
>
> ok, not really sure what is going on here.. I do end up w/
> -DHAVE_FUNC_ATTRIBUTE_WEAK=1 in generated makefiles, and reverted the
> removal of ifdef guard and it is working still.  Possibly it was just
> that touching the header (originally deleting the ifdef guard) caused
> something to get recompiled which didn't initially..
>
The pendantic test was done as standalone (separate) test. The rest...
I seems to have punched the wrong script during testing. The
backburner is spinning with v2, and I'm keeping a closer eye on the
results.

-Emil