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 >
