[Patch, MIPS] Fix PR 58158 - ICE on MIPS Loongson

[Patch, MIPS] Fix PR 58158 - ICE on MIPS Loongson

[Steve Ellcey]

While looking at the outstanding GCC regressions in bugzilla I noticed PR 58158
which has a proposed fix in the comments but which doesn't seem to have ever
been checked in.  I updated the change to apply to top-of-tree sources and
tested the specific test case but I don't have a Loongson system to do a
complete test run on.  The change seems very safe though and it should be OK
for R6 as well as Loongson since '!ISA_HAS_FP_CONDMOVE' is true for R6 so using
it instead of 'ISA_HAS_SEL' should be OK and more descriptive of why we want
to fail in this case.

OK to checkin?

Steve Ellcey
sellcey@imgtec.com


2015-03-02  Steve Ellcey  <sellcey@imgtec.com>

	PR target/58158
	* config/mips/mips.md (mov<mode>cc): Change ISA_HAS_SEL check to
	!ISA_HAS_FP_CONDMOVE.


diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 2fb2786..3672c0b 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -7150,7 +7150,8 @@
 			  (match_operand:GPR 3 "reg_or_0_operand")))]
   "ISA_HAS_CONDMOVE || ISA_HAS_SEL"
 {
-  if (ISA_HAS_SEL && !INTEGRAL_MODE_P (GET_MODE (XEXP (operands[1], 0))))
+  if (!ISA_HAS_FP_CONDMOVE
+      && !INTEGRAL_MODE_P (GET_MODE (XEXP (operands[1], 0))))
     FAIL;
 
   mips_expand_conditional_move (operands);

RE: [Patch, MIPS] Fix PR 58158 - ICE on MIPS Loongson

[Matthew Fortune]

Thanks for looking through and catching this.

I had conflicting thoughts on whether the new condition should
reference both !ISA_HAS_FP_CONDMOVE || ISA_HAS_SEL but if we take it
that FP_CONDMOVE is the only way to get an integer conditional
move based on an FP condition then that's fine.

> 2015-03-02  Steve Ellcey  <sellcey@imgtec.com>
> 
> 	PR target/58158
> 	* config/mips/mips.md (mov<mode>cc): Change ISA_HAS_SEL check to
> 	!ISA_HAS_FP_CONDMOVE.

OK.

Are you planning on doing any backports of this patch to release
branches?

Thanks,
Matthew

RE: [Patch, MIPS] Fix PR 58158 - ICE on MIPS Loongson

[Steve Ellcey]

On Mon, 2015-03-02 at 15:54 -0800, Matthew Fortune wrote:
> Thanks for looking through and catching this.
> 
> I had conflicting thoughts on whether the new condition should
> reference both !ISA_HAS_FP_CONDMOVE || ISA_HAS_SEL but if we take it
> that FP_CONDMOVE is the only way to get an integer conditional
> move based on an FP condition then that's fine.
> 
> > 2015-03-02  Steve Ellcey  <sellcey@imgtec.com>
> > 
> > 	PR target/58158
> > 	* config/mips/mips.md (mov<mode>cc): Change ISA_HAS_SEL check to
> > 	!ISA_HAS_FP_CONDMOVE.
> 
> OK.
> 
> Are you planning on doing any backports of this patch to release
> branches?
> 
> Thanks,
> Matthew

Given that it is very small and safe I think backporting it would make
sense.  I will check it in on the main line today and if there are no
problems after a few days I could check it in on the 4.8 and 4.9
branches if that sounds good to you.

Steve Ellcey
sellcey@imgtec.com

RE: [Patch, MIPS] Fix PR 58158 - ICE on MIPS Loongson

[Matthew Fortune]

Steve Ellcey <Steve.Ellcey@imgtec.com> writes:
> On Mon, 2015-03-02 at 15:54 -0800, Matthew Fortune wrote:
> > Thanks for looking through and catching this.
> >
> > I had conflicting thoughts on whether the new condition should
> > reference both !ISA_HAS_FP_CONDMOVE || ISA_HAS_SEL but if we take it
> > that FP_CONDMOVE is the only way to get an integer conditional move
> > based on an FP condition then that's fine.
> >
> > > 2015-03-02  Steve Ellcey  <sellcey@imgtec.com>
> > >
> > > 	PR target/58158
> > > 	* config/mips/mips.md (mov<mode>cc): Change ISA_HAS_SEL check to
> > > 	!ISA_HAS_FP_CONDMOVE.
> >
> > OK.
> >
> > Are you planning on doing any backports of this patch to release
> > branches?
> >
> > Thanks,
> > Matthew
> 
> Given that it is very small and safe I think backporting it would make
> sense.  I will check it in on the main line today and if there are no
> problems after a few days I could check it in on the 4.8 and 4.9
> branches if that sounds good to you.

Sounds good.

Thanks,
Matthew