| Message ID | e0039e02-1a23-cdad-341c-187f1460f4f6@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 22 May 2017, Bill Schmidt wrote: > Hi, > > The subject test requires little endian, but the target selector doesn't > specify this. This patch fixes that. Tested on BE/LE P8 systems, > committed as obvious. It's not obvious; it's wrong. You can configure for a BE default while supporting LE, using appropriate --enable-targets options. That's what config/rs6000/t-linux64bele is for, for example. In such cases, this test should be enabled when testing LE multilibs. Likewise, you can configure for LE default while supporting BE, and then the test should be disabled when testing BE multilibs. For any target property that can depend on the multilib, you have to use a general target pattern such as powerpc*-*-*, then test effective-target keywords for any per-multilib properties that are relevant (such as endianness in this case). Testing for BE/LE, or for 32-bit/64-bit, via target triplet patterns, is always wrong in powerpc tests, just as it's always wrong for x86 tests to support one of x86_64 and i?86 but not the other (those should similarly use effective-targets as needed).
On Mon, May 22, 2017 at 08:14:13PM +0000, Joseph Myers wrote: > > The subject test requires little endian, but the target selector doesn't > > specify this. This patch fixes that. Tested on BE/LE P8 systems, > > committed as obvious. > > It's not obvious; it's wrong. You can configure for a BE default while > supporting LE, using appropriate --enable-targets options. That's what > config/rs6000/t-linux64bele is for, for example. In such cases, this test > should be enabled when testing LE multilibs. Likewise, you can configure > for LE default while supporting BE, and then the test should be disabled > when testing BE multilibs. > > For any target property that can depend on the multilib, you have to use a > general target pattern such as powerpc*-*-*, then test effective-target > keywords for any per-multilib properties that are relevant (such as > endianness in this case). Testing for BE/LE, or for 32-bit/64-bit, via > target triplet patterns, is always wrong in powerpc tests, just as it's > always wrong for x86 tests to support one of x86_64 and i?86 but not the > other (those should similarly use effective-targets as needed). Yes, all this is true. But the patch is an obvious improvement, and in line with the many other tests where we have this test for powerpc64le-*-*. As far as I know no one tests LE on a BE system, and the other way around. But yes you are right, it should be supported, just like powerpc64 on a non-powerpc64 system (and importantly, the other way around), for which we currently use lp64 (which isn't exactly right), except of course the tests that do it completely wrong :-/ Segher
Index: gcc/testsuite/gcc.target/powerpc/p8-vec-xl-xst.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/p8-vec-xl-xst.c (revision 248347) +++ gcc/testsuite/gcc.target/powerpc/p8-vec-xl-xst.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target { powerpc*-*-* } } } */ +/* { dg-do compile { target { powerpc64le-*-* } } } */ /* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */ /* { dg-require-effective-target powerpc_p8vector_ok } */ /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */