| Message ID | B5E67142681B53468FAF6B7C3135656244118FCB@hhmail02.hh.imgtec.org |
|---|---|
| State | New |
| Headers | show |
> 2015-01-23 Robert Suchanek <robert.suchanek@imgtec.com> > > * config/mips/mips.c (mips_hard_regno_mode_ok_p): Prohibit > accumulators > for all vector modes. This seems like a genuine bug and although it can only be triggered by loongson or paired-single support it probably qualifies for fixing. My suspicion is that the switch to LRA since GCC 4.9 may be the reason this hasn't been noticed before. Reload seemed better in some cases at eliminating bad decisions from IRA so this may have simply never made it through reload by fluke. I'd like Catherine to review too since we are in stage4 without a reproducible test case. Matthew
> -----Original Message----- > From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com] > Sent: Friday, January 23, 2015 2:51 PM > To: Robert Suchanek; gcc-patches@gcc.gnu.org > Cc: Moore, Catherine > Subject: RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators > > > 2015-01-23 Robert Suchanek <robert.suchanek@imgtec.com> > > > > * config/mips/mips.c (mips_hard_regno_mode_ok_p): Prohibit > > accumulators > > for all vector modes. > > This seems like a genuine bug and although it can only be triggered by > loongson or paired-single support it probably qualifies for fixing. > My suspicion is that the switch to LRA since GCC 4.9 may be the reason this > hasn't been noticed before. Reload seemed better in some cases at > eliminating bad decisions from IRA so this may have simply never made it > through reload by fluke. > > I'd like Catherine to review too since we are in stage4 without a reproducible > test case. > The patch looks reasonable, but I'd like to see a test case that fails before we agree to include for GCC 5.0.
Hi Catherine,
> The patch looks reasonable, but I'd like to see a test case that fails before we agree to include for GCC 5.0.
It's possible to reproduce ICEs with SVN revision 212354 on mipsel-linux-gnu target. The concerned tests are
loongson-simd.c and loongson-shift-count-truncated-1.c in gcc/testsuite/gcc.target/mips.
mipsel-linux-gcc -O0 -march=loongson2f loongson-shift-count-truncated-1.c
Regards,
Robert
Matthew Fortune <Matthew.Fortune@imgtec.com> writes: >> 2015-01-23 Robert Suchanek <robert.suchanek@imgtec.com> >> >> * config/mips/mips.c (mips_hard_regno_mode_ok_p): Prohibit >> accumulators >> for all vector modes. > > This seems like a genuine bug and although it can only be triggered by > loongson or paired-single support it probably qualifies for fixing. Agreed FWIW. We shouldn't mark something as valid for a mode if even the mode's move pattern can't handle it. I think this kind of thing should go in regardless of development stage. Thanks, Richard
Richard Sandiford <rdsandiford@googlemail.com> writes: > Matthew Fortune <Matthew.Fortune@imgtec.com> writes: > >> 2015-01-23 Robert Suchanek <robert.suchanek@imgtec.com> > >> > >> * config/mips/mips.c (mips_hard_regno_mode_ok_p): Prohibit > >> accumulators > >> for all vector modes. > > > > This seems like a genuine bug and although it can only be triggered by > > loongson or paired-single support it probably qualifies for fixing. > > Agreed FWIW. We shouldn't mark something as valid for a mode if even > the mode's move pattern can't handle it. > > I think this kind of thing should go in regardless of development stage. Given that it was one of the pre-existing tests that failed I'm happy that we are covering this issue. All of these LRA related issues are likely to phase in and out with subtle changes to code-gen so I don't think we can always get a test case that fails on trunk. Since Catherine asked for further info then I will leave her to say if she is happy to accept on this basis. Matthew
> -----Original Message----- > From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com] > Sent: Tuesday, January 27, 2015 7:19 AM > To: Richard Sandiford > Cc: Robert Suchanek; gcc-patches@gcc.gnu.org; Moore, Catherine > Subject: RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators > > Richard Sandiford <rdsandiford@googlemail.com> writes: > > Matthew Fortune <Matthew.Fortune@imgtec.com> writes: > > >> 2015-01-23 Robert Suchanek <robert.suchanek@imgtec.com> > > >> > > >> * config/mips/mips.c (mips_hard_regno_mode_ok_p): Prohibit > > >> accumulators > > >> for all vector modes. > > > > > > This seems like a genuine bug and although it can only be triggered > > > by loongson or paired-single support it probably qualifies for fixing. > > > > Agreed FWIW. We shouldn't mark something as valid for a mode if even > > the mode's move pattern can't handle it. > > > > I think this kind of thing should go in regardless of development stage. > > Given that it was one of the pre-existing tests that failed I'm happy that we > are covering this issue. All of these LRA related issues are likely to phase in > and out with subtle changes to code-gen so I don't think we can always get a > test case that fails on trunk. > That's true. > Since Catherine asked for further info then I will leave her to say if she is > happy to accept on this basis. > I withdraw my request for a testcase. Catherine
> > Since Catherine asked for further info then I will leave her to say if she > is > > happy to accept on this basis. > > > > I withdraw my request for a testcase. > > Catherine Committed as r220200. Regards, Robert
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 443a712..1733457 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -12131,7 +12131,9 @@ mips_hard_regno_mode_ok_p (unsigned int regno, machine_mode mode) return size >= MIN_UNITS_PER_WORD && size <= UNITS_PER_FPREG; } + /* Don't allow vector modes in accumulators. */ if (ACC_REG_P (regno) + && !VECTOR_MODE_P (mode) && (INTEGRAL_MODE_P (mode) || ALL_FIXED_POINT_MODE_P (mode))) { if (MD_REG_P (regno))