Re: [PATCH] PR target/65242, Fix powerpc abort in gen_add2_insn
On 03/11/15 08:44, David Edelsohn wrote: On Mon, Mar 9, 2015 at 7:30 PM, Michael Meissner meiss...@linux.vnet.ibm.com wrote: This bug was one I unfortunately introduced with the -mupper-regs support. If the reload pass needed to reload a PLUS operation (for example, due to using odd address with the LD/STD instructions), it would go through all of the registers you could load DImode into, and see if it is a preferred register class. This lead the compiler to believe it could do integer arithmetic in the floating point registers. This patch fixes the problem, by not allowing PLUS to be reloaded into FPR registers. I have done bootstraps and make checks on both a big endian Power7 and a little endian Power8 system, and there were no regressions. Is the patch ok to apply? I do not believe it needs to be back ported to GCC 4.9 since the -mupper-regs changes are not installed currently on that branch. [gcc] 2015-03-09 Michael Meissner meiss...@linux.vnet.ibm.com PR target/65242 * config/rs6000/rs6000.c (rs6000_preferred_reload_class): Do not allow reloads of PLUS in floating point/VSX registers. [gcc/testsuite] 2015-03-09 Michael Meissner meiss...@linux.vnet.ibm.com PR target/65242 * g++.dg/pr65242.C: New test. This is okay. What about Jeff Law's Bugzilla comment #6 to change ?m to !m in the movdi_internal64 pattern? That also seems reasonable. It doesn't matter much to me either way as long as it gets fixed :-) Avoiding floating point registers via preferred reload class is a valid approach. My only concern then would be cases where we have similar looking arithmetic and even though we no longer prefer the FP classes, we still end up selecting that problematical alternative -- say perhaps because the pseudos in question have many other uses where FP regs make sense. I know we could get into those kind of situations on the PA because of the weird way in which integer multiplies were implemented (FP unit, using FP regs) -- which could occur even when using '?' to disparage those alternatives. I'm not familiar enough with PPC implementations to know if we can get into that same situation with that port. Jeff
Re: [PATCH] PR target/65242, Fix powerpc abort in gen_add2_insn
Jeff, I completely agree. The example exposed a problematic alternative in the pattern and I would like to fix a latent problem, in addition to Mike's patch. Thanks, David On Wed, Mar 11, 2015 at 12:05 PM, Jeff Law l...@redhat.com wrote: On 03/11/15 08:44, David Edelsohn wrote: On Mon, Mar 9, 2015 at 7:30 PM, Michael Meissner meiss...@linux.vnet.ibm.com wrote: This bug was one I unfortunately introduced with the -mupper-regs support. If the reload pass needed to reload a PLUS operation (for example, due to using odd address with the LD/STD instructions), it would go through all of the registers you could load DImode into, and see if it is a preferred register class. This lead the compiler to believe it could do integer arithmetic in the floating point registers. This patch fixes the problem, by not allowing PLUS to be reloaded into FPR registers. I have done bootstraps and make checks on both a big endian Power7 and a little endian Power8 system, and there were no regressions. Is the patch ok to apply? I do not believe it needs to be back ported to GCC 4.9 since the -mupper-regs changes are not installed currently on that branch. [gcc] 2015-03-09 Michael Meissner meiss...@linux.vnet.ibm.com PR target/65242 * config/rs6000/rs6000.c (rs6000_preferred_reload_class): Do not allow reloads of PLUS in floating point/VSX registers. [gcc/testsuite] 2015-03-09 Michael Meissner meiss...@linux.vnet.ibm.com PR target/65242 * g++.dg/pr65242.C: New test. This is okay. What about Jeff Law's Bugzilla comment #6 to change ?m to !m in the movdi_internal64 pattern? That also seems reasonable. It doesn't matter much to me either way as long as it gets fixed :-) Avoiding floating point registers via preferred reload class is a valid approach. My only concern then would be cases where we have similar looking arithmetic and even though we no longer prefer the FP classes, we still end up selecting that problematical alternative -- say perhaps because the pseudos in question have many other uses where FP regs make sense. I know we could get into those kind of situations on the PA because of the weird way in which integer multiplies were implemented (FP unit, using FP regs) -- which could occur even when using '?' to disparage those alternatives. I'm not familiar enough with PPC implementations to know if we can get into that same situation with that port. Jeff
Re: [PATCH] PR target/65242, Fix powerpc abort in gen_add2_insn
On Mon, Mar 9, 2015 at 7:30 PM, Michael Meissner meiss...@linux.vnet.ibm.com wrote: This bug was one I unfortunately introduced with the -mupper-regs support. If the reload pass needed to reload a PLUS operation (for example, due to using odd address with the LD/STD instructions), it would go through all of the registers you could load DImode into, and see if it is a preferred register class. This lead the compiler to believe it could do integer arithmetic in the floating point registers. This patch fixes the problem, by not allowing PLUS to be reloaded into FPR registers. I have done bootstraps and make checks on both a big endian Power7 and a little endian Power8 system, and there were no regressions. Is the patch ok to apply? I do not believe it needs to be back ported to GCC 4.9 since the -mupper-regs changes are not installed currently on that branch. [gcc] 2015-03-09 Michael Meissner meiss...@linux.vnet.ibm.com PR target/65242 * config/rs6000/rs6000.c (rs6000_preferred_reload_class): Do not allow reloads of PLUS in floating point/VSX registers. [gcc/testsuite] 2015-03-09 Michael Meissner meiss...@linux.vnet.ibm.com PR target/65242 * g++.dg/pr65242.C: New test. This is okay. What about Jeff Law's Bugzilla comment #6 to change ?m to !m in the movdi_internal64 pattern? That also seems reasonable. Thanks, David
[PATCH] PR target/65242, Fix powerpc abort in gen_add2_insn
This bug was one I unfortunately introduced with the -mupper-regs support. If the reload pass needed to reload a PLUS operation (for example, due to using odd address with the LD/STD instructions), it would go through all of the registers you could load DImode into, and see if it is a preferred register class. This lead the compiler to believe it could do integer arithmetic in the floating point registers. This patch fixes the problem, by not allowing PLUS to be reloaded into FPR registers. I have done bootstraps and make checks on both a big endian Power7 and a little endian Power8 system, and there were no regressions. Is the patch ok to apply? I do not believe it needs to be back ported to GCC 4.9 since the -mupper-regs changes are not installed currently on that branch. [gcc] 2015-03-09 Michael Meissner meiss...@linux.vnet.ibm.com PR target/65242 * config/rs6000/rs6000.c (rs6000_preferred_reload_class): Do not allow reloads of PLUS in floating point/VSX registers. [gcc/testsuite] 2015-03-09 Michael Meissner meiss...@linux.vnet.ibm.com PR target/65242 * g++.dg/pr65242.C: New test. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
