Re: [PATCH] rs6000: Fix fsel pattern (PR79211)

[Segher Boessenkool]

On Fri, Jan 27, 2017 at 03:11:46PM +, Segher Boessenkool wrote:
> 2017-01-27  Segher Boessenkool  
> 
>   * config/rs6000/rs6000.md (*fsel4): Use
>   gpc_reg_operand instead of fpr_reg_operand.

I committed the patch.


Segher

Re: [PATCH] rs6000: Fix fsel pattern (PR79211)

[Segher Boessenkool]

On Fri, Jan 27, 2017 at 05:36:10PM -0500, Michael Meissner wrote:
> On Fri, Jan 27, 2017 at 03:11:46PM +, Segher Boessenkool wrote:
> > The fsel define_insn uses fpr_reg_operand for its predicates.  This
> > won't work because passes can put a hard register in the operands: in
> > the testcase, combine likes to forward the parameter registers to what
> > then is still an smin, and then split1 uses "*s3_fpr"
> > (which has gpc_reg_operand).  And then we have a GPR in the operand,
> > which does not match fpr_reg_operand.
> > 
> > It seems to me the predicates should be gpc_reg_operand here as well.
> > This patch changes that.  Mike, does this look correct?  Was there a
> > reason it used fpr_reg_operand?
> > 
> > Bootstrapped and tested on powerpc64-linux.
> 
> I dunno, we need to look at why somebody is putting a GPR in there in the 
> first
> place.  The instruction doesn't support GPRs.  Perhaps the generator function
> should do a copy before doing the call.

This is the same for *all* other FP insns though!

I looked if perhaps this pattern would hide another one, but the xxsel
pattern uses a different mode (V2DI), so it cannot conflict.


Segher

Re: [PATCH] rs6000: Fix fsel pattern (PR79211)

[Michael Meissner]

On Fri, Jan 27, 2017 at 03:11:46PM +, Segher Boessenkool wrote:
> The fsel define_insn uses fpr_reg_operand for its predicates.  This
> won't work because passes can put a hard register in the operands: in
> the testcase, combine likes to forward the parameter registers to what
> then is still an smin, and then split1 uses "*s3_fpr"
> (which has gpc_reg_operand).  And then we have a GPR in the operand,
> which does not match fpr_reg_operand.
> 
> It seems to me the predicates should be gpc_reg_operand here as well.
> This patch changes that.  Mike, does this look correct?  Was there a
> reason it used fpr_reg_operand?
> 
> Bootstrapped and tested on powerpc64-linux.

I dunno, we need to look at why somebody is putting a GPR in there in the first
place.  The instruction doesn't support GPRs.  Perhaps the generator function
should do a copy before doing the call.

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

[PATCH] rs6000: Fix fsel pattern (PR79211)

[Segher Boessenkool]

The fsel define_insn uses fpr_reg_operand for its predicates.  This
won't work because passes can put a hard register in the operands: in
the testcase, combine likes to forward the parameter registers to what
then is still an smin, and then split1 uses "*s3_fpr"
(which has gpc_reg_operand).  And then we have a GPR in the operand,
which does not match fpr_reg_operand.

It seems to me the predicates should be gpc_reg_operand here as well.
This patch changes that.  Mike, does this look correct?  Was there a
reason it used fpr_reg_operand?

Bootstrapped and tested on powerpc64-linux.


Segher


2017-01-27  Segher Boessenkool  

* config/rs6000/rs6000.md (*fsel4): Use
gpc_reg_operand instead of fpr_reg_operand.

---
 gcc/config/rs6000/rs6000.md | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 3f29221..f32b381 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -4946,12 +4946,12 @@ (define_expand "movcc"
 }")
 
 (define_insn "*fsel4"
-  [(set (match_operand:SFDF 0 "fpr_reg_operand" "=&")
+  [(set (match_operand:SFDF 0 "gpc_reg_operand" "=&")
(if_then_else:SFDF
-(ge (match_operand:SFDF2 1 "fpr_reg_operand" "")
+(ge (match_operand:SFDF2 1 "gpc_reg_operand" "")
 (match_operand:SFDF2 4 "zero_fp_constant" "F"))
-(match_operand:SFDF 2 "fpr_reg_operand" "")
-(match_operand:SFDF 3 "fpr_reg_operand" "")))]
+(match_operand:SFDF 2 "gpc_reg_operand" "")
+(match_operand:SFDF 3 "gpc_reg_operand" "")))]
   "TARGET__FPR && TARGET_PPC_GFXOPT"
   "fsel %0,%1,%2,%3"
   [(set_attr "type" "fp")])
-- 
1.9.3