Re: [PATCH] Define __NO_MATH_INLINES and __NO_STRING_INLINES

["Giovanni Bajo" <giovannibajo at libero dot it>]

Joseph S. Myers wrote:

> I take it this doesn't cause problems for building glibc

No, I had not tested building glibc with this patch. I did not know there was
any obligation to do so, nor I am comfortable with glibc code base, building or
testing process enough to prepare and submit a patch in a reasonable chunk of
my time. I am only taking care of the GCC side of this, and no, this does not
mean I am not caring about glibc.

> Since other systems than Linux (e.g. GNU Hurd, *bsd-gnu) use glibc, I
> don't think linux.h is the right place.

Right, but it looked good enough as a first step. Suggestions?

> Has this approach (predefining these macros) been agreed with the
> glibc maintainers as the right way forward?

Not really. We have come to a situation where disabling the inline macros give
good speed boost on many benchmarks. Of course, I understand Jakub's position
of fixing this inside glibc itself rather than GCC, and I appreciate his work
on this, but my patch was not meant only to be an offense to glibc hackers'
hard work, as it seems to have been percieved. Instead, it was meant as a way
of taking a fast step forward for GCC in itself, a clear cut from relying on
other people doing work that should be done in the compiler, and also a good
way of transforming enhancement requests for missing builtins into regressions,
thus having them actually *done* before 2040. You may think I am cheating on
this, but I am reasonable sure that, by doing this, we would have a much better
situation in GCC 3.5.0 than it is in 3.4.0.

> We shouldn't base this on guesses about
> what might or might not be lost; we should document exactly
> what needs to be done to be uniformly better than glibc.

I agree with Zack here, that I don't care much about micro-optimization, and
constructed testcases. As you said, the fact that many benchmarks (and my code
too, which is what pushed me to do this in the first place) do so much better
with __NO_*_INLINES defined should be a reason good enough to approve my patch.

After that, I can prepare such project list if it is a requirement for the
approval, but my understanding is that nobody will ever go through it until we
find real world testcases where we regressed, or until people like Ukos start
working on builtins because they need to. Either way, having a list of what we
would lose from glibc inlines does not seem to be a great help for development.

> Benchmarks are good, but they mix performance on different functions.

Which is the whole point of them, since real code happens to be that way.

> Each function's inline can reasonably be disabled separately, using
> fixincludes, or a patch to glibc to add an appropriate __GNUC_PREREQ
> conditional; the latter also benefits people using glibc with a range
> of different compilers by tuning the inlines to their version.

As I said, I understand this is preferred from glibc, and I appreciate they are
going this way right now, with Jakub's patches. They are also improving
performance of glibc on old versions of GCC, which means that it has probably
been long since the last time they did this kind of tests. I understand that
coordinated efforts are hard to achieve, for different schedules etc. I think
GCC is come to a point where it can start walking on its own legs for builtins,
and cutting all the inlines out with a single patch is a good way to enforce
this, in my opinion. Benchmarks show that we are not going to create tons of
regressions, so why not. Meanwhile, glibc hackers can still work on GNUC_PREREQ
for older GCC versions, that helps a lot. Let's keep in mind that we still
support 3.3 and 3.4, where probably lots of tuning can be done to disable
inlines where they are not needed.

>> * The correct way to operate for the present and the future is to
>> use GCC builtins for anything for which it would make sense to
>> provide an inline version in glibc.
>
> Agreed.  My concern is that each release should be *uniformly* no
> worse than previous releases;

The fact that we have many regressions for each new version show that this is
much of an ideal goal. A good one, indeed, but ideal. My patch is probably
going to increase performance for many users, and cause a few regressions. I
believe this is true for most patches we apply. Yet, we are in stage 1. We
still merged tree-ssa, which is known to have regressions that will be fixed
before release, so I hope my patch is just acceptable as well.

> The benchmarks are enough  for it to make sense to put this patch
> in now without waiting for every glibc optimization to be implemented
> in GCC - provided we document every optimization we are missing,
> in the projects list or (maybe better) in Bugzilla as regressions,

I am willing to coordinate myself with Jakub or whoever to do this, if it is a
requisite for approval.

> and have agreement from the glibc maintainers
> that they will add no more optimizations that are disabled by
> __NO_STRING_INLINES or __NO_MATH_INLINES,
> instead deferring purely to the compiler for all future
> such optimizations.

I *really* don't expect them cheating on us by changing the name of those
macros or playing tricks like that.

Thank you for taking the time for such a thorough review.

Giovanni Bajo