[PATCH RFA MIPS] Prohibit vector modes in accumulators

[PATCH RFA MIPS] Prohibit vector modes in accumulators

[Robert Suchanek]

Hi,

A bug was exposed by LRA for loongson-shift-count-truncated-1.c and
loongson-simd.c with -O0 optimization.  These testcases were ICEing
with 'Max. number of generated reloads insns per insn is achieved (90)'
error.

The problem appears to be with vector modes where contents of memory
is meant to be copied into GPRs but LRA chose to put them into accumulators
first and started inserting reloads to move the values into GPRs.

In both cases, IRA assigned the alternative GR_AND_MD0_REGS class to
allocnos.  Since accumulators appear first in REG_ALLOC_ORDER macro,
LRA picks LO/HI registers even if it's more costly than GPRs and wants
to insert reloads to move in/out values to/from accumulators for which
we don't have patterns i.e. to handle (set (reg:V2SI ) (mem:V2SI ...)).

The fix is to prohibit the use of accumulator registers for vector modes.

Unfortunately, I have no testcase exposing this on the trunk but it seems
reasonable to prohibit vectors in accumulators anyway.

I'm not sure if this patch would go to the trunk now and be queued for
Stage 1.

Regards,
Robert

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.
---
 gcc/config/mips/mips.c | 2 ++
 1 file changed, 2 insertions(+)

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))
-- 
2.2.2

RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators

[Matthew Fortune]

> 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

RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators

[Moore, Catherine]

> -----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. 

RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators

[Robert Suchanek]

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

Re: [PATCH RFA MIPS] Prohibit vector modes in accumulators

[Richard Sandiford]

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

RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators

[Matthew Fortune]

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

RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators

[Moore, Catherine]

> -----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

RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators

[Robert Suchanek]

> > 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