From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id E8AA83856DD6; Tue, 10 Oct 2023 07:01:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E8AA83856DD6 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id E72AB21855; Tue, 10 Oct 2023 07:01:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1696921295; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ctzbbzGpQPC3W9Cl20rP73imsU2yCfWEhmNRzrod9Lo=; b=frm58Vz9H5P8rQXnpAP5I8D8D+UcyUWVTb4XhVHppab6mAE9wKBQNpiupkyw4kBQCsT6i9 EqyMqLpf0TrcNcJcsMgWccpfN+3RJanIiC91sQ/7xCxnKpXdAosZjwgt+MgpOenvuJVmf0 XxlH5HOnkk9i/PaeW21CEfOhMkfYSNM= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1696921295; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ctzbbzGpQPC3W9Cl20rP73imsU2yCfWEhmNRzrod9Lo=; b=mJq56rKCfaUP6yc09RbIKOd97ASAHAxfaLiWbHqVuHD5+wIoN7fpKvQByNNgf5sI0Wos2f i8XkwFvRiNhVhmBg== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id A63532C363; Tue, 10 Oct 2023 07:01:35 +0000 (UTC) Date: Tue, 10 Oct 2023 07:01:35 +0000 (UTC) From: Richard Biener To: Jiufu Guo cc: gcc-patches@gcc.gnu.org, jeffreyalaw@gmail.com, richard.sandiford@arm.com, segher@kernel.crashing.org, dje.gcc@gmail.com, linkw@gcc.gnu.org, bergner@linux.ibm.com, amacleod@redhat.com, aldyh@redhat.com Subject: Re: [PATCH] use get_range_query to replace get_global_range_query In-Reply-To: <20231010025311.3642757-1-guojiufu@linux.ibm.com> Message-ID: References: <20231010025311.3642757-1-guojiufu@linux.ibm.com> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)