On Tuesday 17 December 2013, Allan Sandfeld Jensen wrote:
> On Monday 16 December 2013, Uros Bizjak wrote:
> > On Mon, Dec 16, 2013 at 10:34 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
> > > On Sun, Dec 15, 2013 at 7:54 PM, Allan Sandfeld Jensen
> > > 
> > > <carew...@gmail.com> wrote:
> > >> Hi again
> > >> 
> > >> On Wednesday 11 December 2013, Uros Bizjak wrote:
> > >>> Hello!
> > >>> 
> > >>> > PR gcc/59422
> > >>> > 
> > >>> > This patch extends the supported targets for function multi
> > >>> > versiong to also include Haswell, Silvermont, and the most recent
> > >>> > AMD models. It also prioritizes AVX2 versions over AMD specific
> > >>> > pre-AVX2 versions.
> > >>> 
> > >>> Please add a ChangeLog entry and attach the complete patch. Please
> > >>> also state how you tested the patch, as outlined in the instructions
> > >>> [1].
> > >>> 
> > >>> [1] http://gcc.gnu.org/contribute.html
> > >> 
> > >> Updated patch for better CPU model detection and added ChangeLog.
> > >> 
> > >> The patch has been tested with the attached test.cpp. Verified that it
> > >> doesn't build before the patch, and that it builds after, and verified
> > >> it selects correct versions at runtime based on either CPU model or
> > >> supported ISA (tested on 3 machines: SandyBridge, IvyBridge and Phenom
> > >> II).
> > >> 
> > >> Btw, I couldn't find anything that corresponds to gcc's btver2 arch.
> > >> Is that an old term for what has become the Jaguar architecture?
> > > 
> > > Thanks for the patch!
> > > 
> > > However, you should not change the existing order of enums in
> > > cpuinfo.c (enum processor_vendor, enum processor_types, enum
> > > processor_subtypes, enum processor_features), but new entries should
> > > be added at the end (before *_MAX entry, if exists) of the enum. The
> > > enums (enum processor_features and enum processor_model) in
> > > config/i386/i386.c should mirror these changes. Please see [1].
> > > 
> > > Probably, we should document this in the source...
> > > 
> > > -      {"sandybridge", M_INTEL_COREI7_SANDYBRIDGE},
> > > +      {"corei7-avx", M_INTEL_COREI7_SANDYBRIDGE},
> > > 
> > > Huh... Thanks for catching this. -march=sandybridge is not
> > > recognized...
> > 
> > -      {"sandybridge", M_INTEL_COREI7_SANDYBRIDGE},
> > +      {"corei7-avx", M_INTEL_COREI7_SANDYBRIDGE},
> > +      {"core-avx-i", M_INTEL_COREI7_IVYBRIDGE},
> > +      {"core-avx2", M_INTEL_COREI7_HASWELL},
> > 
> > Ah, no. These names are not intended to be used in -march. We can
> > follow the tradition and use sandybridge, ivybridge and haswell here.
> 
> I had the problem that "arch=corei7-avx" was not recognized as a valid
> property argument until I made that change. I thought it was the intend to
> merge this list of models with the canonical names, but perhaps it is an
> error in the new parameter validation?
> 
Ah, sorry. I think I misremembered the problem. After reviewing the code 
again, I think the only problem is with target("arch=core-avx-i") because it 
is not in the list of architectures (because it is treated as the same 
architecture as corei7-avx presumably).

I will revert the sandybridge name change in the next patch, and make the new 
names match.

`Allan

Reply via email to