From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id E51193858404 for ; Mon, 3 Jan 2022 14:30:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E51193858404 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id B7E582110B; Mon, 3 Jan 2022 14:30:50 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (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 8C206A3BC3; Mon, 3 Jan 2022 14:30:50 +0000 (UTC) Date: Mon, 3 Jan 2022 15:30:50 +0100 (CET) From: Richard Biener To: Jiufu Guo cc: gcc-patches@gcc.gnu.org, amker.cheng@gmail.com, wschmidt@linux.ibm.com, segher@kernel.crashing.org, dje.gcc@gmail.com, jlaw@tachyum.com, Jan Hubicka Subject: Re: [PATCH] disable aggressive_loop_optimizations until niter ready In-Reply-To: <20211222022617.33192-1-guojiufu@linux.ibm.com> Message-ID: <17p3q4r5-31q1-s990-8q64-2s835p396q57@fhfr.qr> References: <20211222022617.33192-1-guojiufu@linux.ibm.com> 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, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Jan 2022 14:30:53 -0000 On Wed, 22 Dec 2021, Jiufu Guo wrote: > Hi, > > Normaly, estimate_numbers_of_iterations get/caculate niter first, > and then invokes infer_loop_bounds_from_undefined. While in some case, > after a few call stacks, estimate_numbers_of_iterations is invoked before > niter is ready (e.g. before number_of_latch_executions returns). > > e.g. number_of_latch_executions->...follow_ssa_edge_expr--> > --> estimate_numbers_of_iterations --> infer_loop_bounds_from_undefined. > > Since niter is still not computed, call to infer_loop_bounds_from_undefined > may not get final result. > To avoid infer_loop_bounds_from_undefined to be called with interim state > and avoid infer_loop_bounds_from_undefined generates interim data, during > niter's computing, we could disable flag_aggressive_loop_optimizations. > > Bootstrap and regtest pass on ppc64* and x86_64. Is this ok for trunk? So this is a optimality fix, not a correctness one? I suppose the estimates are computed/used from scev_probably_wraps_p via loop_exits_before_overflow and ultimatively chrec_convert. We have a call cycle here, estimate_numbers_of_iterations -> number_of_latch_executions -> ... -> estimate_numbers_of_iterations where the first estimate_numbers_of_iterations will make sure the later call will immediately return. I'm not sure what your patch tries to do - it seems to tackle the case where we enter the cycle via number_of_latch_executions? Why do we get "non-final" values? idx_infer_loop_bounds resorts to SCEV and thus may recurse again - to me it would be more logical to try avoid recursing in number_of_latch_executions by setting ->nb_iterations to something early, maybe chrec_dont_know, to signal we're using something we're just trying to compute. Richard. > BR, > Jiufu > > gcc/ChangeLog: > > * tree-ssa-loop-niter.c (number_of_iterations_exit_assumptions): > Disable/restore flag_aggressive_loop_optimizations. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/scev-16.c: New test. > > --- > gcc/tree-ssa-loop-niter.c | 23 +++++++++++++++++++---- > gcc/testsuite/gcc.dg/tree-ssa/scev-16.c | 20 ++++++++++++++++++++ > 2 files changed, 39 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/scev-16.c > > diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c > index 06954e437f5..51bb501019e 100644 > --- a/gcc/tree-ssa-loop-niter.c > +++ b/gcc/tree-ssa-loop-niter.c > @@ -2534,18 +2534,31 @@ number_of_iterations_exit_assumptions (class loop *loop, edge exit, > && !POINTER_TYPE_P (type)) > return false; > > + /* Before niter is calculated, avoid to analyze interim state. */ > + int old_aggressive_loop_optimizations = flag_aggressive_loop_optimizations; > + flag_aggressive_loop_optimizations = 0; > + > tree iv0_niters = NULL_TREE; > if (!simple_iv_with_niters (loop, loop_containing_stmt (stmt), > op0, &iv0, safe ? &iv0_niters : NULL, false)) > - return number_of_iterations_popcount (loop, exit, code, niter); > + { > + bool res = number_of_iterations_popcount (loop, exit, code, niter); > + flag_aggressive_loop_optimizations = old_aggressive_loop_optimizations; > + return res; > + } > tree iv1_niters = NULL_TREE; > if (!simple_iv_with_niters (loop, loop_containing_stmt (stmt), > op1, &iv1, safe ? &iv1_niters : NULL, false)) > - return false; > + { > + flag_aggressive_loop_optimizations = old_aggressive_loop_optimizations; > + return false; > + } > /* Give up on complicated case. */ > if (iv0_niters && iv1_niters) > - return false; > - > + { > + flag_aggressive_loop_optimizations = old_aggressive_loop_optimizations; > + return false; > + } > /* We don't want to see undefined signed overflow warnings while > computing the number of iterations. */ > fold_defer_overflow_warnings (); > @@ -2565,6 +2578,7 @@ number_of_iterations_exit_assumptions (class loop *loop, edge exit, > only_exit_p, safe)) > { > fold_undefer_and_ignore_overflow_warnings (); > + flag_aggressive_loop_optimizations = old_aggressive_loop_optimizations; > return false; > } > > @@ -2608,6 +2622,7 @@ number_of_iterations_exit_assumptions (class loop *loop, edge exit, > niter->may_be_zero); > > fold_undefer_and_ignore_overflow_warnings (); > + flag_aggressive_loop_optimizations = old_aggressive_loop_optimizations; > > /* If NITER has simplified into a constant, update MAX. */ > if (TREE_CODE (niter->niter) == INTEGER_CST) > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-16.c b/gcc/testsuite/gcc.dg/tree-ssa/scev-16.c > new file mode 100644 > index 00000000000..708ffab88ca > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-16.c > @@ -0,0 +1,20 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-ivopts-scev" } */ > + > +/* Expect scalar_evolution = {(int) start_7(D), +, 1}_1), instead > + (int) (short int) {(unsigned short) start_7(D), +, 1}_1 */ > + > +int arr[1000]; > + > +void > +s2i (short start, short end) > +{ > + int res = 0; > + for (short i = start; i < end; i++) > + { > + int lv = i; > + arr[lv] += lv; > + } > +} > + > +/* { dg-final { scan-tree-dump-times "scalar_evolution = \{\\(int\\) start_\[0-9\]+\\(D\\), \\+, 1\}_1" 1 "ivopts" } } */ > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)