On Mon, Jul 9, 2018 at 9:05 AM Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org> wrote: > > Hi Richard, > > Thanks for the review. > > On 6 July 2018 at 20:17, Richard Biener <richard.guent...@gmail.com> wrote: > > On Fri, Jul 6, 2018 at 11:45 AM Kugan Vivekanandarajah > > <kugan.vivekanandara...@linaro.org> wrote: > >> > >> Hi Richard, > >> > >> > It was rewrite_to_non_trapping_overflow available in tree.h. Thus > >> > final value replacement > >> > could use that before gimplifying instead of using > >> > rewrite_to_defined_overflow > >> Thanks. > >> > >> Is the attached patch OK? I am testing this on x86_64-linux-gnu and if > >> there is no new regressions. > > > > Please clean up the control flow to > > > > if (...) > > def = rewrite_to_non_trapping_overflow (def); > > def = force_gimple_operand_gsi (&gsi, def, false, NULL_TREE, > > true, GSI_SAME_STMT); > > I also had to add flag_trapv like we do in other places (for > flag_wrapv) when calling rewrite_to_non_trapping_overflow. Attached > patch bootstraps and there is no new regressions in x86-64-linux-gnu. > Is this OK?
Aww, no. Sorry for misleading you - final value replacement - in addition to the trapping issue - needs to make sure to not introduce undefined overflow as well. So the rewrite_to_non_trapping_overflow should avoid the gimplification issue you ran into (and thus the extra predicate in expression_expensive) but you can't remove the rewrite_to_defined_overflow code. So, can you rework things again, doing the rewrtite_to_non_trapping_overflow but keep the rewrite_to_defined_overflow code as well? The you should be able to remove the generic_xpr_could_trap_p checks (and TREE_SIDE_EFFECTS). Thanks, Richard. > Thanks, > Kugan > > > > OK with that change. > > Richard. > > > >> Thanks, > >> Kugan > >> > >> gcc/ChangeLog: > >> > >> 2018-07-06 Kugan Vivekanandarajah <kug...@linaro.org> > >> > >> * tree-scalar-evolution.c (final_value_replacement_loop): Use > >> rewrite_to_non_trapping_overflow instead of > >> rewrite_to_defined_overflow.
