On 1/25/23 09:48, Jakub Jelinek wrote:
On Wed, Jan 25, 2023 at 09:30:44AM -0500, Andrew MacLeod wrote:
But I'm afraid the above has VREL_OTHER for too many important cases,
unlike intersect where it is for none unless VREL_OTHER is involved, or just
a few ones for union.
Im not sure it is quite that bad. Floating point ranges and range-ops does
a pretty good job of tracking NANs in the ranges. They then utilize any
available relation in addition to that. So within floating point processing,
What I meant is that when we need to (and we have to, trying to do some
weird changes in intersect doesn't really improve anything) change the
relation_negate or its callers of a relation for floating point with
possible NANs from current inversion of VREL_{LT,GT,LE,GE} which are quite
frequent to VREL_OTHER (I don't know), it can affect a lot of code.
Now, sure, we could try to improve the situation a little bit by not
using just HONOR_NANS (type) as the decider whether we need the new 16
cases VREL_* handling (or 8 + VREL_OTHER) or whether we can use just the 8
cases VREL_* handling. Because, if HONOR_NANS (type) and frange can prove
that neither operand is maybe_nan and neither operand is known_nan, then
we can also use just the old 8 VREL_* codes and their relationships.
And perhaps if either operand is known_nan, then on the other side we know
it is VREL_OTHER (VREL_UNORDERED), not anything else.
Though, exactly for this I'd say it is more work and something for GCC 14.
Proper handling of relation_negate is I'm afraid required for GCC 13.
Except we don't actually use relation_negate () anywhere... I can
delete the functions in class value_relation and value-relation.h/cc and
everything compiles fine. Its provided because I figured it would be
used someday, but the range-ops handlers simply issues the appropriate
relation for the LHS.. be it true or false. they don't pick one and
negate it to produce the other.
I think you are missing that the VREL_* relation is not the end result
used to calculate things, merely a tag that used by the range-ops
handlers to assist with understanding un-obvious relations between
operands on the stmt.
This change is mostly for the benefit of the one place in ranger where
its slightly beyond range-ops.. when we have 2 conditions feeding and
logical && or ||, we look to see if there is any common ground/relation
between the operands feeding the logical operation:
// c_2 = a_6 > b_7
// c_3 = a_6 < b_7
// c_4 = c_2 && c_3
// c_2 and c_3 can never be true at the same time,
// Therefore c_4 can always resolve to false based purely on the relations.
void fold_using_range::relation_fold_and_or (irange& lhs_range, gimple
*s, fur_source &src)
Range-ops is unable to do anything with this as it requires examining
things from outside the statement, and is not easily communicated a
simple relation to operator_logical_and.
THIs routine proceeds to look at the defintions of c_2 and c_3, tries to
determine if there are common operands, and queries for any relations
between them. If it turns out, this is something, we use intersection
or fold to determine if the result of the logical operation can be
folded. THis fix is almost exclusively about that.
In GCC13, I don't think there are any uses of the relation oracle
outside of ranger and range-ops.
So, given that, perhaps the simplest thing to do is bail on all this
change, and instead simply do the following which also fixes the PR. (im
running it thru tests as we speak)
diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 91eb6298254..9c5359a3fc6 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -1039,6 +1039,9 @@ fold_using_range::relation_fold_and_or (irange&
lhs_range, gimple *s,
if (!ssa1_dep1 || !ssa1_dep2 || !ssa2_dep1 || !ssa2_dep2)
return;
+ if (HONOR_NANS (TREE_TYPE (ssa1_dep1)))
+ return;
+
// Make sure they are the same dependencies, and detect the order of the
// relationship.
bool reverse_op2 = true;
