This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [[PATCH RFC 2] 02/63] Y2038: add function __difftime64
- From: Albert ARIBAUD <albert dot aribaud at 3adev dot fr>
- To: Paul Eggert <eggert at cs dot ucla dot edu>
- Cc: Florian Weimer <fweimer at redhat dot com>, libc-alpha at sourceware dot org
- Date: Tue, 22 May 2018 22:58:05 +0200
- Subject: Re: [[PATCH RFC 2] 02/63] Y2038: add function __difftime64
- References: <20180418201819.15952-1-albert.aribaud@3adev.fr> <20180418201819.15952-2-albert.aribaud@3adev.fr> <20180418201819.15952-3-albert.aribaud@3adev.fr> <0a04fc43-9e92-1bbc-843d-049d1026d971@cs.ucla.edu> <20180419150411.4d7ee629@athena> <34e2461c-5da4-181d-d34b-b63c7f89dbf6@cs.ucla.edu> <20180502092209.540847ad@athena> <1a4c2101-130f-79bd-71ff-3393c42c000e@redhat.com> <ccf24625-05e8-85ec-9767-31e90e20c7d0@cs.ucla.edu> <20180503193056.0ded171f@athena> <6f29bd3f-6d2f-f6d6-4efd-a46f2b1cf5fa@cs.ucla.edu>
Hi Paul,
On Thu, 3 May 2018 10:53:06 -0700, Paul Eggert <eggert@cs.ucla.edu>
wrote :
> On 05/03/2018 10:30 AM, Albert ARIBAUD wrote:
> > the simpler code in difftime.c which 64-bit signed integer
> > needs is enclosed in run-time conditionals, not compile-time
> > conditionals
>
>
> These run-time conditionals are optimized away, and have zero run-time
> overhead.
>
> Really, you should use difftime.c and see what it does (look at the
> machine code that is generated if you like). You will see that it does
> the right thing and generates the same instructions that your copy
> would. Similarly for many of the other routines that you incorrectly
> think we need to make source-code copies of.
>
>
> > I am choosing not to assume how clever compilers can be, and I consider
> > that putting a run-time test in some source code will be costlier in
> > execution time than not putting it there.
>
> This is completely, 100%, backwards. It is routine in high-quality
> software nowadays to assume decent optimizing compilers; for example,
> that's why we now use functions rather than macros, since simple
> functions are routinely inlined. The glibc implementation assumes GCC,
> it is not our job to second-guess GCC, and we should not add complexity
> to glibc when GCC does a perfectly good job of optimizing simpler code.
I make a difference between optimizing and optimizing *away*.
Optimizing: no problem -- let the compiler reorganize the code, factor
it, tweak it in whatever way preserves its semantics while making it
smaller or faster or both, I'm happy with it, and nowadays compilers do
a very good job of it indeed, I can see that whenever I'm trying to
step-debug with -O3.
But here we're talking about optimizing *away* code which the
compiler finds it won't use at all any more.
With the 32-bit time implementation, I do understand that it is
intended to work with different time_t types, and this implies the
presence of source code intended for one implementation but seen as
dead code for others. Simplifying the 32-bit difftime would indeed
imply removing some functionality required by some existing time_t
implementation, and is therefore out of question (plus, that's not on
my plate).
OTOH, the 64-bit implementation uses a single, well-enough-defined,
64-bit, signed integer time_t implementation across all architectures
where it will exist (endianness notwithstanding, and that's not an
issue for difftime) which can and should benefit from a single
implementation simpler that the 32-bit one. Any code in the 64-bit
difftime which assumes a time_t implementation other than a 64-bit
signed int is simply dead code in*all* cases, and it won't *ever* be
used. Yes, the compiler can optimize it away again and again, but why
keep true dead source code when we know it is truly dead? That's
keeping source code complexity where we could remove it, IMO.
[that is, of course, assuming that we *know* it is dead -- again, I may
have oversimplified my previous attempt. I am going through the code
again and triple-checking that the implementation works with a 64-bit
signed time_t.]
Cordialement,
Albert ARIBAUD
3ADEV