diff mbox series

Restrict :c to commutative ops as intended

Message ID 20241024151249.B67101368E@imap1.dmz-prg2.suse.org
State New
Headers show
Series Restrict :c to commutative ops as intended | expand

Commit Message

Richard Biener Oct. 24, 2024, 3:12 p.m. UTC 
genmatch was supposed to restrict :c to verifiable commutative
operations while leaving :C to the "I know what I'm doing" case.
The following enforces this, cleaning up parsing and amending
the commutative_op helper.  There's one pattern that needs adjustment,
the pattern optimizing fmax (x, NaN) or fmax (NaN, x) to x since
fmax isn't commutative.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

	* genmatch.cc (commutative_op): Add paramter to indicate whether
	all compares should be considered commutative.  Handle
	hypot, add_overflow and mul_overflow.
	(parser::parse_expr): Simplify 'c' handling by using
	commutative_op and error out when the operation is not.
	* match.pd ((minmax:c @0 NaN@1) -> @0): Use :C, we know
	what we are doing.
---
 gcc/genmatch.cc | 40 +++++++++++++++-------------------------
 gcc/match.pd    |  2 +-
 2 files changed, 16 insertions(+), 26 deletions(-)
diff mbox series

Patch

diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
index 6287d6a7bc2..e2c02ee75ff 100644
--- a/gcc/genmatch.cc
+++ b/gcc/genmatch.cc
@@ -1106,12 +1106,13 @@  is_a_helper <user_id *>::test (id_base *id)
    index of the first, otherwise return -1.  */
 
 static int
-commutative_op (id_base *id)
+commutative_op (id_base *id, bool compares_are_commutative = false)
 {
   if (operator_id *code = dyn_cast <operator_id *> (id))
     {
       if (commutative_tree_code (code->code)
-	  || commutative_ternary_tree_code (code->code))
+	  || commutative_ternary_tree_code (code->code)
+	  || (compares_are_commutative && comparison_code_p (code->code)))
 	return 0;
       return -1;
     }
@@ -1119,9 +1120,12 @@  commutative_op (id_base *id)
     switch (fn->fn)
       {
       CASE_CFN_FMA:
+      CASE_CFN_HYPOT:
       case CFN_FMS:
       case CFN_FNMA:
       case CFN_FNMS:
+      case CFN_ADD_OVERFLOW:
+      case CFN_MUL_OVERFLOW:
 	return 0;
 
       case CFN_COND_ADD:
@@ -1157,11 +1161,13 @@  commutative_op (id_base *id)
       }
   if (user_id *uid = dyn_cast<user_id *> (id))
     {
-      int res = commutative_op (uid->substitutes[0]);
+      int res = commutative_op (uid->substitutes[0],
+				compares_are_commutative);
       if (res < 0)
 	return -1;
       for (unsigned i = 1; i < uid->substitutes.length (); ++i)
-	if (res != commutative_op (uid->substitutes[i]))
+	if (res != commutative_op (uid->substitutes[i],
+				   compares_are_commutative))
 	  return -1;
       return res;
     }
@@ -5213,27 +5219,11 @@  parser::parse_expr ()
 		{
 		  if (*sp == 'c')
 		    {
-		      if (operator_id *o
-			    = dyn_cast<operator_id *> (e->operation))
-			{
-			  if (!commutative_tree_code (o->code)
-			      && !comparison_code_p (o->code))
-			    fatal_at (token, "operation is not commutative");
-			}
-		      else if (user_id *p = dyn_cast<user_id *> (e->operation))
-			for (unsigned i = 0;
-			     i < p->substitutes.length (); ++i)
-			  {
-			    if (operator_id *q
-				  = dyn_cast<operator_id *> (p->substitutes[i]))
-			      {
-				if (!commutative_tree_code (q->code)
-				    && !comparison_code_p (q->code))
-				  fatal_at (token, "operation %s is not "
-					    "commutative", q->id);
-			      }
-			  }
-		      is_commutative = true;
+		      if (commutative_op (e->operation, true) != -1)
+			is_commutative = true;
+		      else
+			fatal_at (token, "operation %s is not known "
+				  "commutative", e->operation->id);
 		    }
 		  else if (*sp == 'C')
 		    is_commutative = true;
diff --git a/gcc/match.pd b/gcc/match.pd
index 0455dfa6993..d78a19f450e 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4667,7 +4667,7 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  /* If either argument is NaN and other one is not sNaN, return the other
     one.  Avoid the transformation if we get (and honor) a signalling NaN.  */
  (simplify
-  (minmax:c @0 REAL_CST@1)
+  (minmax:C @0 REAL_CST@1)
    (if (real_isnan (TREE_REAL_CST_PTR (@1))
        && (!HONOR_SNANS (@1) || !TREE_REAL_CST (@1).signalling)
        && !tree_expr_maybe_signaling_nan_p (@0))