$ gcc/xgcc -B gcc/ ../gcc/gcc/testsuite/gcc.target/powerpc/p8vector-ldst.c -O2 -S -mcpu=power8 -mupper-regs-df -mupper-regs-sf -m32 && grep -c lxsdx p8vector-ldst.s 0 1e5911a98f7733c3263a6c9dee5ab3ff06dbf57c is the first bad commit commit 1e5911a98f7733c3263a6c9dee5ab3ff06dbf57c Author: vmakarov <vmakarov@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Fri May 16 17:37:17 2014 +0000 2014-05-16 Vladimir Makarov <vmakarov@redhat.com> PR rtl-optimization/60969 * ira-costs.c (record_reg_classes): Allow only memory for pseudo. Calculate costs for this case.
Also in 4.9.1, regressed from 4.9.0.
Sorry, I have [vmakarov@gcc2-power8 gcc]$ ./xgcc -B./ ../../gcc/gcc/testsuite/gcc.target/powerpc/p8vector-ldst.c -O2 -S -mcpu=power8 -mupper-regs-df -mupper-regs-sf -m32 && grep -c lxsdx p8vecto\ r-ldst.s ../../gcc/gcc/testsuite/gcc.target/powerpc/p8vector-ldst.c:1:0: error: -mupper-regs-sf requires -mpower8-vector /* { dg-do compile { target { powerpc*-*-* && lp64 } } } */ ^ I suppose the test should not work with this set of options as make RUNTESTFLAGS='powerpc.exp=p8vector-ldst.c' check-gcc gives Running /home/vmakarov/build/trunk/gcc/gcc/testsuite/gcc.target/powerpc/powerpc.exp ... === gcc Summary === # of unsupported tests 1 /home/vmakarov/build/trunk/build/gcc/xgcc version 5.0.0 20150213 (experimental) (GCC)
c9f03f9b6e7a888a270638c07190513189f8c33d
I put a LP64 on the test, because it was using 64-bit shifts in order to force registers to be allocated from the Altivec register set. If you compile it in 32-bit mode, the emulation of 64-bit shifts/masks can complicate things.
So why exactly are we tracking a regression in a testcase that isn't meant to be compiled there at all (is undefined behavior with -m32)? Furthermore, this ICEs for me with older gcc versions (say around r208000) and errors out in recent gcc versions: -mupper-regs-sf requires -mpower8-vector
It wasn't that way when reported.
Created attachment 34859 [details] Proposed patch. I may be missing something but, AFAICS, simply changing the type of the constants to long long as in the attached patch makes the test pass in both 32-bit and 64-bit modes. $ /build/gcc-trunk/gcc/xgcc -B/build/gcc-trunk/gcc -Wall ~/fsf/gcc/trunk/gcc/testsuite/gcc.target/powerpc/p8vector-ldst.c -O2 -S -mcpu=power8 -mupper-regs-df -mupper-regs-sf -m32 && for insn in lxsspx lxsdx stxsspx stxsdx xsaddsp xsadddp; do if ! grep -q $insn p8vector-ldst.s ; then echo $insn; fi; done | wc -l 0 $ make -C /build/gcc-trunk RUNTESTFLAGS='powerpc.exp=p8vector-ldst.c' check-c ... === gcc Summary === # of expected passes 7 /build/gcc-trunk/gcc/xgcc version 5.0.0 20150221 (experimental) (GCC)
I added the lp64 test when I added the -mupper-regs support and rewrote the test. The rationale is I was using a long bit vector to make sure that each floating point variable got used (in_mask and out_mask each have a bit set for the 40 floating point values, to make sure that each value gets set in ways that the optimizer can't rearrange things and use less live variables). If we use long long in doing the masking in 32-bit, we run the risk that you have to call a function to do long long shifting, and/or, and testing. So, I added the && lp64 conditional to the test. The original version of p8vector-ldst.c no longer works with the -mupper-regs support, as the compiler will happily move variables into the appropriate register class without keeping the register there normally. So, I wrote the test to have 40 live FP values. If people feel strongly enough that it needs to work on big endian 32-bit (little endian powerpc64 no longer supports 32-bit mode), we can rewrite the test to have 2 separate in_masks and out_masks. Note as of this writing, the 4.9 branch has the original version of the p8vector-ldst.c (and no restriction on 32-bit), since the -mupper-regs patches are not back ported to GCC 4.9. If the back port of the patches for upper regs for 4.9 that I have in a branch are applied to the official FSF 4.9 branch, then the new version of p8vector-ldst.c will be installed.
It seems to me Andreas has a valid point in that the generated for that test could be improved for the set of options in c#0. He also has a point that apparently there was a period of time when we did generate better code. And while the code exhibits undefined behaviour for -m32, unless someone shows that undefined behaviour is a factor in the difference in quality of code regression Andreas saw I would suggest we simply ignore the undefinedness of that part of the testcase. So it seems to me that a regression marker is appropriate. However, it's hard to see it as worthy of a P2 status. So I'm going to downgrade to P4.
However, before I made the changes that finished the implementation of upper regs support, the option was not functional for anything other than power8-ldst.c. It could not be used for any real program that involved reloading for the GCC 4.8 and 4.9 releases. The original power8-ldst.c was there just to make sure the very minimal support that allowed DFmode to go into the Altivec registers would still work. When I picked up the work that went into the 5.0 compiler, the test case no longer guaranteed that the register was allocated to an Altivec vector register and that memory accesses to the vector register now tended to involve a copy to/from the traditional floating point registers. So, I rewrote the case, to work with the final code.
Would you prefer RESOLVED/NOTABUG? Always happy to do that! Your call.
Created attachment 34865 [details] Patch that tests the desired functionality for both 32-bit and 64-bit
GCC 5.1 has been released.
GCC 5.2 is being released, adjusting target milestone to 5.3.
GCC 5.3 is being released, adjusting target milestone.
Mike, did your patch go upstream, or is there more work to do here?
No, it does not appear the patch went upstream (either on trunk or gcc 5).
I've applied Mike's test case patch to GCC 5 and GCC 6.