public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Aldy Hernandez <aldyh@redhat.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: GCC patches <gcc-patches@gcc.gnu.org>,
	Andrew MacLeod <amacleod@redhat.com>,
	richard.sandiford@arm.com, Aldy Hernandez <aldyh@redhat.com>
Subject: [PATCH] [PR68097] Try to avoid recursing for floats in tree_*_nonnegative_warnv_p.
Date: Sat, 12 Nov 2022 19:30:48 +0100	[thread overview]
Message-ID: <20221112183048.389811-1-aldyh@redhat.com> (raw)

It irks me that a PR named "we should track ranges for floating-point
hasn't been closed in this release.  This is an attempt to do just
that.

As mentioned in the PR, even though we track ranges for floats, it has
been suggested that avoiding recursing through SSA defs in
gimple_assign_nonnegative_warnv_p is also a goal.  We can do this with
various ranger components without the need for a heavy handed approach
(i.e. a full ranger).

I have implemented two versions of known_float_sign_p() that answer
the question whether we definitely know the sign for an operation or a
tree expression.

Both versions use get_global_range_query, which is a wrapper to query
global ranges.  This means, that no caching or propagation is done.
In the case of an SSA, we just return the global range for it (think
SSA_NAME_RANGE_INFO).  In the case of a tree code with operands, we
also use get_global_range_query to resolve the operands, and then call
into range-ops, which is our lowest level component.  There is no
ranger or gori involved.  All we're doing is resolving the operation
with the ranges passed.

This is enough to avoid recursing in the case where we definitely know
the sign of a range.  Otherwise, we still recurse.

Note that instead of get_global_range_query(), we could use
get_range_query() which uses a ranger (if active in a pass), or
get_global_range_query if not.  This would allow passes that have an
active ranger (with enable_ranger) to use a full ranger.  These passes
are currently, VRP, loop unswitching, DOM, loop versioning, etc.  If
no ranger is active, get_range_query defaults to global ranges, so
there's no additional penalty.

Would this be acceptable, at least enough to close (or rename the PR ;-))?

	PR tree-optimization/68097

gcc/ChangeLog:

	* fold-const.cc (known_float_sign_p): New.
	(tree_unary_nonnegative_warnv_p): Call known_float_sign_p.
	(tree_binary_nonnegative_warnv_p): Same.
	(tree_single_nonnegative_warnv_p): Same.
---
 gcc/fold-const.cc | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index b89cac91cae..bd74cfca996 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -14577,6 +14577,44 @@ tree_simple_nonnegative_warnv_p (enum tree_code code, tree type)
   return false;
 }
 
+/* Return true if T is of type floating point and has a known sign.
+   If so, set the sign in SIGN.  */
+
+static bool
+known_float_sign_p (bool &sign, tree t)
+{
+  if (!frange::supports_p (TREE_TYPE (t)))
+    return false;
+
+  frange r;
+  return (get_global_range_query ()->range_of_expr (r, t)
+	  && r.signbit_p (sign));
+}
+
+/* Return true if TYPE is a floating-point type and (CODE OP0 OP1) has
+   a known sign.  If so, set the sign in SIGN.  */
+
+static bool
+known_float_sign_p (bool &sign, enum tree_code code, tree type, tree op0,
+		    tree op1 = NULL_TREE)
+{
+  if (!frange::supports_p (type))
+    return false;
+
+  range_op_handler handler (code, type);
+  if (handler)
+    {
+      frange res, r0, r1;
+      get_global_range_query ()->range_of_expr (r0, op0);
+      if (op1)
+	get_global_range_query ()->range_of_expr (r1, op1);
+      else
+	r1.set_varying (type);
+      return handler.fold_range (res, type, r0, r1) && res.signbit_p (sign);
+    }
+  return false;
+}
+
 /* Return true if (CODE OP0) is known to be non-negative.  If the return
    value is based on the assumption that signed overflow is undefined,
    set *STRICT_OVERFLOW_P to true; otherwise, don't change
@@ -14589,6 +14627,10 @@ tree_unary_nonnegative_warnv_p (enum tree_code code, tree type, tree op0,
   if (TYPE_UNSIGNED (type))
     return true;
 
+  bool sign;
+  if (known_float_sign_p (sign, code, type, op0))
+    return !sign;
+
   switch (code)
     {
     case ABS_EXPR:
@@ -14656,6 +14698,10 @@ tree_binary_nonnegative_warnv_p (enum tree_code code, tree type, tree op0,
   if (TYPE_UNSIGNED (type))
     return true;
 
+  bool sign;
+  if (known_float_sign_p (sign, code, type, op0, op1))
+    return !sign;
+
   switch (code)
     {
     case POINTER_PLUS_EXPR:
@@ -14778,6 +14824,8 @@ tree_binary_nonnegative_warnv_p (enum tree_code code, tree type, tree op0,
 bool
 tree_single_nonnegative_warnv_p (tree t, bool *strict_overflow_p, int depth)
 {
+  bool sign;
+
   if (TYPE_UNSIGNED (TREE_TYPE (t)))
     return true;
 
@@ -14796,6 +14844,9 @@ tree_single_nonnegative_warnv_p (tree t, bool *strict_overflow_p, int depth)
       return RECURSE (TREE_OPERAND (t, 1)) && RECURSE (TREE_OPERAND (t, 2));
 
     case SSA_NAME:
+      if (known_float_sign_p (sign, t))
+	return !sign;
+
       /* Limit the depth of recursion to avoid quadratic behavior.
 	 This is expected to catch almost all occurrences in practice.
 	 If this code misses important cases that unbounded recursion
-- 
2.38.1


             reply	other threads:[~2022-11-12 18:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-12 18:30 Aldy Hernandez [this message]
2022-11-14  9:12 ` Richard Biener
2022-11-14 19:05   ` Aldy Hernandez
2022-11-15  7:15     ` Richard Biener
2022-11-15 10:46       ` Aldy Hernandez
2022-11-16 16:04         ` Richard Biener
2022-11-16 17:38           ` Aldy Hernandez
2022-11-17  8:01             ` Richard Biener
2022-11-15 13:52   ` Aldy Hernandez
2022-11-16 15:59     ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221112183048.389811-1-aldyh@redhat.com \
    --to=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.guenther@gmail.com \
    --cc=richard.sandiford@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).