Hi all, I've committed this and with one more change.
--- gcc/loop-iv.c (revision 272731) +++ gcc/loop-iv.c (working copy) @@ -3004,7 +3004,7 @@ find_simple_exit (struct loop *loop, struct niter_ well. It results in incorrect predicate information on the exit condition expression. For example, if says [(int) _1 + -8, + , -8] != 0 finite, it means _1 can exactly divide -8. */ - if (single_exit (loop) && finite_loop_p (loop)) + if (desc->infinite && single_exit (loop) && finite_loop_p (loop)) { desc->infinite = NULL_RTX; if (dump_file) on 2019/6/26 上午11:45, Kewen.Lin wrote: > Hi Jeff, > > on 2019/6/26 上午5:49, Jeff Law wrote: >> On 6/25/19 3:41 AM, Kewen.Lin wrote: >>> Hi Richard, >>> >>> Thanks a lot for review comments. >>> >>> on 2019/6/25 下午3:23, Richard Biener wrote: >>>> On Tue, 25 Jun 2019, Kewen.Lin wrote: >>>> >>>>> Hi all, >>>>> >>>>> >>>>> It's based on two observations: >>>>> 1) the loop structure for one specific loop is shared between >>>>> middle-end and >>>>> back-end. >>>>> 2) for one specific loop, if it's finite then never become infinite >>>>> itself. >>>>> >>>>> As one gcc newbie, I'm not sure whether these two observations are true >>>>> in all >>>>> cases. Please feel free to correct me if anything missing. >>>> >>>> I think 2) is not true with -ffinite-loops. >>> >>> I just looked at the patch on this option, I don't fully understand it can >>> affect >>> 2). It's to take one loop as finite with any normal exit, can some loop >>> with this >>> assertion turn into infinite later by some other analysis? >>> >>>> >>>>> btw, I also took a look at how the loop constraint LOOP_C_FINITE is used, >>>>> I think >>>>> it's not suitable for this purpose, it's mainly set by vectorizer and >>>>> tell niter >>>>> and scev to take one loop as finite. The original patch has the words >>>>> "constraint flag is mainly set by consumers and affects certain semantics >>>>> of >>>>> niter analyzer APIs". >>>>> >>>>> Bootstrapped and regression testing passed on >>>>> powerpc64le-unknown-linux-gnu. >>>> >>>> Did you consider to simply use finite_loop_p () from doloop.c? That >>>> would be a much simpler patch. >>> >>> Good suggestion! I took it for granted that the function can be only >>> efficient in >>> middle-end, but actually some information like bit any_upper_bound could be >>> kept to >>> RTL. >>> >>>> >>>> For the testcase in question -ffinite-loops would provide this guarantee >>>> even on RTL, so would the upper bound that may be still set. >>>> >>>> Richard. >>>> >>> >>> The new version with Richard's suggestion listed below. >>> Regression testing is ongoing. >>> >>> >>> Thanks, >>> Kewen >>> >>> ----------- >>> >>> gcc/ChangeLog >>> >>> 2019-06-25 Kewen Lin <li...@gcc.gnu.org> >>> >>> PR target/62147 >>> * gcc/loop-iv.c (find_simple_exit): Call finite_loop_p to update >>> finiteness. >>> >>> gcc/testsuite/ChangeLog >>> >>> 2019-06-25 Kewen Lin <li...@gcc.gnu.org> >>> >>> PR target/62147 >>> * gcc.target/powerpc/pr62147.c: New test. >> This is fine assuming regression testing was OK. >> > > Thanks Jeff! Bootstrapped and regression testing passed on > powerpc64le-unknown-linux-gnu. > >> One might argue that "finite_loop_p" belongs elsewhere since it's not >> really querying tree/gimple structures. > > I guess it will do something gimple specific (estimate_numbers_of_iterations) > when it can > so it was placed there. > > > Thanks, > Kewen > >> >> jeff >> >