[patch] disentangle range_fold_*ary_expr into various pieces

Jeff Law law@redhat.com
Fri Oct 4 18:09:00 GMT 2019


On 10/4/19 7:25 AM, Aldy Hernandez wrote:
> I promised I would clean up the VRP/range-ops interface once the patch
> went in.  I was afraid of shuffling things too much initially, because I
> was trying to keep to the original structure of
> extract_range_from_*ary_expr.  Now that things are in place, it's easier
> to abstract things out, and clean them up a bit.
> 
> The gist of this patch is to pull out all the different sections of the
> VRP/range-ops glue (range_fold_{binary,unary}) into different functions.
>  In doing this, the overall structure is easier to see, and I was able
> to pin point (and remove) a lot of redundant work.
> 
> Unfortunately, the patch is hard to read because of the reshuffling.  It
> may be easier to just look at the new patched tree-vrp.c starting at
> get_range_op_handler() through range_fold_unary_expr().
> 
> The general idea is that we now have:
> 
> void
> range_fold_binary_expr (value_range_base *vr,
>             enum tree_code code,
>             tree expr_type,
>             const value_range_base *vr0_,
>             const value_range_base *vr1_)
> {
>   if (!supported_types_p (vr, expr_type)
>       || !defined_ranges_p (vr, vr0_, vr1_))
>     return;
>   const range_operator *op = get_range_op_handler (vr, code, expr_type);
>   if (!op)
>     return;
> 
>   value_range_base vr0 = drop_undefines_to_varying (vr0_, expr_type);
>   value_range_base vr1 = drop_undefines_to_varying (vr1_, expr_type);
>   if (range_fold_binary_symbolics_p (vr, code, expr_type, &vr0, &vr1))
>     return;
> 
>   *vr = op->fold_range (expr_type,
>             vr0.normalize_addresses (),
>             vr1.normalize_addresses ());
> }
> 
> ...which is WAY easier to read than the spaghetti we had before.  Note
> that all the symbolic stuff is neatly contained in range_fold_*symbolics_p.
> 
> In doing so, I added the ability to normalize addresses into
> normalize_symbolics, which centralizes the place where we transform a
> non-numeric range into a strictly numeric range.  As a reference,
> "symbolics" have traditionally been things like [x + 5, 8], but a
> reference is not strictly symbolic, but it's still not numeric [&a, &a],
> etc.  With the current patch, normalize_symbolics will return a numeric
> (or varying or undefined).
> 
> [Yeah, we should call that ::normalize_non_numeric or something, perhaps
> later.]
> 
> I also exposed the new ::normalize_addresses method, so the
> range_fold_*ary* stuff can normalize addresses instead of going through
> the entire normalize_symbolics dance.  Though, I could just as easily
> just call normalize_symbolics from the range_fold* stuff.  It just
> seemed like a useful small optimization.
> 
> I was pleasantly surprised that the symbolic handling of
> CONVERT_EXPR_CODE_P melted in favor of the generic ::normalize_symbolics
> mechanism.
> 
> Anyways, sorry for the gymnastics here, but regardless of what happens
> to range_fold_*ary*, it needed a good clean-up.
> 
> OK?
> 
> Aldy
> 
> range-fold-cleanups.patch
> 
> commit 70d336374fb39e857ecbd720900df9869c3c2ce9
> Author: Aldy Hernandez <aldyh@redhat.com>
> Date:   Thu Oct 3 14:53:12 2019 +0200
> 
>     Disentangle range_fold_*ary_expr() into various independent pieces.
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 3934b41fdf9..714b22781ee 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,17 @@
> +2019-10-04  Aldy Hernandez  <aldyh@redhat.com>
> +
> +	* tree-vrp.c (get_range_op_handler): New.
> +	(supported_types_p): New.
> +	(defined_ranges_p): New.
> +	(drop_undefines_to_varying): New.
> +	(range_fold_binary_symbolics_p): New.
> +	(range_fold_unary_symbolics_p): New.
> +	(range_fold_unary_expr): Extract out into above functions.
> +	(range_fold_binary_expr): Same.
> +	(value_range_base::normalize_addresses): New.
> +	(value_range_base::normalize_symbolics): Normalize addresses.
> +	* tree-vrp.h (class value_range_base): Add normalize_addresses.
You're right.  Easier to refer to the before/after directly.  Sometimes
diffs just suck.

OK
jeff



More information about the Gcc-patches mailing list