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
