An earlier change introduced a think-o in the altivec capability check, allowing a false positive if the compiler supported altivec but the hardware did not. Reproducible: Always Steps to Reproduce: 1. Run "make regtest" on power 5 platform 2. 3. Actual Results: Altivec tests run and fail Expected Results: Should correctly detect that altivec support does not exist on the platform and not run the altivec tests
Created attachment 100973 [details] Fix false positive check for altivec support on PPC machines The attached patch was submitted directly to me from Will Schmidt. I have reviewed and tested the patch. It looks fine. Will post patch to mailing list for community review as it touches the general configure.ac file.
Comment on attachment 100973 [details] Fix false positive check for altivec support on PPC machines Can't really comment on this (autoconf is somewhat mysterious to me) but I guess this is ok.
You are changing the AC_DEFINE from HAS_ALTIVEC to COMPILER_SUPPORTS_ALTIVEC. But COMPILER_SUPPORTS_ALTIVEC is never used as C preprocessor symbol, so I think you should just remove it. HAS_ALTIVEC is used as an C preprocessor symbol in none/tests/ppc32/jm-insns.c but there it is defined in the Makefile depending on the AM_CONDITIONAL HAS_ALTIVEC. So that should be fine (although the comment in the code implies it comes from config.h, which isn't true anymore).
Created attachment 101056 [details] updated patch Refreshed the patch per comments and feedback.
(In reply to Mark Wielaard from comment #3) > You are changing the AC_DEFINE from HAS_ALTIVEC to COMPILER_SUPPORTS_ALTIVEC. > But COMPILER_SUPPORTS_ALTIVEC is never used as C preprocessor symbol, so I > think you should just remove it. Yup, agreed, done. Thanks for the review. > HAS_ALTIVEC is used as an C preprocessor symbol in > none/tests/ppc32/jm-insns.c but there it is defined in the Makefile > depending on the AM_CONDITIONAL HAS_ALTIVEC. So that should be fine Yup, .. I had added the AM_CONDITIONAL way back in 15424, but I obviously missed removing the old definition, which caused the problem.. > (although the comment in the code implies it comes from config.h, which > isn't true anymore).
Thanks, pushed as valgrind svn r15952
Patch has been committed. Closing.