On Wed, Dec 25, 2013 at 12:49 PM, Uros Bizjak <ubiz...@gmail.com> wrote: > On Tue, Dec 24, 2013 at 7:03 PM, H.J. Lu <hjl.to...@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: > > -#define SUBTARGET32_DEFAULT_CPU "i386" > +# ifdef TARGET_CPU_DEFAULT > +# define SUBTARGET32_DEFAULT_CPU > processor_target_table[TARGET_CPU_DEFAULT].name > +# else > +# define SUBTARGET32_DEFAULT_CPU "i386" > +# endif > > Not in this patch ... > > - opts->x_ix86_tune_string = cpu_names[TARGET_CPU_DEFAULT]; > + opts->x_ix86_tune_string > +#ifdef TARGET_CPU_DEFAULT > + = TARGET_64BIT_P (opts->x_ix86_isa_flags) > + ? "generic" : processor_target_table[TARGET_CPU_DEFAULT].name; > +#else > + = "generic"; > +#endif > > Please split these to another patch. > > + gcc_assert (ptr->arch < PROCESSOR_max); > fprintf (file, "%*sarch = %d (%s)\n", > indent, "", > - ptr->arch, > - ((ptr->arch < TARGET_CPU_DEFAULT_max) > - ? cpu_names[ptr->arch] > - : "<unknown>")); > + ptr->arch, processor_target_table[ptr->arch].name); > > I think we should leave the original, with <unknown>, comparing with > PROCESSOR_max and looking into processor_target_table for the name. We > can remove the assert. > > + gcc_assert (ptr->tune < PROCESSOR_max); > fprintf (file, "%*stune = %d (%s)\n", > indent, "", > - ptr->tune, > - ((ptr->tune < TARGET_CPU_DEFAULT_max) > - ? cpu_names[ptr->tune] > - : "<unknown>")); > + ptr->tune, processor_target_table[ptr->tune].name);
ptr->tune and ptr->arch are set by ptr->arch = ix86_arch; ptr->tune = ix86_tune; Both ix86_arch and ix86_tune are enum processor_type. If ix86_arch or ix86_tune >= PROCESSOR_max, x86 backend won't work at all. -- H.J.
