* [PATCH] use get_range_query to replace get_global_range_query
@ 2023-10-10 2:53 Jiufu Guo
2023-10-10 7:01 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jiufu Guo @ 2023-10-10 2:53 UTC (permalink / raw)
To: gcc-patches
Cc: rguenther, jeffreyalaw, richard.sandiford, segher, dje.gcc,
linkw, bergner, guojiufu, amacleod, aldyh
Hi,
For "get_global_range_query" SSA_NAME_RANGE_INFO can be queried.
For "get_range_query", it could get more context-aware range info.
And look at the implementation of "get_range_query", it returns
global range if no local fun info.
So, if not quering for SSA_NAME, it would be ok to use get_range_query
to replace get_global_range_query.
Patch https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630389.html,
Uses get_range_query could handle more cases.
This patch replaces get_global_range_query by get_range_query for
most possible code pieces (but deoes not draft new test cases).
Pass bootstrap & regtest on ppc64{,le} and x86_64.
Is this ok for trunk.
BR,
Jeff (Jiufu Guo)
gcc/ChangeLog:
* builtins.cc (expand_builtin_strnlen): Replace get_global_range_query
by get_range_query.
* fold-const.cc (expr_not_equal_to): Likewise.
* gimple-fold.cc (size_must_be_zero_p): Likewise.
* gimple-range-fold.cc (fur_source::fur_source): Likewise.
* gimple-ssa-warn-access.cc (check_nul_terminated_array): Likewise.
* tree-dfa.cc (get_ref_base_and_extent): Likewise.
* tree-ssa-loop-split.cc (split_at_bb_p): Likewise.
* tree-ssa-loop-unswitch.cc (evaluate_control_stmt_using_entry_checks):
Likewise.
---
gcc/builtins.cc | 2 +-
gcc/fold-const.cc | 6 +-----
gcc/gimple-fold.cc | 6 ++----
gcc/gimple-range-fold.cc | 4 +---
gcc/gimple-ssa-warn-access.cc | 2 +-
gcc/tree-dfa.cc | 5 +----
gcc/tree-ssa-loop-split.cc | 2 +-
gcc/tree-ssa-loop-unswitch.cc | 2 +-
8 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index cb90bd03b3e..4e0a77ff8e0 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -3477,7 +3477,7 @@ expand_builtin_strnlen (tree exp, rtx target, machine_mode target_mode)
wide_int min, max;
value_range r;
- get_global_range_query ()->range_of_expr (r, bound);
+ get_range_query (cfun)->range_of_expr (r, bound);
if (r.varying_p () || r.undefined_p ())
return NULL_RTX;
min = r.lower_bound ();
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 4f8561509ff..15134b21b9f 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -11056,11 +11056,7 @@ expr_not_equal_to (tree t, const wide_int &w)
if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
return false;
- if (cfun)
- get_range_query (cfun)->range_of_expr (vr, t);
- else
- get_global_range_query ()->range_of_expr (vr, t);
-
+ get_range_query (cfun)->range_of_expr (vr, t);
if (!vr.undefined_p () && !vr.contains_p (w))
return true;
/* If T has some known zero bits and W has any of those bits set,
diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index dc89975270c..853edd9e5d4 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -876,10 +876,8 @@ size_must_be_zero_p (tree size)
wide_int zero = wi::zero (TYPE_PRECISION (type));
value_range valid_range (type, zero, ssize_max);
value_range vr;
- if (cfun)
- get_range_query (cfun)->range_of_expr (vr, size);
- else
- get_global_range_query ()->range_of_expr (vr, size);
+ get_range_query (cfun)->range_of_expr (vr, size);
+
if (vr.undefined_p ())
vr.set_varying (TREE_TYPE (size));
vr.intersect (valid_range);
diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index d1945ccb554..6e9530c3d7f 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -50,10 +50,8 @@ fur_source::fur_source (range_query *q)
{
if (q)
m_query = q;
- else if (cfun)
- m_query = get_range_query (cfun);
else
- m_query = get_global_range_query ();
+ m_query = get_range_query (cfun);
m_gori = NULL;
}
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index fcaff128d60..e439d1b9b68 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -332,7 +332,7 @@ check_nul_terminated_array (GimpleOrTree expr, tree src, tree bound)
{
Value_Range r (TREE_TYPE (bound));
- get_global_range_query ()->range_of_expr (r, bound);
+ get_range_query (cfun)->range_of_expr (r, bound);
if (r.undefined_p () || r.varying_p ())
return true;
diff --git a/gcc/tree-dfa.cc b/gcc/tree-dfa.cc
index af8e9243947..5355af2c869 100644
--- a/gcc/tree-dfa.cc
+++ b/gcc/tree-dfa.cc
@@ -531,10 +531,7 @@ get_ref_base_and_extent (tree exp, poly_int64 *poffset,
value_range vr;
range_query *query;
- if (cfun)
- query = get_range_query (cfun);
- else
- query = get_global_range_query ();
+ query = get_range_query (cfun);
if (TREE_CODE (index) == SSA_NAME
&& (low_bound = array_ref_low_bound (exp),
diff --git a/gcc/tree-ssa-loop-split.cc b/gcc/tree-ssa-loop-split.cc
index 64464802c1e..e85a1881526 100644
--- a/gcc/tree-ssa-loop-split.cc
+++ b/gcc/tree-ssa-loop-split.cc
@@ -145,7 +145,7 @@ split_at_bb_p (class loop *loop, basic_block bb, tree *border, affine_iv *iv,
else
{
int_range<2> r;
- get_global_range_query ()->range_of_expr (r, op0, stmt);
+ get_range_query (cfun)->range_of_expr (r, op0, stmt);
if (!r.varying_p () && !r.undefined_p ()
&& TREE_CODE (op1) == INTEGER_CST)
{
diff --git a/gcc/tree-ssa-loop-unswitch.cc b/gcc/tree-ssa-loop-unswitch.cc
index 619b50fb4bb..b3dc2ded931 100644
--- a/gcc/tree-ssa-loop-unswitch.cc
+++ b/gcc/tree-ssa-loop-unswitch.cc
@@ -764,7 +764,7 @@ evaluate_control_stmt_using_entry_checks (gimple *stmt,
int_range_max r;
if (!ranger->gori ().outgoing_edge_range_p (r, e, idx,
- *get_global_range_query ()))
+ *get_range_query (cfun)))
continue;
r.intersect (path_range);
if (r.undefined_p ())
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] use get_range_query to replace get_global_range_query
2023-10-10 2:53 [PATCH] use get_range_query to replace get_global_range_query Jiufu Guo
@ 2023-10-10 7:01 ` Richard Biener
2023-10-10 7:23 ` Andrew Pinski
2023-10-10 9:07 ` Jiufu Guo
0 siblings, 2 replies; 4+ messages in thread
From: Richard Biener @ 2023-10-10 7:01 UTC (permalink / raw)
To: Jiufu Guo
Cc: gcc-patches, jeffreyalaw, richard.sandiford, segher, dje.gcc,
linkw, bergner, amacleod, aldyh
On Tue, 10 Oct 2023, Jiufu Guo wrote:
> Hi,
>
> For "get_global_range_query" SSA_NAME_RANGE_INFO can be queried.
> For "get_range_query", it could get more context-aware range info.
> And look at the implementation of "get_range_query", it returns
> global range if no local fun info.
>
> So, if not quering for SSA_NAME, it would be ok to use get_range_query
> to replace get_global_range_query.
>
> Patch https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630389.html,
> Uses get_range_query could handle more cases.
>
> This patch replaces get_global_range_query by get_range_query for
> most possible code pieces (but deoes not draft new test cases).
>
> Pass bootstrap & regtest on ppc64{,le} and x86_64.
> Is this ok for trunk.
See below
>
> BR,
> Jeff (Jiufu Guo)
>
> gcc/ChangeLog:
>
> * builtins.cc (expand_builtin_strnlen): Replace get_global_range_query
> by get_range_query.
> * fold-const.cc (expr_not_equal_to): Likewise.
> * gimple-fold.cc (size_must_be_zero_p): Likewise.
> * gimple-range-fold.cc (fur_source::fur_source): Likewise.
> * gimple-ssa-warn-access.cc (check_nul_terminated_array): Likewise.
> * tree-dfa.cc (get_ref_base_and_extent): Likewise.
> * tree-ssa-loop-split.cc (split_at_bb_p): Likewise.
> * tree-ssa-loop-unswitch.cc (evaluate_control_stmt_using_entry_checks):
> Likewise.
>
> ---
> gcc/builtins.cc | 2 +-
> gcc/fold-const.cc | 6 +-----
> gcc/gimple-fold.cc | 6 ++----
> gcc/gimple-range-fold.cc | 4 +---
> gcc/gimple-ssa-warn-access.cc | 2 +-
> gcc/tree-dfa.cc | 5 +----
> gcc/tree-ssa-loop-split.cc | 2 +-
> gcc/tree-ssa-loop-unswitch.cc | 2 +-
> 8 files changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> index cb90bd03b3e..4e0a77ff8e0 100644
> --- a/gcc/builtins.cc
> +++ b/gcc/builtins.cc
> @@ -3477,7 +3477,7 @@ expand_builtin_strnlen (tree exp, rtx target, machine_mode target_mode)
>
> wide_int min, max;
> value_range r;
> - get_global_range_query ()->range_of_expr (r, bound);
> + get_range_query (cfun)->range_of_expr (r, bound);
expand doesn't have a ranger instance so this is a no-op. I'm unsure
if it would be safe given we're half GIMPLE, half RTL. Please leave it
out.
> if (r.varying_p () || r.undefined_p ())
> return NULL_RTX;
> min = r.lower_bound ();
> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> index 4f8561509ff..15134b21b9f 100644
> --- a/gcc/fold-const.cc
> +++ b/gcc/fold-const.cc
> @@ -11056,11 +11056,7 @@ expr_not_equal_to (tree t, const wide_int &w)
> if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
> return false;
>
> - if (cfun)
> - get_range_query (cfun)->range_of_expr (vr, t);
> - else
> - get_global_range_query ()->range_of_expr (vr, t);
> -
> + get_range_query (cfun)->range_of_expr (vr, t);
These kind of changes look obvious.
> if (!vr.undefined_p () && !vr.contains_p (w))
> return true;
> /* If T has some known zero bits and W has any of those bits set,
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index dc89975270c..853edd9e5d4 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -876,10 +876,8 @@ size_must_be_zero_p (tree size)
> wide_int zero = wi::zero (TYPE_PRECISION (type));
> value_range valid_range (type, zero, ssize_max);
> value_range vr;
> - if (cfun)
> - get_range_query (cfun)->range_of_expr (vr, size);
> - else
> - get_global_range_query ()->range_of_expr (vr, size);
> + get_range_query (cfun)->range_of_expr (vr, size);
> +
> if (vr.undefined_p ())
> vr.set_varying (TREE_TYPE (size));
> vr.intersect (valid_range);
> diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
> index d1945ccb554..6e9530c3d7f 100644
> --- a/gcc/gimple-range-fold.cc
> +++ b/gcc/gimple-range-fold.cc
> @@ -50,10 +50,8 @@ fur_source::fur_source (range_query *q)
> {
> if (q)
> m_query = q;
> - else if (cfun)
> - m_query = get_range_query (cfun);
> else
> - m_query = get_global_range_query ();
> + m_query = get_range_query (cfun);
> m_gori = NULL;
> }
>
> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
> index fcaff128d60..e439d1b9b68 100644
> --- a/gcc/gimple-ssa-warn-access.cc
> +++ b/gcc/gimple-ssa-warn-access.cc
> @@ -332,7 +332,7 @@ check_nul_terminated_array (GimpleOrTree expr, tree src, tree bound)
> {
> Value_Range r (TREE_TYPE (bound));
>
> - get_global_range_query ()->range_of_expr (r, bound);
> + get_range_query (cfun)->range_of_expr (r, bound);
>
> if (r.undefined_p () || r.varying_p ())
> return true;
The pass has a ranger instance, so yes, this should improve things.
Since the pass doesn't do any IL modification it should also be safe.
> diff --git a/gcc/tree-dfa.cc b/gcc/tree-dfa.cc
> index af8e9243947..5355af2c869 100644
> --- a/gcc/tree-dfa.cc
> +++ b/gcc/tree-dfa.cc
> @@ -531,10 +531,7 @@ get_ref_base_and_extent (tree exp, poly_int64 *poffset,
>
> value_range vr;
> range_query *query;
> - if (cfun)
> - query = get_range_query (cfun);
> - else
> - query = get_global_range_query ();
> + query = get_range_query (cfun);
>
> if (TREE_CODE (index) == SSA_NAME
> && (low_bound = array_ref_low_bound (exp),
> diff --git a/gcc/tree-ssa-loop-split.cc b/gcc/tree-ssa-loop-split.cc
> index 64464802c1e..e85a1881526 100644
> --- a/gcc/tree-ssa-loop-split.cc
> +++ b/gcc/tree-ssa-loop-split.cc
> @@ -145,7 +145,7 @@ split_at_bb_p (class loop *loop, basic_block bb, tree *border, affine_iv *iv,
> else
> {
> int_range<2> r;
> - get_global_range_query ()->range_of_expr (r, op0, stmt);
> + get_range_query (cfun)->range_of_expr (r, op0, stmt);
loop splitting doesn't have a ranger instance so this is a no-op change
but I'm also not sure it would be safe to use a dynamic ranger instance
here since we are doing even CFG manipulations between. Please leave
this change out.
> if (!r.varying_p () && !r.undefined_p ()
> && TREE_CODE (op1) == INTEGER_CST)
> {
> diff --git a/gcc/tree-ssa-loop-unswitch.cc b/gcc/tree-ssa-loop-unswitch.cc
> index 619b50fb4bb..b3dc2ded931 100644
> --- a/gcc/tree-ssa-loop-unswitch.cc
> +++ b/gcc/tree-ssa-loop-unswitch.cc
> @@ -764,7 +764,7 @@ evaluate_control_stmt_using_entry_checks (gimple *stmt,
>
> int_range_max r;
> if (!ranger->gori ().outgoing_edge_range_p (r, e, idx,
> - *get_global_range_query ()))
> + *get_range_query (cfun)))
> continue;
unswitching has a ranger instance but it does perform IL modification.
Did you check whether the use of the global ranger was intentional here?
Specifically we do have the 'ranger' object here and IIRC using global
ranges was intentional. So please leave this change out.
Thanks,
Richard.
> r.intersect (path_range);
> if (r.undefined_p ())
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] use get_range_query to replace get_global_range_query
2023-10-10 7:01 ` Richard Biener
@ 2023-10-10 7:23 ` Andrew Pinski
2023-10-10 9:07 ` Jiufu Guo
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Pinski @ 2023-10-10 7:23 UTC (permalink / raw)
To: Richard Biener
Cc: Jiufu Guo, gcc-patches, jeffreyalaw, richard.sandiford, segher,
dje.gcc, linkw, bergner, amacleod, aldyh
On Tue, Oct 10, 2023 at 12:02 AM Richard Biener <rguenther@suse.de> wrote:
>
> On Tue, 10 Oct 2023, Jiufu Guo wrote:
>
> > Hi,
> >
> > For "get_global_range_query" SSA_NAME_RANGE_INFO can be queried.
> > For "get_range_query", it could get more context-aware range info.
> > And look at the implementation of "get_range_query", it returns
> > global range if no local fun info.
> >
> > So, if not quering for SSA_NAME, it would be ok to use get_range_query
> > to replace get_global_range_query.
> >
> > Patch https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630389.html,
> > Uses get_range_query could handle more cases.
> >
> > This patch replaces get_global_range_query by get_range_query for
> > most possible code pieces (but deoes not draft new test cases).
> >
> > Pass bootstrap & regtest on ppc64{,le} and x86_64.
> > Is this ok for trunk.
>
> See below
>
> >
> > BR,
> > Jeff (Jiufu Guo)
> >
> > gcc/ChangeLog:
> >
> > * builtins.cc (expand_builtin_strnlen): Replace get_global_range_query
> > by get_range_query.
> > * fold-const.cc (expr_not_equal_to): Likewise.
> > * gimple-fold.cc (size_must_be_zero_p): Likewise.
> > * gimple-range-fold.cc (fur_source::fur_source): Likewise.
> > * gimple-ssa-warn-access.cc (check_nul_terminated_array): Likewise.
> > * tree-dfa.cc (get_ref_base_and_extent): Likewise.
> > * tree-ssa-loop-split.cc (split_at_bb_p): Likewise.
> > * tree-ssa-loop-unswitch.cc (evaluate_control_stmt_using_entry_checks):
> > Likewise.
> >
> > ---
> > gcc/builtins.cc | 2 +-
> > gcc/fold-const.cc | 6 +-----
> > gcc/gimple-fold.cc | 6 ++----
> > gcc/gimple-range-fold.cc | 4 +---
> > gcc/gimple-ssa-warn-access.cc | 2 +-
> > gcc/tree-dfa.cc | 5 +----
> > gcc/tree-ssa-loop-split.cc | 2 +-
> > gcc/tree-ssa-loop-unswitch.cc | 2 +-
> > 8 files changed, 9 insertions(+), 20 deletions(-)
> >
> > diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> > index cb90bd03b3e..4e0a77ff8e0 100644
> > --- a/gcc/builtins.cc
> > +++ b/gcc/builtins.cc
> > @@ -3477,7 +3477,7 @@ expand_builtin_strnlen (tree exp, rtx target, machine_mode target_mode)
> >
> > wide_int min, max;
> > value_range r;
> > - get_global_range_query ()->range_of_expr (r, bound);
> > + get_range_query (cfun)->range_of_expr (r, bound);
>
> expand doesn't have a ranger instance so this is a no-op. I'm unsure
> if it would be safe given we're half GIMPLE, half RTL. Please leave it
> out.
It definitely does not work and can't as I tried to enable a ranger
instance and it didn't work. I wrote up my experience here:
https://gcc.gnu.org/pipermail/gcc/2023-September/242407.html
Thanks,
Andrew Pinski
>
> > if (r.varying_p () || r.undefined_p ())
> > return NULL_RTX;
> > min = r.lower_bound ();
> > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> > index 4f8561509ff..15134b21b9f 100644
> > --- a/gcc/fold-const.cc
> > +++ b/gcc/fold-const.cc
> > @@ -11056,11 +11056,7 @@ expr_not_equal_to (tree t, const wide_int &w)
> > if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
> > return false;
> >
> > - if (cfun)
> > - get_range_query (cfun)->range_of_expr (vr, t);
> > - else
> > - get_global_range_query ()->range_of_expr (vr, t);
> > -
> > + get_range_query (cfun)->range_of_expr (vr, t);
>
> These kind of changes look obvious.
>
> > if (!vr.undefined_p () && !vr.contains_p (w))
> > return true;
> > /* If T has some known zero bits and W has any of those bits set,
> > diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> > index dc89975270c..853edd9e5d4 100644
> > --- a/gcc/gimple-fold.cc
> > +++ b/gcc/gimple-fold.cc
> > @@ -876,10 +876,8 @@ size_must_be_zero_p (tree size)
> > wide_int zero = wi::zero (TYPE_PRECISION (type));
> > value_range valid_range (type, zero, ssize_max);
> > value_range vr;
> > - if (cfun)
> > - get_range_query (cfun)->range_of_expr (vr, size);
> > - else
> > - get_global_range_query ()->range_of_expr (vr, size);
> > + get_range_query (cfun)->range_of_expr (vr, size);
> > +
> > if (vr.undefined_p ())
> > vr.set_varying (TREE_TYPE (size));
> > vr.intersect (valid_range);
> > diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
> > index d1945ccb554..6e9530c3d7f 100644
> > --- a/gcc/gimple-range-fold.cc
> > +++ b/gcc/gimple-range-fold.cc
> > @@ -50,10 +50,8 @@ fur_source::fur_source (range_query *q)
> > {
> > if (q)
> > m_query = q;
> > - else if (cfun)
> > - m_query = get_range_query (cfun);
> > else
> > - m_query = get_global_range_query ();
> > + m_query = get_range_query (cfun);
> > m_gori = NULL;
> > }
> >
> > diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
> > index fcaff128d60..e439d1b9b68 100644
> > --- a/gcc/gimple-ssa-warn-access.cc
> > +++ b/gcc/gimple-ssa-warn-access.cc
> > @@ -332,7 +332,7 @@ check_nul_terminated_array (GimpleOrTree expr, tree src, tree bound)
> > {
> > Value_Range r (TREE_TYPE (bound));
> >
> > - get_global_range_query ()->range_of_expr (r, bound);
> > + get_range_query (cfun)->range_of_expr (r, bound);
> >
> > if (r.undefined_p () || r.varying_p ())
> > return true;
>
> The pass has a ranger instance, so yes, this should improve things.
> Since the pass doesn't do any IL modification it should also be safe.
>
> > diff --git a/gcc/tree-dfa.cc b/gcc/tree-dfa.cc
> > index af8e9243947..5355af2c869 100644
> > --- a/gcc/tree-dfa.cc
> > +++ b/gcc/tree-dfa.cc
> > @@ -531,10 +531,7 @@ get_ref_base_and_extent (tree exp, poly_int64 *poffset,
> >
> > value_range vr;
> > range_query *query;
> > - if (cfun)
> > - query = get_range_query (cfun);
> > - else
> > - query = get_global_range_query ();
> > + query = get_range_query (cfun);
> >
> > if (TREE_CODE (index) == SSA_NAME
> > && (low_bound = array_ref_low_bound (exp),
> > diff --git a/gcc/tree-ssa-loop-split.cc b/gcc/tree-ssa-loop-split.cc
> > index 64464802c1e..e85a1881526 100644
> > --- a/gcc/tree-ssa-loop-split.cc
> > +++ b/gcc/tree-ssa-loop-split.cc
> > @@ -145,7 +145,7 @@ split_at_bb_p (class loop *loop, basic_block bb, tree *border, affine_iv *iv,
> > else
> > {
> > int_range<2> r;
> > - get_global_range_query ()->range_of_expr (r, op0, stmt);
> > + get_range_query (cfun)->range_of_expr (r, op0, stmt);
>
> loop splitting doesn't have a ranger instance so this is a no-op change
> but I'm also not sure it would be safe to use a dynamic ranger instance
> here since we are doing even CFG manipulations between. Please leave
> this change out.
>
> > if (!r.varying_p () && !r.undefined_p ()
> > && TREE_CODE (op1) == INTEGER_CST)
> > {
> > diff --git a/gcc/tree-ssa-loop-unswitch.cc b/gcc/tree-ssa-loop-unswitch.cc
> > index 619b50fb4bb..b3dc2ded931 100644
> > --- a/gcc/tree-ssa-loop-unswitch.cc
> > +++ b/gcc/tree-ssa-loop-unswitch.cc
> > @@ -764,7 +764,7 @@ evaluate_control_stmt_using_entry_checks (gimple *stmt,
> >
> > int_range_max r;
> > if (!ranger->gori ().outgoing_edge_range_p (r, e, idx,
> > - *get_global_range_query ()))
> > + *get_range_query (cfun)))
> > continue;
>
> unswitching has a ranger instance but it does perform IL modification.
> Did you check whether the use of the global ranger was intentional here?
> Specifically we do have the 'ranger' object here and IIRC using global
> ranges was intentional. So please leave this change out.
>
> Thanks,
> Richard.
>
> > r.intersect (path_range);
> > if (r.undefined_p ())
> >
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] use get_range_query to replace get_global_range_query
2023-10-10 7:01 ` Richard Biener
2023-10-10 7:23 ` Andrew Pinski
@ 2023-10-10 9:07 ` Jiufu Guo
1 sibling, 0 replies; 4+ messages in thread
From: Jiufu Guo @ 2023-10-10 9:07 UTC (permalink / raw)
To: Richard Biener
Cc: gcc-patches, jeffreyalaw, richard.sandiford, segher, dje.gcc,
linkw, bergner, amacleod, aldyh
Hi,
Richard Biener <rguenther@suse.de> writes:
> On Tue, 10 Oct 2023, Jiufu Guo wrote:
>
>> Hi,
>>
>> For "get_global_range_query" SSA_NAME_RANGE_INFO can be queried.
>> For "get_range_query", it could get more context-aware range info.
>> And look at the implementation of "get_range_query", it returns
>> global range if no local fun info.
>>
>> So, if not quering for SSA_NAME, it would be ok to use get_range_query
>> to replace get_global_range_query.
>>
>> Patch https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630389.html,
>> Uses get_range_query could handle more cases.
>>
>> This patch replaces get_global_range_query by get_range_query for
>> most possible code pieces (but deoes not draft new test cases).
>>
>> Pass bootstrap & regtest on ppc64{,le} and x86_64.
>> Is this ok for trunk.
>
> See below
Thanks so much for your quick review!
>
>>
>> BR,
>> Jeff (Jiufu Guo)
>>
>> gcc/ChangeLog:
>>
>> * builtins.cc (expand_builtin_strnlen): Replace get_global_range_query
>> by get_range_query.
>> * fold-const.cc (expr_not_equal_to): Likewise.
>> * gimple-fold.cc (size_must_be_zero_p): Likewise.
>> * gimple-range-fold.cc (fur_source::fur_source): Likewise.
>> * gimple-ssa-warn-access.cc (check_nul_terminated_array): Likewise.
>> * tree-dfa.cc (get_ref_base_and_extent): Likewise.
>> * tree-ssa-loop-split.cc (split_at_bb_p): Likewise.
>> * tree-ssa-loop-unswitch.cc (evaluate_control_stmt_using_entry_checks):
>> Likewise.
>>
>> ---
>> gcc/builtins.cc | 2 +-
>> gcc/fold-const.cc | 6 +-----
>> gcc/gimple-fold.cc | 6 ++----
>> gcc/gimple-range-fold.cc | 4 +---
>> gcc/gimple-ssa-warn-access.cc | 2 +-
>> gcc/tree-dfa.cc | 5 +----
>> gcc/tree-ssa-loop-split.cc | 2 +-
>> gcc/tree-ssa-loop-unswitch.cc | 2 +-
>> 8 files changed, 9 insertions(+), 20 deletions(-)
>>
>> diff --git a/gcc/builtins.cc b/gcc/builtins.cc
>> index cb90bd03b3e..4e0a77ff8e0 100644
>> --- a/gcc/builtins.cc
>> +++ b/gcc/builtins.cc
>> @@ -3477,7 +3477,7 @@ expand_builtin_strnlen (tree exp, rtx target, machine_mode target_mode)
>>
>> wide_int min, max;
>> value_range r;
>> - get_global_range_query ()->range_of_expr (r, bound);
>> + get_range_query (cfun)->range_of_expr (r, bound);
>
> expand doesn't have a ranger instance so this is a no-op. I'm unsure
> if it would be safe given we're half GIMPLE, half RTL. Please leave it
> out.
Oh, yeap. There is no local ranger, and 'bound' is SSA_NAME,
and SSA_NAME_RANGE_INFO is there.
get_global_range_query should be used.
>
>> if (r.varying_p () || r.undefined_p ())
>> return NULL_RTX;
>> min = r.lower_bound ();
>> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
>> index 4f8561509ff..15134b21b9f 100644
>> --- a/gcc/fold-const.cc
>> +++ b/gcc/fold-const.cc
>> @@ -11056,11 +11056,7 @@ expr_not_equal_to (tree t, const wide_int &w)
>> if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
>> return false;
>>
>> - if (cfun)
>> - get_range_query (cfun)->range_of_expr (vr, t);
>> - else
>> - get_global_range_query ()->range_of_expr (vr, t);
>> -
>> + get_range_query (cfun)->range_of_expr (vr, t);
>
> These kind of changes look obvious.
>
>> if (!vr.undefined_p () && !vr.contains_p (w))
>> return true;
>> /* If T has some known zero bits and W has any of those bits set,
>> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
>> index dc89975270c..853edd9e5d4 100644
>> --- a/gcc/gimple-fold.cc
>> +++ b/gcc/gimple-fold.cc
>> @@ -876,10 +876,8 @@ size_must_be_zero_p (tree size)
>> wide_int zero = wi::zero (TYPE_PRECISION (type));
>> value_range valid_range (type, zero, ssize_max);
>> value_range vr;
>> - if (cfun)
>> - get_range_query (cfun)->range_of_expr (vr, size);
>> - else
>> - get_global_range_query ()->range_of_expr (vr, size);
>> + get_range_query (cfun)->range_of_expr (vr, size);
>> +
>> if (vr.undefined_p ())
>> vr.set_varying (TREE_TYPE (size));
>> vr.intersect (valid_range);
>> diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
>> index d1945ccb554..6e9530c3d7f 100644
>> --- a/gcc/gimple-range-fold.cc
>> +++ b/gcc/gimple-range-fold.cc
>> @@ -50,10 +50,8 @@ fur_source::fur_source (range_query *q)
>> {
>> if (q)
>> m_query = q;
>> - else if (cfun)
>> - m_query = get_range_query (cfun);
>> else
>> - m_query = get_global_range_query ();
>> + m_query = get_range_query (cfun);
>> m_gori = NULL;
>> }
>>
>> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
>> index fcaff128d60..e439d1b9b68 100644
>> --- a/gcc/gimple-ssa-warn-access.cc
>> +++ b/gcc/gimple-ssa-warn-access.cc
>> @@ -332,7 +332,7 @@ check_nul_terminated_array (GimpleOrTree expr, tree src, tree bound)
>> {
>> Value_Range r (TREE_TYPE (bound));
>>
>> - get_global_range_query ()->range_of_expr (r, bound);
>> + get_range_query (cfun)->range_of_expr (r, bound);
>>
>> if (r.undefined_p () || r.varying_p ())
>> return true;
>
> The pass has a ranger instance, so yes, this should improve things.
> Since the pass doesn't do any IL modification it should also be safe.
Yes.
>
>> diff --git a/gcc/tree-dfa.cc b/gcc/tree-dfa.cc
>> index af8e9243947..5355af2c869 100644
>> --- a/gcc/tree-dfa.cc
>> +++ b/gcc/tree-dfa.cc
>> @@ -531,10 +531,7 @@ get_ref_base_and_extent (tree exp, poly_int64 *poffset,
>>
>> value_range vr;
>> range_query *query;
>> - if (cfun)
>> - query = get_range_query (cfun);
>> - else
>> - query = get_global_range_query ();
>> + query = get_range_query (cfun);
>>
>> if (TREE_CODE (index) == SSA_NAME
>> && (low_bound = array_ref_low_bound (exp),
>> diff --git a/gcc/tree-ssa-loop-split.cc b/gcc/tree-ssa-loop-split.cc
>> index 64464802c1e..e85a1881526 100644
>> --- a/gcc/tree-ssa-loop-split.cc
>> +++ b/gcc/tree-ssa-loop-split.cc
>> @@ -145,7 +145,7 @@ split_at_bb_p (class loop *loop, basic_block bb, tree *border, affine_iv *iv,
>> else
>> {
>> int_range<2> r;
>> - get_global_range_query ()->range_of_expr (r, op0, stmt);
>> + get_range_query (cfun)->range_of_expr (r, op0, stmt);
>
> loop splitting doesn't have a ranger instance so this is a no-op change
> but I'm also not sure it would be safe to use a dynamic ranger instance
> here since we are doing even CFG manipulations between. Please leave
> this change out.
Oh, yes, get_global_range_query would be prefer here.
>
>> if (!r.varying_p () && !r.undefined_p ()
>> && TREE_CODE (op1) == INTEGER_CST)
>> {
>> diff --git a/gcc/tree-ssa-loop-unswitch.cc b/gcc/tree-ssa-loop-unswitch.cc
>> index 619b50fb4bb..b3dc2ded931 100644
>> --- a/gcc/tree-ssa-loop-unswitch.cc
>> +++ b/gcc/tree-ssa-loop-unswitch.cc
>> @@ -764,7 +764,7 @@ evaluate_control_stmt_using_entry_checks (gimple *stmt,
>>
>> int_range_max r;
>> if (!ranger->gori ().outgoing_edge_range_p (r, e, idx,
>> - *get_global_range_query ()))
>> + *get_range_query (cfun)))
>> continue;
>
> unswitching has a ranger instance but it does perform IL modification.
> Did you check whether the use of the global ranger was intentional here?
> Specifically we do have the 'ranger' object here and IIRC using global
> ranges was intentional. So please leave this change out.
Thanks for pointing this out!
BR,
Jeff (Jiufu Guo)
>
> Thanks,
> Richard.
>
>> r.intersect (path_range);
>> if (r.undefined_p ())
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-10 9:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-10 2:53 [PATCH] use get_range_query to replace get_global_range_query Jiufu Guo
2023-10-10 7:01 ` Richard Biener
2023-10-10 7:23 ` Andrew Pinski
2023-10-10 9:07 ` Jiufu Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).