| Message ID | DB5PR0801MB2742CA4F16D94A39726A9030E76F0@DB5PR0801MB2742.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PR82163] Rewrite loop into lcssa form instantly | expand |
On Thu, Sep 14, 2017 at 5:02 PM, Bin Cheng <Bin.Cheng@arm.com> wrote: > Hi, > Current pcom implementation rewrites into lcssa form after all loops are transformed, this is > not enough because unrolling of later loop checks lcssa form in function tree_transform_and_unroll_loop. > This simple patch rewrites loop into lcssa form if store-store chain is handled. I think it doesn't > affect compilation time since rewrite_into_loop_closed_ssa_1 is only called for store-store chain > transformation and only the transformed loop is rewritten. Well, it may look like only the transformed loop is rewritten -- yes, it is, but rewrite_into_loop_closed_ssa calls update_ssa () which operates on the whole function. So I'd rather _not_ do this. Is there a real problem or is it just the overly aggressive checking done? IMHO we should remove the checking or pass in a param whether to skip the checking. Or even better, restrict the checking to those loops trans_form_and_unroll actually touches. Richard. > Bootstrap and test ongoing on x86_64. is it OK if no failures? > > Thanks, > bin > 2017-09-14 Bin Cheng <bin.cheng@arm.com> > > PR tree-optimization/82163 > * tree-predcom.c (tree_predictive_commoning_loop): Rewrite into > loop closed ssa instantly. Return boolean true if loop is unrolled. > (tree_predictive_commoning): Return TODO_cleanup_cfg if loop is > unrolled. > > gcc/testsuite > 2017-09-14 Bin Cheng <bin.cheng@arm.com> > > PR tree-optimization/82163 > * gcc.dg/tree-ssa/pr82163.c: New test.
On Fri, Sep 15, 2017 at 12:49 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Sep 14, 2017 at 5:02 PM, Bin Cheng <Bin.Cheng@arm.com> wrote: >> Hi, >> Current pcom implementation rewrites into lcssa form after all loops are transformed, this is >> not enough because unrolling of later loop checks lcssa form in function tree_transform_and_unroll_loop. >> This simple patch rewrites loop into lcssa form if store-store chain is handled. I think it doesn't >> affect compilation time since rewrite_into_loop_closed_ssa_1 is only called for store-store chain >> transformation and only the transformed loop is rewritten. > > Well, it may look like only the transformed loop is rewritten -- yes, > it is, but rewrite_into_loop_closed_ssa > calls update_ssa () which operates on the whole function. I see. > > So I'd rather _not_ do this. > > Is there a real problem or is it just the overly aggressive checking > done? IMHO we should remove In this case, it's the check itself. > the checking or pass in a param whether to skip the checking. Or even > better, restrict the > checking to those loops trans_form_and_unroll actually touches. Yes, will see if we can check loops only related to trans_form_and_unroll. Thanks, bin > > Richard. > >> Bootstrap and test ongoing on x86_64. is it OK if no failures? >> >> Thanks, >> bin >> 2017-09-14 Bin Cheng <bin.cheng@arm.com> >> >> PR tree-optimization/82163 >> * tree-predcom.c (tree_predictive_commoning_loop): Rewrite into >> loop closed ssa instantly. Return boolean true if loop is unrolled. >> (tree_predictive_commoning): Return TODO_cleanup_cfg if loop is >> unrolled. >> >> gcc/testsuite >> 2017-09-14 Bin Cheng <bin.cheng@arm.com> >> >> PR tree-optimization/82163 >> * gcc.dg/tree-ssa/pr82163.c: New test.
On Fri, Sep 15, 2017 at 3:04 PM, Bin.Cheng <amker.cheng@gmail.com> wrote: > On Fri, Sep 15, 2017 at 12:49 PM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Thu, Sep 14, 2017 at 5:02 PM, Bin Cheng <Bin.Cheng@arm.com> wrote: >>> Hi, >>> Current pcom implementation rewrites into lcssa form after all loops are transformed, this is >>> not enough because unrolling of later loop checks lcssa form in function tree_transform_and_unroll_loop. >>> This simple patch rewrites loop into lcssa form if store-store chain is handled. I think it doesn't >>> affect compilation time since rewrite_into_loop_closed_ssa_1 is only called for store-store chain >>> transformation and only the transformed loop is rewritten. >> >> Well, it may look like only the transformed loop is rewritten -- yes, >> it is, but rewrite_into_loop_closed_ssa >> calls update_ssa () which operates on the whole function. > I see. >> >> So I'd rather _not_ do this. >> >> Is there a real problem or is it just the overly aggressive checking >> done? IMHO we should remove > In this case, it's the check itself. >> the checking or pass in a param whether to skip the checking. Or even >> better, restrict the >> checking to those loops trans_form_and_unroll actually touches. > Yes, will see if we can check loops only related to trans_form_and_unroll. That would be very welcome -- it's really useful to be able to assert you didn't break LCSSA when transforming a single loop (from our generic infrastructure routines). Richard. > Thanks, > bin >> >> Richard. >> >>> Bootstrap and test ongoing on x86_64. is it OK if no failures? >>> >>> Thanks, >>> bin >>> 2017-09-14 Bin Cheng <bin.cheng@arm.com> >>> >>> PR tree-optimization/82163 >>> * tree-predcom.c (tree_predictive_commoning_loop): Rewrite into >>> loop closed ssa instantly. Return boolean true if loop is unrolled. >>> (tree_predictive_commoning): Return TODO_cleanup_cfg if loop is >>> unrolled. >>> >>> gcc/testsuite >>> 2017-09-14 Bin Cheng <bin.cheng@arm.com> >>> >>> PR tree-optimization/82163 >>> * gcc.dg/tree-ssa/pr82163.c: New test.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr82163.c b/gcc/testsuite/gcc.dg/tree-ssa/pr82163.c new file mode 100644 index 0000000..fef2b1d --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr82163.c @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "-O3" } */ + +int a, b, c[4], d, e, f, g; + +void h () +{ + for (; a; a++) + { + c[a + 3] = g; + if (b) + c[a] = f; + else + { + for (; d; d++) + c[d + 3] = c[d]; + for (e = 1; e == 2; e++) + ; + if (e) + break; + } + } +} diff --git a/gcc/tree-predcom.c b/gcc/tree-predcom.c index e7b10cb..ffbe332 100644 --- a/gcc/tree-predcom.c +++ b/gcc/tree-predcom.c @@ -3014,11 +3014,10 @@ insert_init_seqs (struct loop *loop, vec<chain_p> chains) } } -/* Performs predictive commoning for LOOP. Sets bit 1<<0 of return value - if LOOP was unrolled; Sets bit 1<<1 of return value if loop closed ssa - form was corrupted. */ +/* Performs predictive commoning for LOOP. Returns true if LOOP was + unrolled. */ -static unsigned +static bool tree_predictive_commoning_loop (struct loop *loop) { vec<data_reference_p> datarefs; @@ -3154,7 +3153,13 @@ end: ; free_affine_expand_cache (&name_expansions); - return (unroll ? 1 : 0) | (loop_closed_ssa ? 2 : 0); + /* Rewrite loop into loop closed ssa form if necessary. We can not do it + after all loops are transformed because unrolling of later loop checks + loop closed ssa form. */ + if (loop_closed_ssa) + rewrite_into_loop_closed_ssa_1 (NULL, 0, SSA_OP_USE, loop); + + return unroll; } /* Runs predictive commoning. */ @@ -3163,7 +3168,7 @@ unsigned tree_predictive_commoning (void) { struct loop *loop; - unsigned ret = 0, changed = 0; + bool changed = 0; initialize_original_copy_tables (); FOR_EACH_LOOP (loop, LI_ONLY_INNERMOST) @@ -3173,17 +3178,13 @@ tree_predictive_commoning (void) } free_original_copy_tables (); - if (changed > 0) + if (changed) { scev_reset (); - - if (changed > 1) - rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa); - - ret = TODO_cleanup_cfg; + return TODO_cleanup_cfg; } - return ret; + return 0; } /* Predictive commoning Pass. */