On Thu, Nov 22, 2018 at 3:22 PM Martin Liška <mli...@suse.cz> wrote:
>
> On 11/22/18 3:04 PM, Uros Bizjak wrote:
> > On Thu, Nov 22, 2018 at 3:00 PM Martin Liška <mli...@suse.cz> wrote:
> >>
> >> On 11/22/18 2:51 PM, Uros Bizjak wrote:
> >>> On Thu, Nov 22, 2018 at 2:43 PM Martin Liška <mli...@suse.cz> wrote:
> >>>
> >>>> The patch makes clear we'll not diverge number of elements in
> >>>> processor_names and the corresponding enum. Plus I fixed
> >>>> -march=znver2 native as valid options that were not listed.
> >>>>
> >>>> Patch survives tests and bootstrap on x86_64-linux-gnu.
> >>>>
> >>>> Ready for trunk?
> >>>> Martin
> >>>>
> >>>> gcc/ChangeLog:
> >>>>
> >>>> 2018-11-22 Martin Liska <mli...@suse.cz>
> >>>>
> >>>> * common/config/i386/i386-common.c (processor_names): Add
> >>>> static assert and add missing "znver2".
> >>>> (ix86_get_valid_option_values): Add checking assert for null
> >>>> values and add "native" value if feasible.
> >>>> * config/i386/i386.h: Do not declare size of processor_names.
> >>>> ---
> >>>> gcc/common/config/i386/i386-common.c | 26 ++++++++++++++++++++++----
> >>>> gcc/config/i386/i386.h | 2 +-
> >>>> 2 files changed, 23 insertions(+), 5 deletions(-)
> >>>
> >>> +/* Guarantee that the array is aligned with henum processor_type. */
> >>> +STATIC_ASSERT ((sizeof (processor_names) / sizeof (processor_names[0])
> >>> + == PROCESSOR_max));
> >>>
> >>> Please use ARRAY_SIZE macro here.
> >>
> >> Fixed, it's definitely nicer!
> >>
> >>>
> >>> +#ifdef HAVE_LOCAL_CPU_DETECT
> >>> + /* Add also "native" as possible value. */
> >>> + v.safe_push ("native");
> >>> +#endif
> >>>
> >>> "native" is processed by the driver and this option is never passed to
> >>> cc1.
> >>
> >> But it's a target common hook that is used both from driver and cc1:
> >>
> >> $ ./xgcc -B. --completion=-march=nat
> >> -march=native
> >>
> >>
> >> (Starting program: /dev/shm/objdir/gcc/xgcc -B. --completion=-march=nat)
> >>
> >> $ ./xgcc -B. --help=target | grep native
> >> ...
> >> i386 i486 i586 pentium lakemont pentium-mmx winchip-c6 winchip2 c3
> >> samuel-2 c3-2 nehemiah c7 esther i686 pentiumpro pentium2 pentium3
> >> pentium3m pentium-m pentium4 pentium4m prescott nocona core2 nehalem
> >> corei7 westmere sandybridge corei7-avx ivybridge core-avx-i haswell
> >> core-avx2 broadwell skylake skylake-avx512 cannonlake icelake-client
> >> icelake-server bonnell atom silvermont slm goldmont goldmont-plus tremont
> >> knl knm intel geode k6 k6-2 k6-3 athlon athlon-tbird athlon-4 athlon-xp
> >> athlon-mp x86-64 eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2
> >> eden-x4 nano-x4 k8 k8-sse3 opteron opteron-sse3 athlon64 athlon64-sse3
> >> athlon-fx amdfam10 barcelona bdver1 bdver2 bdver3 bdver4 znver1 znver2
> >> btver1 btver2 generic native
> >>
> >>
> >>
> >> (Starting program: /dev/shm/objdir/gcc/cc1 -quiet -iprefix
> >> /dev/shm/objdir/gcc/../lib64/gcc/x86_64-pc-linux-gnu/9.0.0/ -isystem
> >> ./include -isystem ./include-fixed help-dummy -quiet -dumpbase help-dummy
> >> -mtune=generic -march=x86-64 -auxbase help-dummy --help=target -o
> >> /tmp/cc5NWl4E.s)
> >>
> >> So should be fine.
> >
> > Is -march=native accepted or rejected by cc1? It should be rejected.
>
> It's rejected, but we provide bad hints:
>
> $ ./cc1 -march=native /tmp/arch.c
> cc1: error: bad value (‘native’) for ‘-march=’ switch
> cc1: note: valid arguments to ‘-march=’ switch are: nocona core2 nehalem
> corei7 westmere sandybridge corei7-avx ivybridge core-avx-i haswell core-avx2
> broadwell skylake skylake-avx512 cannonlake icelake-client icelake-server
> bonnell atom silvermont slm goldmont goldmont-plus tremont knl knm x86-64
> eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2 eden-x4 nano-x4 k8 k8-sse3
> opteron opteron-sse3 athlon64 athlon64-sse3 athlon-fx amdfam10 barcelona
> bdver1 bdver2 bdver3 bdver4 znver1 znver2 btver1 btver2 native
>
> It's quite bad because we can't distinguish whether bad -march= value was
> passed to driver, or directly into a FE:
>
> ./cc1 -march=native2 /tmp/arch.c
> cc1: error: bad value (‘native2’) for ‘-march=’ switch
> cc1: note: valid arguments to ‘-march=’ switch are: nocona core2 nehalem
> corei7 westmere sandybridge corei7-avx ivybridge core-avx-i haswell core-avx2
> broadwell skylake skylake-avx512 cannonlake icelake-client icelake-server
> bonnell atom silvermont slm goldmont goldmont-plus tremont knl knm x86-64
> eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2 eden-x4 nano-x4 k8 k8-sse3
> opteron opteron-sse3 athlon64 athlon64-sse3 athlon-fx amdfam10 barcelona
> bdver1 bdver2 bdver3 bdver4 znver1 znver2 btver1 btver2 native; did you mean
> ‘native’?
>
> ./xgcc -B. -march=native2 /tmp/arch.c
> cc1: error: bad value (‘native2’) for ‘-march=’ switch
> cc1: note: valid arguments to ‘-march=’ switch are: nocona core2 nehalem
> corei7 westmere sandybridge corei7-avx ivybridge core-avx-i haswell core-avx2
> broadwell skylake skylake-avx512 cannonlake icelake-client icelake-server
> bonnell atom silvermont slm goldmont goldmont-plus tremont knl knm x86-64
> eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2 eden-x4 nano-x4 k8 k8-sse3
> opteron opteron-sse3 athlon64 athlon64-sse3 athlon-fx amdfam10 barcelona
> bdver1 bdver2 bdver3 bdver4 znver1 znver2 btver1 btver2 native; did you mean
> ‘native’?
>
> Second one should offer 'native' as possible value, but not the first one.
Yes, but this precedes your patch. So the situation is the same...
LGTM then, but please also add static assert to processor_cost _table.
Uros.
