Re: Update list of i686-class processors in sysdeps/x86/cpu-features.h

[Florian Weimer <fweimer at redhat dot com>]

Joseph Myers <joseph@codesourcery.com> writes:

> On Wed, 19 Sep 2018, Florian Weimer wrote:
>
>> * Joseph Myers:
>> 
>> > Question: would it be better to implement this conditional in a
>> > negative sense (!defined __i486__ && !defined __i586__ && !defined
>> > __geode__, based on what macros are in the fixed conditional, but
>> > maybe using __k6__ instead of __geode__ based on GCC's understanding
>> > of the options in question) to reduce the chances of it needing
>> > updating in future?  (__i586__ is actually handled above.)
>> 
>> I think the GCC developers expect that we use the individual feature
>> test macros (__SSE2__ etc.).  The model-based macros are useless
>
> Which don't exist for CMOV, for example.

That's really unfortunate.

> We have a general principle that it's best for compile-time selection of 
> subarchitecture variants of functions to be based on how the compiler 
> behaves rather than configuring for a variant host triplet or using 
> --with-cpu.  We could e.g. select SSE functions by default based on 
> __SSE2__ (even if we don't do so at present).  But testing whether the 
> compiler produces a CMOV instruction for particular code seems more 
> fragile than testing #if conditions for processors without CMOV.

I agree about CMOV because it is extremely unlikely that we will see
another useful x86 CPU that doesn't have CMOV.

> As I suggested in the discussion in bug 23630, I think it would make sense 
> to eliminate, or at least minimize, the i586 / i686 / i786 sysdeps 
> directories, to reduce the number of function variants to be maintained; 
> maybe there are a few places where having conditionals in files on whether 
> CMOV is supported would be awkward, but in many places either an 
> i686-tuned version can be used as generic, or only small local 
> conditionals for CMOV support are needed.  E.g. 
> sysdeps/i386/i686/pthread_spin_trylock.S defines HAVE_CMOV; that ought to 
> be something available in a generic header, based on some way on what the 
> compiler predefines.

That particular function should probably be written in C, so that the
compiler can produce a better branch-free sequence, without using CMOV.
(I would be slightly surprised if the availability of CMOV would matter
to assembler code tuned for generic x86.)

>> > Question: is the inclusion of __k6__ in the present conditional
>> > logically incorrect, and the omission of __geode__ logically
>> > incorrect, as regards whether to define HAVE_I686?  The way GCC
>> > defines -march=k6 and -march=geode, it seems to think that the former
>> > excludes CMOV and the latter includes it.  (-march=geode is
>> > specifically "AMD Geode embedded processor with MMX and 3DNow!@:
>> > instruction set support.".)
>> 
>> Geode lacks support for long NOPs, but our i686 port requires them.
>
> Requires them where?  Through the use of -Wa,-mtune=i686 in 
> sysdeps/i386/i686/Makefile, or also through explicit uses?

Hmm.  I may have misremembered.  The current port should be okay as long
as CET isn't enabled.

For a while, GAS generated long NOPs with -Wa,-mtune=i686, but I found
this commit in Fedora:

commit 55a30785d463ecd9cc1fa0e4b384b9f8b93a370a
Author: Jeff Law <law@redhat.com>
Date:   Mon Sep 24 09:25:31 2012 -0600

      - Remove most of fedora-nscd patch as we no longer use the
        old init files, but systemd instead.
      - Remove path-to-vi patch.  With the usr-move changes that
        patch is totally unnecessary.
      - Remove i686-nopl patch.  Gas was changed back in 2011 to
        avoid nopl.
      - Move gai-rfc1918 patch to submitted upstream status

And that re-instantiated the -Wa,-mtune=i686 options which were patched
out before.

We also didn't receive any complaints that Fedora 27 did not run on AMD
Geode, so glibc 2.26 at least was still okay.

Thanks,
Florian