Re: [PATCH 2/2] VR-VALUES: Rewrite test_for_singularity using range_op_handler

[Jeff Law <jeffreyalaw@gmail.com>]

On 9/1/23 11:30, Andrew Pinski via Gcc-patches wrote:
> So it turns out there was a simplier way of starting to
> improve VRP to start to fix PR 110131, PR 108360, and PR 108397.
> That was rewrite test_for_singularity to use range_op_handler
> and Value_Range.
> 
> This patch implements that and
> 
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> 
> gcc/ChangeLog:
> 
> 	* vr-values.cc (test_for_singularity): Add edge argument
> 	and rewrite using range_op_handler.
> 	(simplify_compare_using_range_pairs): Use Value_Range
> 	instead of value_range and update test_for_singularity call.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/tree-ssa/vrp124.c: New test.
> 	* gcc.dg/tree-ssa/vrp125.c: New test.
> ---

> diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
> index 52ab4fe6109..2474e57ee90 100644
> --- a/gcc/vr-values.cc
> +++ b/gcc/vr-values.cc
> @@ -904,69 +904,33 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
>   }
>   
>   /* We are comparing trees OP1 and OP2 using COND_CODE.  OP1 has
> -   a known value range VR.
> +   a known value range OP1_RANGE.
>   
>      If there is one and only one value which will satisfy the
> -   conditional, then return that value.  Else return NULL.
> -
> -   If signed overflow must be undefined for the value to satisfy
> -   the conditional, then set *STRICT_OVERFLOW_P to true.  */
> +   conditional on the EDGE, then return that value.
> +   Else return NULL.  */
>   
>   static tree
>   test_for_singularity (enum tree_code cond_code, tree op1,
> -		      tree op2, const value_range *vr)
> +		      tree op2, const int_range_max &op1_range, bool edge)
>   {
> -  tree min = NULL;
> -  tree max = NULL;
> -
> -  /* Extract minimum/maximum values which satisfy the conditional as it was
> -     written.  */
> -  if (cond_code == LE_EXPR || cond_code == LT_EXPR)
> +  /* This is already a singularity.  */
> +  if (cond_code == NE_EXPR || cond_code == EQ_EXPR)
> +    return NULL;
I don't think this is necessarily the right thing to do for NE.

Consider if op1 has the range [0,1] and op2 has the value 1.  If the 
code is NE, then we should be able to return a singularity of 0 since 
that's the only value for x where x ne 1 is true given the range for x.



I like what you're trying to do, it just needs a bit of refinement I think.

jeff