| Message ID | CAAkRFZK60Yer3SOds_=KbXzvqfz3EBfcxLFJEZdpKVSXiVFMvw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Does this one look ok? thanks, David On Thu, Aug 22, 2013 at 4:20 PM, Xinliang David Li <davidxl@google.com> wrote: > Hi, In this patch, loop alignment peeling and loop versioning > transformation will be reported via -fopt-info by default. This will > help vectorizer size tuning. > > It also enhances the opt-info dump to include current function name as > context. Otherwise, we may see duplicate messeages from inline/cloned > instances. > > An example opt report: > > > > b.c:16:A::foo: note: Loop is vectorized > > b.c:16:A::foo: note: Loop is versioned to remove aliases for vectorization > > b.c:16:A::foo: note: Loop is peeled to enhance alignment for vectorization > > b.c:16:A::foo: note: Completely unroll loop 6 times > > b.c:12:A::foo: note: Completely unroll loop 6 times > > > Ok after testing? > > thanks, > > David
My only concern is whether the dump messages will get too long with the full function name on the same line. The infrastructure that emits inform() notes ensures that the function name is printed before each block of messages related to that function (via an "In function foo:" type message), but I had found before that the new dump infrastructure doesn't do that. OTOH, your approach will make it much easier to grep the output of a large build. Personally, I use grep on this type of output enough to make the longer lines worth it. Either that or the new dump infrastructure needs to be fixed to emit the function name before each block of messages, a la inform(). Thanks, Teresa On Tue, Aug 27, 2013 at 11:22 AM, Xinliang David Li <davidxl@google.com> wrote: > Does this one look ok? > > thanks, > > David > > On Thu, Aug 22, 2013 at 4:20 PM, Xinliang David Li <davidxl@google.com> wrote: >> Hi, In this patch, loop alignment peeling and loop versioning >> transformation will be reported via -fopt-info by default. This will >> help vectorizer size tuning. >> >> It also enhances the opt-info dump to include current function name as >> context. Otherwise, we may see duplicate messeages from inline/cloned >> instances. >> >> An example opt report: >> >> >> >> b.c:16:A::foo: note: Loop is vectorized >> >> b.c:16:A::foo: note: Loop is versioned to remove aliases for vectorization >> >> b.c:16:A::foo: note: Loop is peeled to enhance alignment for vectorization >> >> b.c:16:A::foo: note: Completely unroll loop 6 times >> >> b.c:12:A::foo: note: Completely unroll loop 6 times >> >> >> Ok after testing? >> >> thanks, >> >> David
yes -- the long unmangled names can be annoying -- that is why I chose to dump the short form of the function names -- combined with line numbers, it should be enough to get the full context. David On Tue, Aug 27, 2013 at 11:36 AM, Teresa Johnson <tejohnson@google.com> wrote: > My only concern is whether the dump messages will get too long with > the full function name on the same line. The infrastructure that emits > inform() notes ensures that the function name is printed before each > block of messages related to that function (via an "In function foo:" > type message), but I had found before that the new dump infrastructure > doesn't do that. OTOH, your approach will make it much easier to grep > the output of a large build. Personally, I use grep on this type of > output enough to make the longer lines worth it. Either that or the > new dump infrastructure needs to be fixed to emit the function name > before each block of messages, a la inform(). > > Thanks, > Teresa > > On Tue, Aug 27, 2013 at 11:22 AM, Xinliang David Li <davidxl@google.com> wrote: >> Does this one look ok? >> >> thanks, >> >> David >> >> On Thu, Aug 22, 2013 at 4:20 PM, Xinliang David Li <davidxl@google.com> wrote: >>> Hi, In this patch, loop alignment peeling and loop versioning >>> transformation will be reported via -fopt-info by default. This will >>> help vectorizer size tuning. >>> >>> It also enhances the opt-info dump to include current function name as >>> context. Otherwise, we may see duplicate messeages from inline/cloned >>> instances. >>> >>> An example opt report: >>> >>> >>> >>> b.c:16:A::foo: note: Loop is vectorized >>> >>> b.c:16:A::foo: note: Loop is versioned to remove aliases for vectorization >>> >>> b.c:16:A::foo: note: Loop is peeled to enhance alignment for vectorization >>> >>> b.c:16:A::foo: note: Completely unroll loop 6 times >>> >>> b.c:12:A::foo: note: Completely unroll loop 6 times >>> >>> >>> Ok after testing? >>> >>> thanks, >>> >>> David > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
On Aug 27, 2013, at 11:22 AM, Xinliang David Li <davidxl@google.com> wrote: > Does this one look ok? We don't capitalize text after error:, warning: or note:. > thanks, > > David > > On Thu, Aug 22, 2013 at 4:20 PM, Xinliang David Li <davidxl@google.com> wrote: >> Hi, In this patch, loop alignment peeling and loop versioning >> transformation will be reported via -fopt-info by default. This will >> help vectorizer size tuning. >> >> It also enhances the opt-info dump to include current function name as >> context. Otherwise, we may see duplicate messeages from inline/cloned >> instances. >> >> An example opt report: >> >> >> >> b.c:16:A::foo: note: Loop is vectorized >> >> b.c:16:A::foo: note: Loop is versioned to remove aliases for vectorization >> >> b.c:16:A::foo: note: Loop is peeled to enhance alignment for vectorization >> >> b.c:16:A::foo: note: Completely unroll loop 6 times >> >> b.c:12:A::foo: note: Completely unroll loop 6 times >> >> >> Ok after testing? >> >> thanks, >> >> David
If this is the convention, we should probably have another patch to fix all the existing opt-info messages. thanks, David On Tue, Aug 27, 2013 at 1:23 PM, Mike Stump <mikestump@comcast.net> wrote: > On Aug 27, 2013, at 11:22 AM, Xinliang David Li <davidxl@google.com> wrote: >> Does this one look ok? > > We don't capitalize text after error:, warning: or note:. > >> thanks, >> >> David >> >> On Thu, Aug 22, 2013 at 4:20 PM, Xinliang David Li <davidxl@google.com> wrote: >>> Hi, In this patch, loop alignment peeling and loop versioning >>> transformation will be reported via -fopt-info by default. This will >>> help vectorizer size tuning. >>> >>> It also enhances the opt-info dump to include current function name as >>> context. Otherwise, we may see duplicate messeages from inline/cloned >>> instances. >>> >>> An example opt report: >>> >>> >>> >>> b.c:16:A::foo: note: Loop is vectorized >>> >>> b.c:16:A::foo: note: Loop is versioned to remove aliases for vectorization >>> >>> b.c:16:A::foo: note: Loop is peeled to enhance alignment for vectorization >>> >>> b.c:16:A::foo: note: Completely unroll loop 6 times >>> >>> b.c:12:A::foo: note: Completely unroll loop 6 times >>> >>> >>> Ok after testing? >>> >>> thanks, >>> >>> David >
On Tue, Aug 27, 2013 at 10:30 PM, Xinliang David Li <davidxl@google.com> wrote: > If this is the convention, we should probably have another patch to > fix all the existing opt-info messages. Yes please. Also ... >>>> b.c:16:A::foo: note: Loop is vectorized "loop vectorized" >>>> >>>> b.c:16:A::foo: note: Loop is versioned to remove aliases for vectorization "loop versioned for vectorization because of possible aliasing" >>>> b.c:16:A::foo: note: Loop is peeled to enhance alignment for vectorization "loop peeled for vectorization to enhance alignment" >>>> b.c:16:A::foo: note: Completely unroll loop 6 times maybe "loop with 6 iterations completely unrolled" >>>> >>>> b.c:12:A::foo: note: Completely unroll loop 6 times >>>> I hate the excessive vertical spacing as well. Richard. >>>> Ok after testing? >>>> >>>> thanks, >>>> >>>> David >>
On Wed, Aug 28, 2013 at 2:45 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Tue, Aug 27, 2013 at 10:30 PM, Xinliang David Li <davidxl@google.com> wrote: >> If this is the convention, we should probably have another patch to >> fix all the existing opt-info messages. > > Yes please. > > Also ... > > >>>>> b.c:16:A::foo: note: Loop is vectorized > > "loop vectorized" > >>>>> >>>>> b.c:16:A::foo: note: Loop is versioned to remove aliases for vectorization > > "loop versioned for vectorization because of possible aliasing" > >>>>> b.c:16:A::foo: note: Loop is peeled to enhance alignment for vectorization > > "loop peeled for vectorization to enhance alignment" > >>>>> b.c:16:A::foo: note: Completely unroll loop 6 times > > maybe "loop with 6 iterations completely unrolled" > >>>>> >>>>> b.c:12:A::foo: note: Completely unroll loop 6 times >>>>> > > I hate the excessive vertical spacing as well. This part should be fixable after my related patch that you reviewed (the Convert more passes to new dump framework patch). The problem is that the dump framework was not consistently emitting newlines, so many of the messages were explicitly adding a newline, which often wasn't needed. I'll respond more on that thread, but for cleanup such as removing extra newlines and adjusting the capitalization, I can send a separate cleanup patch after these are in. Teresa > > Richard. > >>>>> Ok after testing? >>>>> >>>>> thanks, >>>>> >>>>> David >>>
Index: tree-vectorizer.c =================================================================== --- tree-vectorizer.c (revision 201751) +++ tree-vectorizer.c (working copy) @@ -119,7 +119,7 @@ vectorize_loops (void) if (LOCATION_LOCUS (vect_location) != UNKNOWN_LOC && dump_enabled_p ()) dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location, - "Vectorized loop\n"); + "Loop is vectorized\n"); vect_transform_loop (loop_vinfo); num_vectorized_loops++; } @@ -180,7 +180,7 @@ execute_vect_slp (void) vect_slp_transform_bb (bb); if (dump_enabled_p ()) dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location, - "Vectorized basic-block\n"); + "Basic block is vectorized\n"); } } Index: tree-vect-loop-manip.c =================================================================== --- tree-vect-loop-manip.c (revision 201751) +++ tree-vect-loop-manip.c (working copy) @@ -2021,8 +2021,9 @@ vect_do_peeling_for_alignment (loop_vec_ int bound = 0; if (dump_enabled_p ()) - dump_printf_loc (MSG_NOTE, vect_location, - "=== vect_do_peeling_for_alignment ==="); + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location, + "Loop is peeled to enhance" + " alignment for vectorization\n"); initialize_original_copy_tables (); @@ -2404,6 +2405,8 @@ vect_loop_versioning (loop_vec_info loop unsigned prob = 4 * REG_BR_PROB_BASE / 5; gimple_seq gimplify_stmt_list = NULL; tree scalar_loop_iters = LOOP_VINFO_NITERS (loop_vinfo); + bool version_align = LOOP_REQUIRES_VERSIONING_FOR_ALIGNMENT (loop_vinfo); + bool version_alias = LOOP_REQUIRES_VERSIONING_FOR_ALIAS (loop_vinfo); if (check_profitability) { @@ -2413,11 +2416,11 @@ vect_loop_versioning (loop_vec_info loop is_gimple_condexpr, NULL_TREE); } - if (LOOP_REQUIRES_VERSIONING_FOR_ALIGNMENT (loop_vinfo)) + if (version_align) vect_create_cond_for_align_checks (loop_vinfo, &cond_expr, &cond_expr_stmt_list); - if (LOOP_REQUIRES_VERSIONING_FOR_ALIAS (loop_vinfo)) + if (version_alias) vect_create_cond_for_alias_checks (loop_vinfo, &cond_expr); cond_expr = force_gimple_operand_1 (cond_expr, &gimplify_stmt_list, @@ -2427,6 +2430,17 @@ vect_loop_versioning (loop_vec_info loop initialize_original_copy_tables (); loop_version (loop, cond_expr, &condition_bb, prob, prob, REG_BR_PROB_BASE - prob, true); + + if (LOCATION_LOCUS (vect_location) != UNKNOWN_LOC + && dump_enabled_p ()) + { + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location, + "Loop is versioned to %s for vectorization\n", + (version_align && version_alias) + ? "enhance alignment and remove aliases" + : (version_align + ? "enhance alignment" : "remove aliases")); + } free_original_copy_tables(); /* Loop versioning violates an assumption we try to maintain during Index: dumpfile.c =================================================================== --- dumpfile.c (revision 201751) +++ dumpfile.c (working copy) @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. #include "dumpfile.h" #include "gimple-pretty-print.h" #include "tree.h" +#include "gimple.h" /* If non-NULL, return one past-the-end of the matching SUBPART of the WHOLE string. */ @@ -261,12 +262,20 @@ dump_loc (int dump_kind, FILE *dfile, so if (dump_kind) { if (LOCATION_LOCUS (loc) > BUILTINS_LOCATION) - fprintf (dfile, "\n%s:%d: note: ", LOCATION_FILE (loc), - LOCATION_LINE (loc)); + { + if (current_function_decl) + fprintf (dfile, "\n%s:%d:%s: note: ", LOCATION_FILE (loc), + LOCATION_LINE (loc), + gimple_decl_printable_name (current_function_decl, 1)); + else + fprintf (dfile, "\n%s:%d: note: ", LOCATION_FILE (loc), + LOCATION_LINE (loc)); + } else if (current_function_decl) - fprintf (dfile, "\n%s:%d: note: ", + fprintf (dfile, "\n%s:%d:%s: note: ", DECL_SOURCE_FILE (current_function_decl), - DECL_SOURCE_LINE (current_function_decl)); + DECL_SOURCE_LINE (current_function_decl), + gimple_decl_printable_name (current_function_decl, 1)); } } Index: ChangeLog =================================================================== --- ChangeLog (revision 201752) +++ ChangeLog (working copy) @@ -1,3 +1,13 @@ +2013-08-22 Xinliang David Li <davidxl@google.com> + + * tree-vect-loop-manip.c (vect_do_peeling_for_alignment): + Emit alignment peeling message with default -fopt-info. + (vect_loop_versioning): Emit loop version info message. + * tree-vectorizer.c (vectorize_loops): Minor message + change. + (execute_vect_slp): Ditto. + * dumpfile.c (dump_loc): Add function name in the dump. + 2013-08-14 Xinliang David Li <davidxl@google.com> * config/i386/i386.c (ix86_option_override_internal):