V2 [PATCH] x86: Move cpuinfo.h from libgcc to common/config/i386

[H.J. Lu]

On Wed, May 20, 2020 at 4:21 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, May 19, 2020 at 11:10 PM Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > On Tue, May 19, 2020 at 11:40 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > > > > > > > > > I will take a look to see if we share the same CPU detection code between
> > > > > > > > > > libgcc and config/i386/driver-i386.c.
> > > > > > > > >
> > > > > > > > > I don't think it will bring any benefit, this is mainly one huge
> > > > > > > > > switch statement that maps to different stuff in libgcc and
> > > > > > > > > driver-i386.
> > > > > > > >
> > > > > > > > libgcc and config/i386/driver-i386.c differ even before my patch.
> > > > > > > > I think we can do better.
> > > > > > > >
> > > > > > >
> > > > > > > Move cpuinfo.h from libgcc to common/config/i386 so that get_intel_cpu
> > > > > > > can be shared by libgcc, GCC driver, gcc.target/i386/builtin_target.c
> > > > > > > and libgfortran to detect the specific type of Intel CPU.  Update
> > > > > > > libgfortran to use has_cpu_feature to detect x86 CPU features.
> > > > > > >
> > > > > > > Tested on Linux/x86 and Linux/x86-64.  OK for master?
> > > > > >
> > > > > > Handling only Intel targets and not others is a sure way for patch to
> > > > > > be ignored.
> > > > > >
> > > > >
> > > > > Here is the updated patch to cover AMD CPU.  It also fixes:
> > > > >
> > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95212
> > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95220
> > > > >
> > > > > OK for master?
> > > >
> > > > Huh... I didn't think the solution will be this messy... I have to
> > > > rethink the approach a bit.
> > >
> > > That is what will happen when we have the same info in more than one place
> > > There should only one place for CPU info.
> >
> > Looking at the patch, it is clear that cpuinfo.c and driver-i386.c
> > should stay apart. They are two different things, and they are
> > orthogonal to each other; one can be updated without affecting the
> > other one. driver-i386.c handles way more targets than cpuinfo.c and
> > your patch only handles a subset of them. The unification does not
> > bring any benefit, it even complicates things more.
>
> There should one place to check a CPU feature, not 2.  It can
> be done with a proper cpuinfo.h.
>

This updated patch does it.   It removes duplicated codes and
simplifies the implementation.  With this patch, we just to update
a single place to add the new ISA support.


--
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Move-cpuinfo.h-from-libgcc-to-common-config-i386.patch
Type: application/x-patch
Size: 102193 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20200521/af893f8d/attachment-0001.bin>