From patchwork Tue Oct 9 09:36:59 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Fix my change to unroll_loop_constant_iterations X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 190249 Message-Id: <20121009093659.GA14364@kam.mff.cuni.cz> To: Jan Hubicka Cc: gcc-patches@gcc.gnu.org Date: Tue, 9 Oct 2012 11:36:59 +0200 From: Jan Hubicka List-Id: > H, > I hope this change is responsible for today misoptimizations of SPEC at -O3. > While updating unroll_loop_constant_iterations and fighting with double_int > operations I made two trivial mistakes. > > I am bootstrapping/regtesting the following and will commit it as obvious if > it passes. > > Honza > > * loop-unroll.c (unroll_loop_constant_iterations): Fix previous patch. Unforutnately there is one other case where one has to be extra cureful about overflows. This is in fact latent bug in the unroller, but it now reproduce with -O3 -fpeel-loops bootstrap because estimates tends to be close to maximum of the integer types. I am re-starting testing with the following patch and will commit if passes (but i will be till 2pm on bus). My apologizes for the breakage. Honza Index: loop-unroll.c =================================================================== --- loop-unroll.c (revision 192240) +++ loop-unroll.c (working copy) @@ -740,6 +740,7 @@ unroll_loop_constant_iterations (struct apply_opt_in_copies (opt_info, exit_mod + 1, false, false); desc->niter -= exit_mod + 1; + loop->nb_iterations_upper_bound -= double_int::from_uhwi (exit_mod + 1); if (loop->any_estimate && double_int::from_uhwi (exit_mod + 1).ule (loop->nb_iterations_estimate)) @@ -795,12 +796,12 @@ unroll_loop_constant_iterations (struct desc->niter /= max_unroll + 1; loop->nb_iterations_upper_bound - = loop->nb_iterations_upper_bound.udiv (double_int::from_uhwi (exit_mod + = loop->nb_iterations_upper_bound.udiv (double_int::from_uhwi (max_unroll + 1), FLOOR_DIV_EXPR); if (loop->any_estimate) loop->nb_iterations_estimate - = loop->nb_iterations_estimate.udiv (double_int::from_uhwi (exit_mod + = loop->nb_iterations_estimate.udiv (double_int::from_uhwi (max_unroll + 1), FLOOR_DIV_EXPR); desc->niter_expr = GEN_INT (desc->niter); @@ -879,8 +880,7 @@ decide_unroll_runtime_iterations (struct /* If we have profile feedback, check whether the loop rolls. */ if ((estimated_loop_iterations (loop, &iterations) || max_loop_iterations (loop, &iterations)) - && iterations.fits_shwi () - && iterations.to_shwi () <= 2 * nunroll) + && iterations.ule (double_int::from_shwi (2 * nunroll))) { if (dump_file) fprintf (dump_file, ";; Not unrolling loop, doesn't roll\n"); @@ -1280,8 +1280,7 @@ decide_peel_simple (struct loop *loop, i /* If we have realistic estimate on number of iterations, use it. */ if (estimated_loop_iterations (loop, &iterations)) { - if (!iterations.fits_shwi () - || iterations.to_shwi () + 1 > npeel) + if (double_int::from_shwi (npeel).ule (iterations)) { if (dump_file) { @@ -1298,8 +1297,7 @@ decide_peel_simple (struct loop *loop, i /* If we have small enough bound on iterations, we can still peel (completely unroll). */ else if (max_loop_iterations (loop, &iterations) - && iterations.fits_shwi () - && iterations.to_shwi () + 1 <= npeel) + && iterations.ult (double_int::from_shwi (npeel))) npeel = iterations.to_shwi () + 1; else { @@ -1449,8 +1447,7 @@ decide_unroll_stupid (struct loop *loop, /* If we have profile feedback, check whether the loop rolls. */ if ((estimated_loop_iterations (loop, &iterations) || max_loop_iterations (loop, &iterations)) - && iterations.fits_shwi () - && iterations.to_shwi () <= 2 * nunroll) + && iterations.ule (double_int::from_shwi (2 * nunroll))) { if (dump_file) fprintf (dump_file, ";; Not unrolling loop, doesn't roll\n");