public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix x86 -march/-mtune after recent icelake-* split (PR target/84902)
@ 2018-03-16 20:35 Jakub Jelinek
  2018-03-17 10:21 ` Uros Bizjak
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2018-03-16 20:35 UTC (permalink / raw)
  To: Uros Bizjak, Kirill Yukhin, Koval, Julia; +Cc: gcc-patches

Hi!

We now have more than 32 enumerators in enum processor_type (well, we had
33 already before the icelake-* split, but the last one isn't really used),
and while all the m_* macros were changed tom 1U<<whatever to
HOST_WIDE_INT_1U<<whatever, the ix86_tune_mask and ix86_arch_mask
vars weren't, so for PROCESSOR_ZNVER1 (-march=znver1 or -mtune=znver1)
we now invoke undefined behavior, which can e.g. mean treat it like
PROCESSOR_GENERIC, or no processor at all.
Also, given that the m_* macros are using HOST_WIDE_INT_1U, it seems more
consistent to use unsigned HOST_WIDE_INT type for the
initial_ix86_*_features tables, rather than unsigned long long.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-03-16  Jakub Jelinek  <jakub@redhat.com>

	PR target/84902
	* config/i386/i386.c (initial_ix86_tune_features,
	initial_ix86_arch_features): Use unsigned HOST_WIDE_INT rather than
	unsigned long long.
	(set_ix86_tune_features): Change ix86_tune_mask from unsigned int
	to unsigned HOST_WIDE_INT, initialize to HOST_WIDE_INT_1U << ix86_tune
	rather than 1u << ix86_tune.  Formatting fix.
	(ix86_option_override_internal): Change ix86_arch_mask from
	unsigned int to unsigned HOST_WIDE_INT, initialize to
	HOST_WIDE_INT_1U << ix86_arch rather than 1u << ix86_arch.
	(ix86_function_specific_restore): Likewise.

--- gcc/config/i386/i386.c.jj	2018-03-15 22:07:57.964242564 +0100
+++ gcc/config/i386/i386.c	2018-03-16 18:35:53.621105345 +0100
@@ -183,7 +183,7 @@ unsigned char ix86_tune_features[X86_TUN
 
 /* Feature tests against the various tunings used to create ix86_tune_features
    based on the processor mask.  */
-static unsigned long long initial_ix86_tune_features[X86_TUNE_LAST] = {
+static unsigned HOST_WIDE_INT initial_ix86_tune_features[X86_TUNE_LAST] = {
 #undef DEF_TUNE
 #define DEF_TUNE(tune, name, selector) selector,
 #include "x86-tune.def"
@@ -195,7 +195,7 @@ unsigned char ix86_arch_features[X86_ARC
 
 /* Feature tests against the various architecture variations, used to create
    ix86_arch_features based on the processor mask.  */
-static unsigned long long initial_ix86_arch_features[X86_ARCH_LAST] = {
+static unsigned HOST_WIDE_INT initial_ix86_arch_features[X86_ARCH_LAST] = {
   /* X86_ARCH_CMOV: Conditional move was added for pentiumpro.  */
   ~(m_386 | m_486 | m_PENT | m_LAKEMONT | m_K6),
 
@@ -3310,7 +3310,7 @@ parse_mtune_ctrl_str (bool dump)
 static void
 set_ix86_tune_features (enum processor_type ix86_tune, bool dump)
 {
-  unsigned int ix86_tune_mask = 1u << ix86_tune;
+  unsigned HOST_WIDE_INT ix86_tune_mask = HOST_WIDE_INT_1U << ix86_tune;
   int i;
 
   for (i = 0; i < X86_TUNE_LAST; ++i)
@@ -3318,7 +3318,8 @@ set_ix86_tune_features (enum processor_t
       if (ix86_tune_no_default)
         ix86_tune_features[i] = 0;
       else
-        ix86_tune_features[i] = !!(initial_ix86_tune_features[i] & ix86_tune_mask);
+	ix86_tune_features[i]
+	  = !!(initial_ix86_tune_features[i] & ix86_tune_mask);
     }
 
   if (dump)
@@ -3373,7 +3374,7 @@ ix86_option_override_internal (bool main
 			       struct gcc_options *opts_set)
 {
   int i;
-  unsigned int ix86_arch_mask;
+  unsigned HOST_WIDE_INT ix86_arch_mask;
   const bool ix86_tune_specified = (opts->x_ix86_tune_string != NULL);
 
   const wide_int_bitmask PTA_3DNOW (HOST_WIDE_INT_1U << 0);
@@ -4234,7 +4235,7 @@ ix86_option_override_internal (bool main
       XDELETEVEC (s);
     }
 
-  ix86_arch_mask = 1u << ix86_arch;
+  ix86_arch_mask = HOST_WIDE_INT_1U << ix86_arch;
   for (i = 0; i < X86_ARCH_LAST; ++i)
     ix86_arch_features[i] = !!(initial_ix86_arch_features[i] & ix86_arch_mask);
 
@@ -5159,7 +5160,7 @@ ix86_function_specific_restore (struct g
 {
   enum processor_type old_tune = ix86_tune;
   enum processor_type old_arch = ix86_arch;
-  unsigned int ix86_arch_mask;
+  unsigned HOST_WIDE_INT ix86_arch_mask;
   int i;
 
   /* We don't change -fPIC.  */
@@ -5210,7 +5211,7 @@ ix86_function_specific_restore (struct g
   /* Recreate the arch feature tests if the arch changed */
   if (old_arch != ix86_arch)
     {
-      ix86_arch_mask = 1u << ix86_arch;
+      ix86_arch_mask = HOST_WIDE_INT_1U << ix86_arch;
       for (i = 0; i < X86_ARCH_LAST; ++i)
 	ix86_arch_features[i]
 	  = !!(initial_ix86_arch_features[i] & ix86_arch_mask);

	Jakub

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix x86 -march/-mtune after recent icelake-* split (PR target/84902)
  2018-03-16 20:35 [PATCH] Fix x86 -march/-mtune after recent icelake-* split (PR target/84902) Jakub Jelinek
@ 2018-03-17 10:21 ` Uros Bizjak
  0 siblings, 0 replies; 2+ messages in thread
From: Uros Bizjak @ 2018-03-17 10:21 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Kirill Yukhin, Koval, Julia, gcc-patches

On Fri, Mar 16, 2018 at 9:23 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> We now have more than 32 enumerators in enum processor_type (well, we had
> 33 already before the icelake-* split, but the last one isn't really used),
> and while all the m_* macros were changed tom 1U<<whatever to
> HOST_WIDE_INT_1U<<whatever, the ix86_tune_mask and ix86_arch_mask
> vars weren't, so for PROCESSOR_ZNVER1 (-march=znver1 or -mtune=znver1)
> we now invoke undefined behavior, which can e.g. mean treat it like
> PROCESSOR_GENERIC, or no processor at all.
> Also, given that the m_* macros are using HOST_WIDE_INT_1U, it seems more
> consistent to use unsigned HOST_WIDE_INT type for the
> initial_ix86_*_features tables, rather than unsigned long long.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-03-16  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/84902
>         * config/i386/i386.c (initial_ix86_tune_features,
>         initial_ix86_arch_features): Use unsigned HOST_WIDE_INT rather than
>         unsigned long long.
>         (set_ix86_tune_features): Change ix86_tune_mask from unsigned int
>         to unsigned HOST_WIDE_INT, initialize to HOST_WIDE_INT_1U << ix86_tune
>         rather than 1u << ix86_tune.  Formatting fix.
>         (ix86_option_override_internal): Change ix86_arch_mask from
>         unsigned int to unsigned HOST_WIDE_INT, initialize to
>         HOST_WIDE_INT_1U << ix86_arch rather than 1u << ix86_arch.
>         (ix86_function_specific_restore): Likewise.

OK.

Thanks,
Uros.

> --- gcc/config/i386/i386.c.jj   2018-03-15 22:07:57.964242564 +0100
> +++ gcc/config/i386/i386.c      2018-03-16 18:35:53.621105345 +0100
> @@ -183,7 +183,7 @@ unsigned char ix86_tune_features[X86_TUN
>
>  /* Feature tests against the various tunings used to create ix86_tune_features
>     based on the processor mask.  */
> -static unsigned long long initial_ix86_tune_features[X86_TUNE_LAST] = {
> +static unsigned HOST_WIDE_INT initial_ix86_tune_features[X86_TUNE_LAST] = {
>  #undef DEF_TUNE
>  #define DEF_TUNE(tune, name, selector) selector,
>  #include "x86-tune.def"
> @@ -195,7 +195,7 @@ unsigned char ix86_arch_features[X86_ARC
>
>  /* Feature tests against the various architecture variations, used to create
>     ix86_arch_features based on the processor mask.  */
> -static unsigned long long initial_ix86_arch_features[X86_ARCH_LAST] = {
> +static unsigned HOST_WIDE_INT initial_ix86_arch_features[X86_ARCH_LAST] = {
>    /* X86_ARCH_CMOV: Conditional move was added for pentiumpro.  */
>    ~(m_386 | m_486 | m_PENT | m_LAKEMONT | m_K6),
>
> @@ -3310,7 +3310,7 @@ parse_mtune_ctrl_str (bool dump)
>  static void
>  set_ix86_tune_features (enum processor_type ix86_tune, bool dump)
>  {
> -  unsigned int ix86_tune_mask = 1u << ix86_tune;
> +  unsigned HOST_WIDE_INT ix86_tune_mask = HOST_WIDE_INT_1U << ix86_tune;
>    int i;
>
>    for (i = 0; i < X86_TUNE_LAST; ++i)
> @@ -3318,7 +3318,8 @@ set_ix86_tune_features (enum processor_t
>        if (ix86_tune_no_default)
>          ix86_tune_features[i] = 0;
>        else
> -        ix86_tune_features[i] = !!(initial_ix86_tune_features[i] & ix86_tune_mask);
> +       ix86_tune_features[i]
> +         = !!(initial_ix86_tune_features[i] & ix86_tune_mask);
>      }
>
>    if (dump)
> @@ -3373,7 +3374,7 @@ ix86_option_override_internal (bool main
>                                struct gcc_options *opts_set)
>  {
>    int i;
> -  unsigned int ix86_arch_mask;
> +  unsigned HOST_WIDE_INT ix86_arch_mask;
>    const bool ix86_tune_specified = (opts->x_ix86_tune_string != NULL);
>
>    const wide_int_bitmask PTA_3DNOW (HOST_WIDE_INT_1U << 0);
> @@ -4234,7 +4235,7 @@ ix86_option_override_internal (bool main
>        XDELETEVEC (s);
>      }
>
> -  ix86_arch_mask = 1u << ix86_arch;
> +  ix86_arch_mask = HOST_WIDE_INT_1U << ix86_arch;
>    for (i = 0; i < X86_ARCH_LAST; ++i)
>      ix86_arch_features[i] = !!(initial_ix86_arch_features[i] & ix86_arch_mask);
>
> @@ -5159,7 +5160,7 @@ ix86_function_specific_restore (struct g
>  {
>    enum processor_type old_tune = ix86_tune;
>    enum processor_type old_arch = ix86_arch;
> -  unsigned int ix86_arch_mask;
> +  unsigned HOST_WIDE_INT ix86_arch_mask;
>    int i;
>
>    /* We don't change -fPIC.  */
> @@ -5210,7 +5211,7 @@ ix86_function_specific_restore (struct g
>    /* Recreate the arch feature tests if the arch changed */
>    if (old_arch != ix86_arch)
>      {
> -      ix86_arch_mask = 1u << ix86_arch;
> +      ix86_arch_mask = HOST_WIDE_INT_1U << ix86_arch;
>        for (i = 0; i < X86_ARCH_LAST; ++i)
>         ix86_arch_features[i]
>           = !!(initial_ix86_arch_features[i] & ix86_arch_mask);
>
>         Jakub

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-03-17  9:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-16 20:35 [PATCH] Fix x86 -march/-mtune after recent icelake-* split (PR target/84902) Jakub Jelinek
2018-03-17 10:21 ` Uros Bizjak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).