Re: [PATCH v2 02/10] Finish move of clock_* functions to libc.

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 03/09/2019 11:34, Zack Weinberg wrote:
> On Tue, Sep 3, 2019 at 9:44 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> On 03/09/2019 10:31, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>> On 03/09/2019 04:29, Florian Weimer wrote:
>>>>> * Zack Weinberg:
>>>>>> diff --git a/rt/Versions b/rt/Versions
>>>>>> index 91e3fd2a20..84d1345420 100644
>>>>>> --- a/rt/Versions
>>>>>> +++ b/rt/Versions
>>>>>> @@ -1,15 +1,3 @@
>>>>>> -libc {
>>>>>> -  GLIBC_2.17 {
>>>>>> -    # c*
>>>>>> -    clock_getres; clock_gettime; clock_settime; clock_getcpuclockid;
>>>>>> -    clock_nanosleep;
>>>>>> -  }
>>>>>> -  GLIBC_PRIVATE {
>>>>>> -    __clock_getres; __clock_gettime; __clock_settime; __clock_getcpuclockid;
>>>>>> -    __clock_nanosleep;
>>>>>> -  }
>>>>>> -}
>>>>>
>>>>> Sorry, you cannot remove the GLIBC_2.17 symbol version in this way,
>>>>> otherwise old binaries will fail to load.  You need to leave behind a
>>>>> dummy function definition.  See __libpthread_version_placeholder for how
>>>>> I handled this in the libpthread/vfork case.
>>>>
>>>> Are you sure about it?
>>>
>>> Yes. 8-)  But I missed that this block was just moved to time/Versions.
>>> Either place will work and generate the same symbol versions.
>>
>> Right, because that was my understanding (the symbol versions will still
>> be generated correctly on libc).
> 
> Yes, I moved the GLIBC_2.17 versions from rt/Versions to time/Versions
> to keep them together with their .c files.
> 
> Adhemerval, Florian, I suspect I won't have very much time to work on
> glibc for the next two weeks.  If either or both of you would like to
> take over the branch for these patches (zack/y2038-preliminaries) and
> keep working on it, please feel free.

I will try to spare some time to create a personal branch based on yours
with my reviewed remarks fixed.

> 
> Incidentally, regarding the cost of converting from a struct timespec
> to a struct timeval on systems where 'long' is 64 bits, I'm getting
> what looks like decent code generation out of this definition of
> TIMESPEC_TO_TIMEVAL:
> 
> #define TIMESPEC_TO_TIMEVAL(tv, ts) do {        \
>     uint32_t __nsec = (ts)->tv_nsec;            \
>     (tv)->tv_sec = (ts)->tv_sec;                \
>     (tv)->tv_usec = __nsec / 1000;              \
> } while (0)
> 
> e.g.
> 
>     movq    8(%rsi), %rax
>     movq    (%rsi), %rdx
>     movl    %eax, %eax
>     movq    %rdx, (%rdi)
>     imulq    $274877907, %rax, %rax
>     shrq    $38, %rax
>     movl    %eax, %eax
>     movq    %rax, 8(%rdi)
>     ret
> 
> (Not sure why the 'movl %eax, %eax', though - clearing the upper 32
> bits maybe?  I might throw this at a superoptimizer and see what it
> comes up with.)
> 
> zw
>