diff mbox

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

Message ID CAMe9rOp+o45EwsMhShB_LpAmy1=1jpNxupudttqTBpuiB2vZMg@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu Jan. 8, 2014, 7:13 p.m. UTC 
On Wed, Dec 25, 2013 at 2:32 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> 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.

I am testing this patch.  I will check it into 4.8 branch after
finishing regression test.

Thanks.
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 6493bb2..f17bf56 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,24 @@ 
+2014-01-08   H.J. Lu  <hongjiu.lu@intel.com>
+
+ Backport from mainline
+ 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_GENERIC32.
+ (target_cpu_default): Removed.
+ (processor_type): Reordered.
+
 2014-01-08  Uros Bizjak  <ubizjak@gmail.com>

  Backport from mainline
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index e03aa72..c06c220 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2409,6 +2409,7 @@  static tree ix86_veclibabi_acml (enum
built_in_function, tree, tree);
 /* Processor target table, indexed by processor number */
 struct ptt
 {
+  const char *const name; /* processor name  */
   const struct processor_costs *cost; /* Processor costs */
   const int align_loop; /* Default alignments.  */
   const int align_loop_max_skip;
@@ -2417,66 +2418,31 @@  struct ptt
   const int align_func;
 };

+/* This table must be in sync with enum processor_type in i386.h.  */
 static const struct ptt processor_target_table[PROCESSOR_max] =
 {
-  {&i386_cost, 4, 3, 4, 3, 4},
-  {&i486_cost, 16, 15, 16, 15, 16},
-  {&pentium_cost, 16, 7, 16, 7, 16},
-  {&pentiumpro_cost, 16, 15, 16, 10, 16},
-  {&geode_cost, 0, 0, 0, 0, 0},
-  {&k6_cost, 32, 7, 32, 7, 32},
-  {&athlon_cost, 16, 7, 16, 7, 16},
-  {&pentium4_cost, 0, 0, 0, 0, 0},
-  {&k8_cost, 16, 7, 16, 7, 16},
-  {&nocona_cost, 0, 0, 0, 0, 0},
-  /* Core 2  */
-  {&core_cost, 16, 10, 16, 10, 16},
-  /* Core i7  */
-  {&core_cost, 16, 10, 16, 10, 16},
-  /* Core avx2  */
-  {&core_cost, 16, 10, 16, 10, 16},
-  {&generic32_cost, 16, 7, 16, 7, 16},
-  {&generic64_cost, 16, 10, 16, 10, 16},
-  {&amdfam10_cost, 32, 24, 32, 7, 32},
-  {&bdver1_cost, 16, 10, 16, 7, 11},
-  {&bdver2_cost, 16, 10, 16, 7, 11},
-  {&bdver3_cost, 16, 10, 16, 7, 11},
-  {&btver1_cost, 16, 10, 16, 7, 11},
-  {&btver2_cost, 16, 10, 16, 7, 11},
-  {&atom_cost, 16, 15, 16, 7, 16}
-};
-
-static const char *const cpu_names[TARGET_CPU_DEFAULT_max] =
-{
-  "generic",
-  "i386",
-  "i486",
-  "pentium",
-  "pentium-mmx",
-  "pentiumpro",
-  "pentium2",
-  "pentium3",
-  "pentium4",
-  "pentium-m",
-  "prescott",
-  "nocona",
-  "core2",
-  "corei7",
-  "core-avx2",
-  "atom",
-  "geode",
-  "k6",
-  "k6-2",
-  "k6-3",
-  "athlon",
-  "athlon-4",
-  "k8",
-  "amdfam10",
-  "bdver1",
-  "bdver2",
-  "bdver3",
-  "btver1",
-  "btver2"
+  {"generic", &generic32_cost, 16, 7, 16, 7, 16},
+  {"generic", &generic64_cost, 16, 10, 16, 10, 16},
+  {"i386", &i386_cost, 4, 3, 4, 3, 4},
+  {"i486", &i486_cost, 16, 15, 16, 15, 16},
+  {"pentium", &pentium_cost, 16, 7, 16, 7, 16},
+  {"pentiumpro", &pentiumpro_cost, 16, 15, 16, 10, 16},
+  {"pentium4", &pentium4_cost, 0, 0, 0, 0, 0},
+  {"nocona", &nocona_cost, 0, 0, 0, 0, 0},
+  {"core2", &core_cost, 16, 10, 16, 10, 16},
+  {"corei7", &core_cost, 16, 10, 16, 10, 16},
+  {"core-avx2", &core_cost, 16, 10, 16, 10, 16},
+  {"atom", &atom_cost, 16, 15, 16, 7, 16},
+  {"geode", &geode_cost, 0, 0, 0, 0, 0},
+  {"k6", &k6_cost, 32, 7, 32, 7, 32},
+  {"athlon", &athlon_cost, 16, 7, 16, 7, 16},
+  {"k8", &k8_cost, 16, 7, 16, 7, 16},
+  {"amdfam10", &amdfam10_cost, 32, 24, 32, 7, 32},
+  {"bdver1", &bdver1_cost, 16, 10, 16, 7, 11},
+  {"bdver2", &bdver2_cost, 16, 10, 16, 7, 11},
+  {"bdver3", &bdver3_cost, 16, 10, 16, 7, 11},
+  {"btver1", &btver1_cost, 16, 10, 16, 7, 11},
+  {"btver2", &btver2_cost, 16, 10, 16, 7, 11}
 };


 static bool
@@ -3125,7 +3091,8 @@  ix86_option_override_internal (bool main_args_p)
  ix86_tune_string = ix86_arch_string;
       if (!ix86_tune_string)
  {
-  ix86_tune_string = cpu_names[TARGET_CPU_DEFAULT];
+  ix86_tune_string
+    = processor_target_table[TARGET_CPU_DEFAULT].name;
   ix86_tune_defaulted = 1;
  }

@@ -4078,19 +4045,15 @@  ix86_function_specific_print (FILE *file, int indent,
     = ix86_target_string (ptr->x_ix86_isa_flags, ptr->x_target_flags,
   NULL, NULL, ptr->x_ix86_fpmath, false);

+  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);

+  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);

   fprintf (file, "%*sbranch_cost = %d\n", indent, "", ptr->branch_cost);

diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index a69862c..335cf61 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -197,10 +197,10 @@  extern const struct processor_costs ix86_size_cost;

 /* Macros used in the machine description to test the flags.  */

-/* configure can arrange to make this 2, to force a 486.  */
+/* configure can arrange to change it.  */

 #ifndef TARGET_CPU_DEFAULT
-#define TARGET_CPU_DEFAULT TARGET_CPU_DEFAULT_generic
+#define TARGET_CPU_DEFAULT PROCESSOR_GENERIC32
 #endif

 #ifndef TARGET_FPMATH_DEFAULT
@@ -591,43 +591,6 @@  extern const char *host_detect_local_cpu (int
argc, const char **argv);
 /* Target Pragmas.  */
 #define REGISTER_TARGET_PRAGMAS() ix86_register_pragmas ()

-enum target_cpu_default
-{
-  TARGET_CPU_DEFAULT_generic = 0,
-
-  TARGET_CPU_DEFAULT_i386,
-  TARGET_CPU_DEFAULT_i486,
-  TARGET_CPU_DEFAULT_pentium,
-  TARGET_CPU_DEFAULT_pentium_mmx,
-  TARGET_CPU_DEFAULT_pentiumpro,
-  TARGET_CPU_DEFAULT_pentium2,
-  TARGET_CPU_DEFAULT_pentium3,
-  TARGET_CPU_DEFAULT_pentium4,
-  TARGET_CPU_DEFAULT_pentium_m,
-  TARGET_CPU_DEFAULT_prescott,
-  TARGET_CPU_DEFAULT_nocona,
-  TARGET_CPU_DEFAULT_core2,
-  TARGET_CPU_DEFAULT_corei7,
-  TARGET_CPU_DEFAULT_haswell,
-  TARGET_CPU_DEFAULT_atom,
-
-  TARGET_CPU_DEFAULT_geode,
-  TARGET_CPU_DEFAULT_k6,
-  TARGET_CPU_DEFAULT_k6_2,
-  TARGET_CPU_DEFAULT_k6_3,
-  TARGET_CPU_DEFAULT_athlon,
-  TARGET_CPU_DEFAULT_athlon_sse,
-  TARGET_CPU_DEFAULT_k8,
-  TARGET_CPU_DEFAULT_amdfam10,
-  TARGET_CPU_DEFAULT_bdver1,
-  TARGET_CPU_DEFAULT_bdver2,
-  TARGET_CPU_DEFAULT_bdver3,
-  TARGET_CPU_DEFAULT_btver1,
-  TARGET_CPU_DEFAULT_btver2,
-
-  TARGET_CPU_DEFAULT_max
-};
-
 #ifndef CC1_SPEC
 #define CC1_SPEC "%(cc1_cpu) "