On 11/14/22 10:12, Richard Biener wrote:
On Sat, Nov 12, 2022 at 7:30 PM Aldy Hernandez <al...@redhat.com> wrote:
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 ;-))?
I think the checks would belong to the gimple_stmt_nonnegative_warnv_p function
only (that's the SSA name entry from the fold-const.cc ones)?
That was my first approach, but I thought I'd cover the unary and binary
operators as well, since they had other callers. But I'm happy with
just the top-level tweak. It's a lot less code :).
I also notice the use of 'bool' for the "sign". That's not really
descriptive. We
have SIGNED and UNSIGNED (aka enum signop), not sure if that's the
perfect match vs. NEGATIVE and NONNEGATIVE. Maybe the functions
name is just bad and they should be known_float_negative_p?
The bool sign is to keep in line with real.*, and was suggested by Jeff
(in real.* not here). I'm happy to change the entire frange API to use
sgnop. It is cleaner. If that's acceptable, I could do that as a
follow-up.
How's this, pending tests once I figure out why my trees have been
broken all day :-/.
Aldy
p.s. First it was sphinx failure, now I'm seeing this:
/home/aldyh/src/clean/gcc/match.pd:7935:8 error: return statement not
allowed in C expression
return NULL_TREE;
^From 6e36626aec81bf97f8f54116a291574c16cbc205 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <al...@redhat.com>
Date: Sat, 12 Nov 2022 11:58:07 +0100
Subject: [PATCH] [PR68097] Try to avoid recursing for floats in
gimple_stmt_nonnegative_warnv_p.
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. This patch uses a
global range query (no on-demand lookups, just global ranges and
minimal folding) to determine if the range of a statement is known to
be non-negative.
PR tree-optimization/68097
gcc/ChangeLog:
* gimple-fold.cc (gimple_stmt_nonnegative_warnv_p): Call
range_of_stmt for floats.
---
gcc/gimple-fold.cc | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index 0a212e6d0d4..79cc4d7f569 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -68,6 +68,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-ssa-strlen.h"
#include "varasm.h"
#include "internal-fn.h"
+#include "gimple-range.h"
enum strlen_range_kind {
/* Compute the exact constant string length. */
@@ -9234,6 +9235,15 @@ bool
gimple_stmt_nonnegative_warnv_p (gimple *stmt, bool *strict_overflow_p,
int depth)
{
+ tree type = gimple_range_type (stmt);
+ if (type && frange::supports_p (type))
+ {
+ frange r;
+ bool sign;
+ return (get_global_range_query ()->range_of_stmt (r, stmt)
+ && r.signbit_p (sign)
+ && sign == false);
+ }
switch (gimple_code (stmt))
{
case GIMPLE_ASSIGN:
--
2.38.1
