Re: PATCH: PR target/59587: cpu_names in i386.c is accessed with wrong index

[Uros Bizjak <ubizjak at gmail dot com>]

On Wed, Dec 25, 2013 at 10:31 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

>> >>>>>>>> cpu_names in i386.c is only used by ix86_function_specific_print which
>> >>>>>>>> accesses it with enum processor_type index. But cpu_names is defined as
>> >>>>>>>> array with enum target_cpu_default index.  This patch adds processor
>> >>>>>>>> names to processor_target_table and uses processor_target_table instead
>> >>>>>>>> of cpu_names.  It removes cpu_names and target_cpu_default.  Tested on
>> >>>>>>>> Linux/x86-64.  OK to install?
>> >>>>>>>
>> >>>>>>> Wait a moment,
>> >>>>>>>
>> >>>>>>> it looks to me that TARGET_CPU_DEFAULT has to be synchronized with
>> >>>>>>> const processor_alias_table, so we are able to define various ISA
>> >>>>>>> extensions by selecting TARGET_CPU_*. The TARGET_CPU_DEFAULT can then
>> >>>>>>
>> >>>>>> TARGET_CPU_DEFAULT sets the default -mtune=, not -march=.
>> >>>>>>
>> >>>>>>> be used to select extensions in the same way as PROCESSOR_* selects
>> >>>>>>> tuning for certain processor.
>> >>>>>>
>> >>>>>> It has been like this for a long time.  For x86, TARGET_CPU_DEFAULT
>> >>>>>> isn't defined no matter which configure options are used.  We can
>> >>>>>> change config.gcc to set TARGET_CPU_DEFAULT to proper PROCESSOR_XXX or
>> >>>>>> set it to a string "xxx" for processor "xxx".
>> >>>>>> But GCC driver always passes -march=/-mtune= to toplev.c so that
>> >>>>>> TARGET_CPU_DEFAULT is normally used.
>> >>>>
>> >>>> I meant to say "TARGET_CPU_DEFAULT isn't normally used."
>> >>>>
>> >>>>>
>> >>>>> Let me rethink this a bit, please do not commit the patch.
>> >>>>>
>> >>>
>> >>> TARGET_CPU_DEFAULT is left over for 32-bit target before --with-arch=
>> >>> and --with-cpu= were added.  Today, -mtune=xxx -march=xxx are
>> >>> always passed to cc1 by GCC driver.  If cc1 is run by hand and
>> >>> -mtune=xxx -march=xxx aren't passed to cc1, we should do
>> >>>
>> >>> 1. For 64-bit, it should be the same as -mtune=generic -march=x86_64
>> >>> are passed.
>> >>> 2. For 32-bit, it should be the same as -mtune=cpu -march=cpu are
>> >>> passed, where "cpu" is the target cpu used to configure GCC,
>> >>> like i386 in i386-linux, i486 in i486-linux, .... But there is no i786
>> >>> cpu.  i786 is treated as i686.  If SUBTARGET32_DEFAULT_CPU
>> >>> is defined, it should be the same -mtune=SUBTARGET32_DEFAULT_CPU
>> >>> -march=SUBTARGET32_DEFAULT_CPU.
>> >>>
>> >>> Here is the patch to implement this.
>> >>
>> >> Let's do one step at a time. So, let's split the patch back to target/59587 fix:

> 2013-12-25   H.J. Lu  <hongjiu.lu@intel.com>
>
>         PR target/59587
>         * config/i386/i386.c (struct ptt): Add a field for processor
>         name.
>         (processor_target_table): Sync with processor_type.  Add
>         processor names.
>         (cpu_names): Removed.
>         (ix86_option_override_internal): Default x_ix86_tune_string
>         to processor_target_table[TARGET_CPU_DEFAULT].name.
>         (ix86_function_specific_print): Assert arch and tune <
>         PROCESSOR_max.  Use processor_target_table to print arch and
>         tune names.
>         * config/i386/i386.h (TARGET_CPU_DEFAULT): Default to
>         PROCESSOR_GENERIC.
>         (target_cpu_default): Removed.
>         (processor_type): Reordered.

OK for mainline and for 4.8 after a few days in mainline.

Thanks,
Uros.