On Tue, May 19, 2020 at 11:10 PM Uros Bizjak <ubiz...@gmail.com> wrote: > > On Tue, May 19, 2020 at 11:40 PM H.J. Lu <hjl.to...@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. > I think that unifying libgcc and i386-builtins.c is the way to go. > libgfortran should benefit from this unification, too. > libgfortran shouldn't include cpuinfo.h. It should use __builtin_cpu_is and __builtin_cpu_supports. -- H.J.
