This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH,rs6000] PR79395: Fix compile error with -mcpu=power9 and -mno-vsx and __builtin_vec_cmpne_p
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Kelvin Nilsen <kdnilsen at linux dot vnet dot ibm dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 28 Feb 2017 19:12:15 -0600
- Subject: Re: [PATCH,rs6000] PR79395: Fix compile error with -mcpu=power9 and -mno-vsx and __builtin_vec_cmpne_p
- Authentication-results: sourceware.org; auth=none
- References: <b210c36f-5f45-09ba-fb1c-a6364851eeae@linux.vnet.ibm.com>
Hi Kelvin,
On Tue, Feb 28, 2017 at 03:46:20PM -0700, Kelvin Nilsen wrote:
> PR 79395 reports a problem that arises when the preprocessor believes
> that the target supports Power9 but the gcc compiler believes that
> Power9 is not supported.
>
> This patch addresses this inconsistency by introducing a new
> preprocessor macro named __POWER9_VECTOR__ which is automatically
> defined if the current gcc configuration, as adjusted by gcc command
> line options, supports Power9. Previously, certain macro definitions
> that were supplied in altivec.h were conditioned upon the _ARCH_PWR9
> macro, which represents statically whether the compiler can support
> Power9, but ignores any command-line options that might disable the
> Power9 support in this run of the compiler. Also addressed in this
> patch is elimination of the xvcmpnesp and xvcmpnedp instructions, which
> are not currently supported.
I don't like this much at all. We should not have command line options
to disable random instructions. In a sane world we could just test for
_ARCH_PWR9 and VSX instead of having this error-prone combinatorial
explosion of macros.
> This patch has been demonstrated to fix the problems identified in the
> test case mentioned in the PR 79395 report.
>
> This patch has been bootstrapped and tested on
> powerpc64le-unknown-linux with no regressions.
>
> Is this ok for trunk?
Okay. Some trivial comments:
> PR target/79395
> * config/rs6000/altivec.h (vec_ctz and others): Change the
> preprocessor macro that controls conditional compilation from
> _ARCH_PWR9 to __POWER9_VECTOR.
It is __POWER9_VECTOR__.
> (vec_all_ne): Change macro definition to use a power9-specific
> expansion under #ifdef __POWER9_VECTOR CONTROL (instead of
> _ARCH_PWR9 control).
Same; and you don't want to SHOUT control I think ;-)
> * config/rs6000/rs6000-c.c (rs6000_target_modify_macros): Add
> support for predefined __POWER9_VECTOR__ macro to indicate that
> Power9 instruction selection is enabled.
> (altivec_overloaded_builtins): Remove extraneous
> ALTIVEC_BUILTIN_VEC_CMPNE entry for overloaded
Trailing space.
> Power9 for impelmentations of vec_cmpne. Change the signature for
s/impel/imple/
> @@ -521,9 +521,9 @@ __altivec_scalar_pred(vec_all_nez,
> __altivec_scalar_pred(vec_any_eqz,
> __builtin_vec_vcmpnez_p (__CR6_LT_REV, a1, a2))
> __altivec_scalar_pred(vec_all_ne,
> - __builtin_vec_vcmpne_p (__CR6_LT, a1, a2))
> + __builtin_vec_allne_p (a1, a2))
> __altivec_scalar_pred(vec_any_eq,
> - __builtin_vec_vcmpne_p (__CR6_LT_REV, a1, a2))
> + __builtin_vec_anyeq_p (a1, a2))
> #endif
Please indent these the same as the surrounding code.
Thanks,
Segher