Re: [RFC PATCH] range-op-float: Fix up op1_op2_relation of comparisons

[Aldy Hernandez <aldyh@redhat.com>]

On 4/11/23 10:21, Jakub Jelinek wrote:
> Hi!
> 
> This patch was what I've tried first before the currently committed
> PR109386 fix.  Still, I think it is the right thing until we have proper
> full set of VREL_* relations for NANs (though it would be really nice
> if op1_op2_relation could be passed either type of the comparison
> operands, or even better ranges of the two operands, such that
> we could choose if inversion of say VREL_LT is VREL_GE (if !MODE_HONOR_NANS
> (TYPE_MODE (type))) or rhs1/rhs2 ranges are guaranteed not to include
> NANs (!known_isnan && !maybe_isnan for both), or VREL_UNGE, etc.
> Anyway, the current state is that for the LE/LT/GE/GT comparisons
> we pretend the inverse is like for integral comparisons, which is
> true only if NANs can't appear in operands, while for UNLE/UNLT/UNGE/UNGT
> we don't override op1_op2_relation (so it always yields VREL_VARYING).
> 
> Though, this patch regresses the
> FAIL: gcc.dg/tree-ssa/vrp-float-6.c scan-tree-dump-times evrp "Folding predicate x_.* <= y_.* to 1" 1
> test, so am not sure what to do with it.  The test has explicit
> !isnan tests around it, so e.g. having the ranges passed to op1_op2_relation
> would also fix it.

I'll defer to Andrew on this, as he's the master of the relation oracle :).

Aldy

> 
> 2023-04-11  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* range-op-float.cc (foperator_lt::op1_op2_relation): Return
> 	VREL_VARYING instead of VREL_GE.
> 	(foperator_le::op1_op2_relation): Return VREL_VARYING instead of
> 	VREL_GT.
> 	(foperator_gt::op1_op2_relation): Return VREL_VARYING instead of
> 	VREL_LE.
> 	(foperator_ge::op1_op2_relation): Return VREL_VARYING instead of
> 	VREL_LT.
> 	(foperator_unordered_lt::op1_op2_relation,
> 	foperator_unordered_le::op1_op2_relation,
> 	foperator_unordered_gt::op1_op2_relation,
> 	foperator_unordered_ge::op1_op2_relation): New.
> 
> --- gcc/range-op-float.cc.jj	2023-04-01 09:32:02.635423345 +0200
> +++ gcc/range-op-float.cc	2023-04-03 10:42:54.000000000 +0200
> @@ -831,7 +831,10 @@ public:
>   		   relation_trio = TRIO_VARYING) const final override;
>     relation_kind op1_op2_relation (const irange &lhs) const final override
>     {
> -    return lt_op1_op2_relation (lhs);
> +    relation_kind ret = lt_op1_op2_relation (lhs);
> +    if (ret == VREL_GE)
> +      ret = VREL_VARYING; // Inverse of VREL_LT is VREL_UNGE.
> +    return ret;
>     }
>     bool op1_range (frange &r, tree type,
>   		  const irange &lhs, const frange &op2,
> @@ -947,6 +950,10 @@ public:
>   		   relation_trio rel = TRIO_VARYING) const final override;
>     relation_kind op1_op2_relation (const irange &lhs) const final override
>     {
> +    relation_kind ret = le_op1_op2_relation (lhs);
> +    if (ret == VREL_GT)
> +      ret = VREL_VARYING; // Inverse of VREL_LE is VREL_UNGT.
> +    return ret;
>       return le_op1_op2_relation (lhs);
>     }
>     bool op1_range (frange &r, tree type,
> @@ -1057,7 +1064,10 @@ public:
>   		   relation_trio = TRIO_VARYING) const final override;
>     relation_kind op1_op2_relation (const irange &lhs) const final override
>     {
> -    return gt_op1_op2_relation (lhs);
> +    relation_kind ret = gt_op1_op2_relation (lhs);
> +    if (ret == VREL_LE)
> +      ret = VREL_VARYING; // Inverse of VREL_GT is VREL_UNLE.
> +    return ret;
>     }
>     bool op1_range (frange &r, tree type,
>   		  const irange &lhs, const frange &op2,
> @@ -1177,7 +1187,10 @@ public:
>   		   relation_trio = TRIO_VARYING) const final override;
>     relation_kind op1_op2_relation (const irange &lhs) const final override
>     {
> -    return ge_op1_op2_relation (lhs);
> +    relation_kind ret = ge_op1_op2_relation (lhs);
> +    if (ret == VREL_LT)
> +      ret = VREL_VARYING; // Inverse of VREL_GE is VREL_UNLT.
> +    return ret;
>     }
>     bool op1_range (frange &r, tree type,
>   		  const irange &lhs, const frange &op2,
> @@ -1571,6 +1584,7 @@ class foperator_unordered_lt : public ra
>     using range_operator_float::fold_range;
>     using range_operator_float::op1_range;
>     using range_operator_float::op2_range;
> +  using range_operator_float::op1_op2_relation;
>   public:
>     bool fold_range (irange &r, tree type,
>   		   const frange &op1, const frange &op2,
> @@ -1599,6 +1613,13 @@ public:
>   	return true;
>         }
>     }
> +  relation_kind op1_op2_relation (const irange &lhs) const final override
> +  {
> +    relation_kind ret = lt_op1_op2_relation (lhs);
> +    if (ret == VREL_LT)
> +      ret = VREL_VARYING; // Should have been VREL_UNLT.
> +    return ret;
> +  }
>     bool op1_range (frange &r, tree type,
>   		  const irange &lhs,
>   		  const frange &op2,
> @@ -1682,6 +1703,7 @@ class foperator_unordered_le : public ra
>     using range_operator_float::fold_range;
>     using range_operator_float::op1_range;
>     using range_operator_float::op2_range;
> +  using range_operator_float::op1_op2_relation;
>   public:
>     bool fold_range (irange &r, tree type,
>   		   const frange &op1, const frange &op2,
> @@ -1710,6 +1732,13 @@ public:
>   	return true;
>         }
>     }
> +  relation_kind op1_op2_relation (const irange &lhs) const final override
> +  {
> +    relation_kind ret = le_op1_op2_relation (lhs);
> +    if (ret == VREL_LE)
> +      ret = VREL_VARYING; // Should have been VREL_UNLE.
> +    return ret;
> +  }
>     bool op1_range (frange &r, tree type,
>   		  const irange &lhs, const frange &op2,
>   		  relation_trio = TRIO_VARYING) const final override;
> @@ -1789,6 +1818,7 @@ class foperator_unordered_gt : public ra
>     using range_operator_float::fold_range;
>     using range_operator_float::op1_range;
>     using range_operator_float::op2_range;
> +  using range_operator_float::op1_op2_relation;
>   public:
>     bool fold_range (irange &r, tree type,
>   		   const frange &op1, const frange &op2,
> @@ -1817,6 +1847,13 @@ public:
>   	return true;
>         }
>     }
> +  relation_kind op1_op2_relation (const irange &lhs) const final override
> +  {
> +    relation_kind ret = gt_op1_op2_relation (lhs);
> +    if (ret == VREL_GT)
> +      ret = VREL_VARYING; // Should have been VREL_UNGT.
> +    return ret;
> +  }
>     bool op1_range (frange &r, tree type,
>   		  const irange &lhs, const frange &op2,
>   		  relation_trio = TRIO_VARYING) const final override;
> @@ -1900,6 +1937,7 @@ class foperator_unordered_ge : public ra
>     using range_operator_float::fold_range;
>     using range_operator_float::op1_range;
>     using range_operator_float::op2_range;
> +  using range_operator_float::op1_op2_relation;
>   public:
>     bool fold_range (irange &r, tree type,
>   		   const frange &op1, const frange &op2,
> @@ -1928,6 +1966,13 @@ public:
>   	return true;
>         }
>     }
> +  relation_kind op1_op2_relation (const irange &lhs) const final override
> +  {
> +    relation_kind ret = ge_op1_op2_relation (lhs);
> +    if (ret == VREL_GE)
> +      ret = VREL_VARYING; // Should have been VREL_UNGE.
> +    return ret;
> +  }
>     bool op1_range (frange &r, tree type,
>   		  const irange &lhs, const frange &op2,
>   		  relation_trio = TRIO_VARYING) const final override;
> 
> 	Jakub
>