Re: [PATCH 2/2] Add VREL_OTHER for FP unsupported relations.

[Andrew MacLeod <amacleod@redhat.com>]

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;