On Wed, Dec 25, 2013 at 10:02 PM, H.J. Lu <hjl.to...@gmail.com> wrote: > 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.
OK, let's leave assert then. Uros.
