Patchwork opt-info message change for vectorizer

login
register
mail settings
Submitter Xinliang David Li
Date Aug. 22, 2013, 11:20 p.m.
Message ID <CAAkRFZK60Yer3SOds_=KbXzvqfz3EBfcxLFJEZdpKVSXiVFMvw@mail.gmail.com>
Download mbox | patch
Permalink /patch/269228/
State New
Headers show

Comments

Xinliang David Li - Aug. 22, 2013, 11:20 p.m.
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
Xinliang David Li - Aug. 27, 2013, 6:22 p.m.
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 - Aug. 27, 2013, 6:36 p.m.
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
Xinliang David Li - Aug. 27, 2013, 6:40 p.m.
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
Mike Stump - Aug. 27, 2013, 8:23 p.m.
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
Xinliang David Li - Aug. 27, 2013, 8:30 p.m.
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
>
Richard Guenther - Aug. 28, 2013, 9:45 a.m.
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
>>
Teresa Johnson - Aug. 28, 2013, 1:11 p.m.
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
>>>

Patch

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):