[committed] Fix v850e3v5 recipf and rsqrt issues

Message ID 38a0ae51-d865-ed16-ca07-e5a55abbfc96@redhat.com
State New
Headers show
Series
  • [committed] Fix v850e3v5 recipf and rsqrt issues
Related show

Commit Message

Jeff Law June 25, 2018, 11:44 p.m.
Code in optabs.c for directly expanding some operations checks the
number of operands in the expander/named pattern against the expected
number of operands.  If they do not match an ICE is triggered.


/* Try to generate instruction ICODE, using operands [OPS, OPS + NOPS)
   as its operands.  Return the instruction pattern on success,
   and emit any necessary set-up code.  Return null and emit no
   code on failure.  */

rtx_insn *
maybe_gen_insn (enum insn_code icode, unsigned int nops,
                struct expand_operand *ops)
{
  gcc_assert (nops == (unsigned int) insn_data[(int)
icode].n_generator_args);


[ ... ]


On a target with recip or rsqrt patterns written in the expected way
we'll trigger the ICE because those patterns expose the recip as a
division with 1.0 as an operand, thus creating a second, unexpected
operand  This showed up testing v850e3v5.

It looks like some targets hide things a bit behind an unspec, possibly
to avoid this problem.  This patch does something very similar for the v850:


(define_expand "rsqrtsf2"
  [(set (match_operand:SF 0 "register_operand" "=")
        (unspec:SF [(match_operand:SF 1 "register_operand" "")]
                   UNSPEC_RSQRT))]

We then generate a more normal looking pattern from the expander which
matches:


(define_insn "rsqrtsf2_insn"
  [(set (match_operand:SF 0 "register_operand" "=r")
        (div:SF (match_operand:SF 1 "const_float_1_operand" "")
                (sqrt:SF (match_operand:SF 2 "register_operand" "r"))))]


While looking at this I noticed that const_float_1_operand would
essentially reject everything.  It had a toplevel check that the operand
was a CONST_INT.  But in the body of the predicate it would reject
anything that was not a CONST_DOUBLE.

The right code is CONST_DOUBLE.  So this patch also fixes
const_float_1_operand.

This fixes a handful of ICEs when testing for v850e3v5 (pr41963,
pr46728-9.  It also results in many recipf instructions being generated
for newlib v850e3v5 multilib whereas none were generated before.
There's no uses of rsqrt in newlib, but they do show up in the gcc
testsuite.


Installing on the trunk,

Jeff

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index f74b2ecaa4d..b5f073de663 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,16 @@ 
+2018-06-25  Jeff Law  <law@redhat.com>
+
+	* config/v850/predicates.md (const_float_1_operand): Fix match_code
+	test.
+	(const_float_0_operand): Remove unused predicate.
+	* config/v850/v850.md (define_constants): Remove UNSPEC_LOOP.
+	(define_c_enum unspec): Add LOOP, RCP and RSQRT constants.
+	(recipsf2): New expander.  Original pattern now called
+	(recipsf2_insn).
+	(recipdf2, recipdf2_insn): Similarly.
+	(rsqrtsf2, rsqrtsf2_insn): Similarly
+	(rsqrtdf2, rsqrtdf2_insn): Similarly
+
 2018-06-26  Gerald Pfeifer  <gerald@pfeifer.com>
 
 	* ginclude/stddef.h: Remove an obsolete comment on FreeBSD 5.
diff --git a/gcc/config/v850/predicates.md b/gcc/config/v850/predicates.md
index 1b50e50b8c3..68390a23eb7 100644
--- a/gcc/config/v850/predicates.md
+++ b/gcc/config/v850/predicates.md
@@ -475,7 +475,7 @@ 
 ;; Return true if OP is a float value operand with value as 1.
 
 (define_predicate "const_float_1_operand"
-  (match_code "const_int")
+  (match_code "const_double")
 {
   if (GET_CODE (op) != CONST_DOUBLE
       || mode != GET_MODE (op)
@@ -485,19 +485,6 @@ 
   return op == CONST1_RTX(mode);
 })
 
-;; Return true if OP is a float value operand with value as 0.
-
-(define_predicate "const_float_0_operand"
-  (match_code "const_int")
-{
-  if (GET_CODE (op) != CONST_DOUBLE
-      || mode != GET_MODE (op)
-      || (mode != DFmode && mode != SFmode))
-    return 0;
-
-  return op == CONST0_RTX(mode);
-})
-
 (define_predicate "label_ref_operand"
   (match_code "label_ref")
 )
diff --git a/gcc/config/v850/v850.md b/gcc/config/v850/v850.md
index e01a3102c31..67d906329e8 100644
--- a/gcc/config/v850/v850.md
+++ b/gcc/config/v850/v850.md
@@ -44,10 +44,15 @@ 
    (LP_REGNUM       		31)         ; Return address register
    (CC_REGNUM       		32)         ; Condition code pseudo register
    (FCC_REGNUM      		33)         ; Floating Condition code pseudo register
-   (UNSPEC_LOOP                200)         ; loop counter
   ]
 )
 
+(define_c_enum "unspec" [
+  UNSPEC_LOOP
+  UNSPEC_RCP
+  UNSPEC_RSQRT
+])
+
 (define_attr "length" ""
   (const_int 4))
 
@@ -2450,8 +2455,22 @@ 
 ;; ---------------- special insns
 ;;
 
-;;; reciprocal
-(define_insn "recipsf2"
+;; Generic code demands that the recip and rsqrt named patterns
+;; have precisely one operand.  So that's what we expose in the
+;; expander via the strange UNSPEC.  However, those expanders
+;; generate normal looking recip and rsqrt patterns.
+
+(define_expand "recipsf2"
+  [(set (match_operand:SF 0 "register_operand" "")
+	(unspec:SF [(match_operand:SF 1 "register_operand" "")]
+		   UNSPEC_RCP))]
+  "TARGET_USE_FPU"
+  {
+    emit_insn (gen_recipsf2_insn (operands[0], CONST1_RTX (SFmode), operands[1]));
+    DONE;
+  })
+
+(define_insn "recipsf2_insn"
   [(set (match_operand:SF 0 "register_operand" "=r")
 	(div:SF (match_operand:SF 1 "const_float_1_operand" "")
 		(match_operand:SF 2 "register_operand" "r")))]
@@ -2461,7 +2480,17 @@ 
    (set_attr "cc" "none_0hit")
    (set_attr "type" "fpu")])
 
-(define_insn "recipdf2"
+(define_expand "recipdf2"
+  [(set (match_operand:DF 0 "even_reg_operand" "")
+	(unspec:DF [(match_operand:SF 1 "even_reg_operand" "")]
+		   UNSPEC_RCP))]
+  "TARGET_USE_FPU"
+  {
+    emit_insn (gen_recipdf2_insn (operands[0], CONST1_RTX (DFmode), operands[1]));
+    DONE;
+  })
+
+(define_insn "recipdf2_insn"
   [(set (match_operand:DF 0 "even_reg_operand" "=r")
 	(div:DF (match_operand:DF 1 "const_float_1_operand" "")
 		(match_operand:DF 2 "even_reg_operand" "r")))]
@@ -2472,7 +2501,17 @@ 
    (set_attr "type" "fpu")])
 
 ;;; reciprocal of square-root
-(define_insn "rsqrtsf2"
+(define_expand "rsqrtsf2"
+  [(set (match_operand:SF 0 "register_operand" "=")
+	(unspec:SF [(match_operand:SF 1 "register_operand" "")]
+		   UNSPEC_RSQRT))]
+  "TARGET_USE_FPU"
+  {
+    emit_insn (gen_rsqrtsf2_insn (operands[0], CONST1_RTX (SFmode), operands[1]));
+    DONE;
+  })
+
+(define_insn "rsqrtsf2_insn"
   [(set (match_operand:SF 0 "register_operand" "=r")
 	(div:SF (match_operand:SF 1 "const_float_1_operand" "")
 		(sqrt:SF (match_operand:SF 2 "register_operand" "r"))))]
@@ -2482,7 +2521,17 @@ 
    (set_attr "cc" "none_0hit")
    (set_attr "type" "fpu")])
 
-(define_insn "rsqrtdf2"
+(define_expand "rsqrtdf2"
+  [(set (match_operand:DF 0 "even_reg_operand" "=r")
+	(unspec:DF [(match_operand:DF 1 "even_reg_operand" "r")]
+		   UNSPEC_RSQRT))]
+  "TARGET_USE_FPU"
+  {
+    emit_insn (gen_rsqrtdf2_insn (operands[0], CONST1_RTX (DFmode), operands[1]));
+    DONE;
+  })
+
+(define_insn "rsqrtdf2_insn"
   [(set (match_operand:DF 0 "even_reg_operand" "=r")
 	(div:DF (match_operand:DF 1 "const_float_1_operand" "")
 		(sqrt:DF (match_operand:DF 2 "even_reg_operand" "r"))))]