Hi Bill,
On Sun, Jan 29, 2017 at 04:17:03PM -0600, Bill Schmidt wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79268 reports that the
> vec_xl and vec_xst intrinsics are being compiled to incorrectly use
> element-reversing loads and stores in some cases, and not accepting
> pointers to vector pixel as a legal parameter. This arose because
> vec_xl and vec_xst were incorrectly redefined in altivec.h to be
> equivalent to these instructions; the intent was to use them to
> implement vec_xl_be and vec_xst_be instead. I.e., I goofed...
I missed it as well :-(
> This affects GCC 6 and GCC 7. This patch fixes the critical issue by
> restoring vec_xl and vec_xst to their former definitions, and adding a
> test case to ensure that vector pixel is covered by them. It also
> deletes the four test cases that were added as part of the errant patch.
> This is a simple patch that is intended for application to trunk and
> backport to GCC 6.
>
> A subsequent patch will address defining vec_xl_be and vec_xst_be in
> terms of the element-reversing memory accesses, which will require more
> extensive changes. This may or may not be deemed worth backporting, but
> I think that decision should be made independently.
Yeah.
> This first patch will be provided to the various distros to fix and/or
> prevent build problems using GCC 6 on packages that use vec_xl. At
> least one such package is known to exist.
>
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions. Regstrap is in process for the GCC 6 version of the patch,
> which applies cleanly with offsets. Is this ok for trunk, and shortly
> thereafter for GCC 6?
Okay for both. A question and a pedant remark about the new testcase.
> --- gcc/testsuite/gcc.target/powerpc/pr79268.c (revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/pr79268.c (working copy)
> @@ -0,0 +1,18 @@
> +/* { dg-do compile { target { powerpc64le-*-* } } } */
Is that correct? Shouldn't it be target powerpc*-*-* and use -m64 -mlittle
-mabi=elfv2 ? Or use some LE selector, LP64, etc.
Target doesn't mean very much.
Does it not run fine on BE / elfv1 anyway?
> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
> "-mcpu=power8" } } */
> +/* { dg-options "-mcpu=power8 -O3 " } */
Stray space before the closing quote... Harmless of course.
Thanks,
Segher
> 2017-01-29 Bill Schmidt <wschm...@linux.vnet.ibm.com>
>
> PR target/79268
> * config/rs6000/altivec.h (vec_xl): Revise #define.
> (vec_xst): Likewise.
>
> [gcc/testsuite]
>
> 2017-01-29 Bill Schmidt <wschm...@linux.vnet.ibm.com>
>
> PR target/79268
> * gcc.target/powerpc/pr79268.c: New file.
> * gcc.target/powerpc/vsx-elemrev-1.c: Delete file.
> * gcc.target/powerpc/vsx-elemrev-2.c: Likewise.
> * gcc.target/powerpc/vsx-elemrev-3.c: Likewise.
> * gcc.target/powerpc/vsx-elemrev-4.c: Likewise.
