Created attachment 23444 [details] testcase Tracked down an issue in cpu2006 benchmark 456.hmmer where incorrect code is generated when spilling a vector register. The normal floating point stfd/lfd insns are being generated, which only spills half the vector (allowing the other half to get clobbered across a function call). Attatched testcase was compiled with: gcc -S -m64 -O3 -mcpu=power7 -mveclibabi=mass -ffast-math spill.c Spill is occuring in the vectorized loop with expd2() calls. A snippet of that loop showing the use vr11 as vector and incorrect spill follows. There are a few other regs spilled in similar fashion in the loop. xvmuldp 11,58,12 xvmuldp 34,57,12 xvadddp 10,63,60 stfd 11,144(1) xvadddp 53,53,10 bl expd2 nop lfd 11,144(1) xxlor 62,34,34 xxlor 34,11,11 xvmuldp 57,57,62 bl expd2
Looks like the bogus spill/restore insns are being inserted via caller-save.c routines since IRA assigned volatile regs to pseudos which are live across a call. 190.sched1 dump for the example above (pseudo 352). (insn 180 177 182 12 (set (reg:V2DF 352 [ vect_var_.59 ]) (mult:V2DF (reg:V2DF 343 [ vect_var_.53 ]) (reg:V2DF 350 [ vect_cst_.60 ]))) spill.c:21 766 {*vsx_mulv2df3} (nil)) ... call expd2() (insn 185 184 186 12 (set (reg:V2DF 79 2) (reg:V2DF 352 [ vect_var_.59 ])) spill.c:21 756 {*vsx_movv2df} (expr_list:REG_DEAD (reg:V2DF 352 [ vect_var_.59 ]) (nil))) 191.ira dump, along with some additional info I dumped in caller-save:insert_one_insn() to show insn being inserted Popping a139(r352,l0) -- assign reg 43 (vector save) Inserting insn ... (set (mem/c:DI (plus:DI (reg/f:DI 113 sfp) (const_int 144 [0x90])) [9 S8 A64]) (reg:DI 43 11)) (vector restore) Inserting insn ... (set (reg:DI 43 11) (mem/c:DI (plus:DI (reg/f:DI 113 sfp) (const_int 144 [0x90])) [9 S8 A64])) ... before ... (insn 185 184 186 12 (set (reg:V2DF 79 2) (reg:V2DF 43 11 [orig:352 vect_var_.59 ] [352])) spill.c:21 756 {*vsx_movv2df} (expr_list:REG_DEAD (reg:V2DF 43 11 [orig:352 vect_var_.59 ] [352]) (nil))) ---final ira dump (insn 180 177 182 12 (set (reg:V2DF 43 11 [orig:352 vect_var_.59 ] [352]) (mult:V2DF (reg:V2DF 103 26 [orig:343 vect_var_.53 ] [343]) (reg:V2DF 44 12 [orig:350 vect_cst_.60 ] [350]))) spill.c:21 766 {*vsx_mulv2df3} (nil)) (insn 323 322 324 12 (set (mem/c:DI (plus:DI (reg/f:DI 1 1) (const_int 144 [0x90])) [9 S8 A64]) (reg:DI 43 11)) spill.c:21 405 {*movdi_internal64} (nil)) ... call expd2() (insn 326 184 185 12 (set (reg:DI 43 11) (mem/c:DI (plus:DI (reg/f:DI 1 1) (const_int 144 [0x90])) [9 S8 A64])) spill.c:21 405 {*movdi_internal64} (nil)) (insn 185 326 186 12 (set (reg:V2DF 79 2) (reg:V2DF 43 11 [orig:352 vect_var_.59 ] [352])) spill.c:21 756 {*vsx_movv2df} (nil))
The DI mode for the spill locations is being decided in caller-save.c:init_caller_save(), which appears to decide what mode should be used when a caller-saved reg needs to be saved/restored (regno_save_mode[][]). This is done up front, without regard to what mode is actually in a register at the time of its use. The volatile floating point regs end up with a DI mode, the volatile Altivec regs get V2DF mode.
The following fixes the problem by changing the save mode for FP regs to V2DF mode for TARGET_VSX. But I have questions/concerns on this that need more digging: - I believe this will affect all FP reg saves, which will be overkill for SF/DF modes since we'll be allocating 16 bytes and doing full vector store/load. Can we modify save/restore code on the fly to use the appropriate size? - Any issues wrt stack slot reuse (i.e. can't reuse smaller slot for vector save)? Index: gcc/config/rs6000/rs6000.h =================================================================== --- gcc/config/rs6000/rs6000.h (revision 170438) +++ gcc/config/rs6000/rs6000.h (working copy) @@ -1005,6 +1005,12 @@ extern unsigned rs6000_pointer_size; #define HARD_REGNO_NREGS(REGNO, MODE) rs6000_hard_regno_nregs[(MODE)][(REGNO)] +/* Ensure vector modes are handled correctly in FP regs for VSX */ +#define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \ + (TARGET_VSX && FP_REGNO_P (REGNO) \ + ? V2DFmode \ + : choose_hard_reg_mode ((REGNO), (NREGS), false)) + #define HARD_REGNO_CALL_PART_CLOBBERED(REGNO, MODE) \ (((TARGET_32BIT && TARGET_POWERPC64 \ && (GET_MODE_SIZE (MODE) > 4) \
Author: pthaugen Date: Mon Mar 7 19:27:09 2011 New Revision: 170748 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170748 Log: PR target/47862 * config/rs6000/rs6000.h (HARD_REGNO_CALLER_SAVE_MODE): Define. * config/rs6000/e500.h (HARD_REGNO_CALLER_SAVE_MODE): Undefine before definition. * testsuite/gcc.target/powerpc/pr47862.c: New. Added: trunk/gcc/testsuite/gcc.target/powerpc/pr47862.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/e500.h trunk/gcc/config/rs6000/rs6000.h trunk/gcc/testsuite/ChangeLog
Author: pthaugen Date: Mon Mar 7 19:40:15 2011 New Revision: 170749 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170749 Log: PR target/47862 * config/rs6000/rs6000.h (HARD_REGNO_CALLER_SAVE_MODE): Define. * gcc.target/powerpc/pr47862.c: New. Added: branches/gcc-4_5-branch/gcc/testsuite/gcc.target/powerpc/pr47862.c Modified: branches/gcc-4_5-branch/gcc/ChangeLog branches/gcc-4_5-branch/gcc/config/rs6000/rs6000.h branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
Author: pthaugen Date: Mon Mar 7 19:47:03 2011 New Revision: 170750 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170750 Log: PR target/47862 * config/rs6000/rs6000.h (HARD_REGNO_CALLER_SAVE_MODE): Define. * gcc.target/powerpc/pr47862.c: New. Added: branches/ibm/gcc-4_5-branch/gcc/testsuite/gcc.target/powerpc/pr47862.c Modified: branches/ibm/gcc-4_5-branch/gcc/ChangeLog.ibm branches/ibm/gcc-4_5-branch/gcc/config/rs6000/rs6000.h branches/ibm/gcc-4_5-branch/gcc/testsuite/ChangeLog.ibm
Fixed.
Author: pthaugen Date: Tue Mar 15 19:43:38 2011 New Revision: 171016 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=171016 Log: PR target/47862 * caller-save.c (insert_restore, insert_save): Use non-validate form of adjust_address. Modified: branches/ibm/gcc-4_5-branch/gcc/ChangeLog.ibm branches/ibm/gcc-4_5-branch/gcc/caller-save.c
Author: pthaugen Date: Wed Mar 16 20:19:14 2011 New Revision: 171072 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=171072 Log: PR target/47862 * caller-save.c (insert_restore, insert_save): Use non-validate form of adjust_address. Modified: branches/gcc-4_5-branch/gcc/ChangeLog branches/gcc-4_5-branch/gcc/caller-save.c