This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix x86 -march/-mtune after recent icelake-* split (PR target/84902)
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Kirill Yukhin <kirill dot yukhin at gmail dot com>, "Koval, Julia" <julia dot koval at intel dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Sat, 17 Mar 2018 10:19:38 +0100
- Subject: Re: [PATCH] Fix x86 -march/-mtune after recent icelake-* split (PR target/84902)
- References: <20180316202324.GJ8577@tucnak>
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