This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] [RFC] Higher-level reporting of vectorization problems


On Mon, 2 Jul 2018, Richard Sandiford wrote:

> Richard Biener <rguenther@suse.de> writes:
> > On Fri, 22 Jun 2018, David Malcolm wrote:
> >
> >> NightStrike and I were chatting on IRC last week about
> >> issues with trying to vectorize the following code:
> >> 
> >> #include <vector>
> >> std::size_t f(std::vector<std::vector<float>> const & v) {
> >> 	std::size_t ret = 0;
> >> 	for (auto const & w: v)
> >> 		ret += w.size();
> >> 	return ret;
> >> }
> >> 
> >> icc could vectorize it, but gcc couldn't, but neither of us could
> >> immediately figure out what the problem was.
> >> 
> >> Using -fopt-info leads to a wall of text.
> >> 
> >> I tried using my patch here:
> >> 
> >>  "[PATCH] v3 of optinfo, remarks and optimization records"
> >>   https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01267.html
> >> 
> >> It improved things somewhat, by showing:
> >> (a) the nesting structure via indentation, and
> >> (b) the GCC line at which each message is emitted (by using the
> >>     "remark" output)
> >> 
> >> but it's still a wall of text:
> >> 
> >>   https://dmalcolm.fedorapeople.org/gcc/2018-06-18/test.cc.remarks.html
> >>   https://dmalcolm.fedorapeople.org/gcc/2018-06-18/test.cc.d/..%7C..%7Csrc%7Ctest.cc.html#line-4
> >> 
> >> It doesn't yet provide a simple high-level message to a
> >> tech-savvy user on what they need to do to get GCC to
> >> vectorize their loop.
> >
> > Yeah, in particular the vectorizer is way too noisy in its low-level
> > functions.  IIRC -fopt-info-vec-missed is "somewhat" better:
> >
> > t.C:4:26: note: step unknown.
> > t.C:4:26: note: vector alignment may not be reachable
> > t.C:4:26: note: not ssa-name.
> > t.C:4:26: note: use not simple.
> > t.C:4:26: note: not ssa-name.
> > t.C:4:26: note: use not simple.
> > t.C:4:26: note: no array mode for V2DI[3]
> > t.C:4:26: note: Data access with gaps requires scalar epilogue loop
> > t.C:4:26: note: can't use a fully-masked loop because the target doesn't 
> > have the appropriate masked load or store.
> > t.C:4:26: note: not ssa-name.
> > t.C:4:26: note: use not simple.
> > t.C:4:26: note: not ssa-name.
> > t.C:4:26: note: use not simple.
> > t.C:4:26: note: no array mode for V2DI[3]
> > t.C:4:26: note: Data access with gaps requires scalar epilogue loop
> > t.C:4:26: note: op not supported by target.
> > t.C:4:26: note: not vectorized: relevant stmt not supported: _15 = _14 
> > /[ex] 4;
> > t.C:4:26: note: bad operation or unsupported loop bound.
> > t.C:4:26: note: not vectorized: no grouped stores in basic block.
> > t.C:4:26: note: not vectorized: no grouped stores in basic block.
> > t.C:6:12: note: not vectorized: not enough data-refs in basic block.
> >
> >
> >> The pertinent dump messages are:
> >> 
> >> test.cc:4:23: remark: === try_vectorize_loop_1 === [../../src/gcc/tree-vectorizer.c:674:try_vectorize_loop_1]
> >> cc1plus: remark:
> >> Analyzing loop at test.cc:4 [../../src/gcc/dumpfile.c:735:ensure_pending_optinfo]
> >> test.cc:4:23: remark:  === analyze_loop_nest === [../../src/gcc/tree-vect-loop.c:2299:vect_analyze_loop]
> >> [...snip...]
> >> test.cc:4:23: remark:   === vect_analyze_loop_operations === [../../src/gcc/tree-vect-loop.c:1520:vect_analyze_loop_operations]
> >> [...snip...]
> >> test.cc:4:23: remark:    ==> examining statement: ‘_15 = _14 /[ex] 4;’ [../../src/gcc/tree-vect-stmts.c:9382:vect_analyze_stmt]
> >> test.cc:4:23: remark:    vect_is_simple_use: operand ‘_14’ [../../src/gcc/tree-vect-stmts.c:10064:vect_is_simple_use]
> >> test.cc:4:23: remark:    def_stmt: ‘_14 = _8 - _7;’ [../../src/gcc/tree-vect-stmts.c:10098:vect_is_simple_use]
> >> test.cc:4:23: remark:    type of def: internal [../../src/gcc/tree-vect-stmts.c:10112:vect_is_simple_use]
> >> test.cc:4:23: remark:    vect_is_simple_use: operand ‘4’ [../../src/gcc/tree-vect-stmts.c:10064:vect_is_simple_use]
> >> test.cc:4:23: remark:    op not supported by target. [../../src/gcc/tree-vect-stmts.c:5932:vectorizable_operation]
> >> test.cc:4:23: remark:    not vectorized: relevant stmt not supported: ‘_15 = _14 /[ex] 4;’ [../../src/gcc/tree-vect-stmts.c:9565:vect_analyze_stmt]
> >> test.cc:4:23: remark:   bad operation or unsupported loop bound. [../../src/gcc/tree-vect-loop.c:2043:vect_analyze_loop_2]
> >> cc1plus: remark: vectorized 0 loops in function. [../../src/gcc/tree-vectorizer.c:904:vectorize_loops]
> >> 
> >> In particular, that complaint from
> >>   [../../src/gcc/tree-vect-stmts.c:9565:vect_analyze_stmt]
> >> is coming from:
> >> 
> >>   if (!ok)
> >>     {
> >>       if (dump_enabled_p ())
> >>         {
> >>           dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> >>                            "not vectorized: relevant stmt not ");
> >>           dump_printf (MSG_MISSED_OPTIMIZATION, "supported: ");
> >>           dump_gimple_stmt (MSG_MISSED_OPTIMIZATION, TDF_SLIM, stmt, 0);
> >>         }
> >> 
> >>       return false;
> >>     }
> >> 
> >> This got me thinking: the user presumably wants to know several
> >> things:
> >> 
> >> * the location of the loop that can't be vectorized (vect_location
> >>   captures this)
> >> * location of the problematic statement
> >> * why it's problematic
> >> * the problematic statement itself.
> >> 
> >> The following is an experiment at capturing that information, by
> >> recording an "opt_problem" instance describing what the optimization
> >> problem is, created deep in the callstack when it occurs, for use
> >> later on back at the top of the vectorization callstack.
> >
> > Nice idea.  Of course the issue is then for which issues to
> > exactly create those.  Like all of the MSG_MISSED_OPTIMIZATION
> > dumpings?
> >
> > I guess the vectorizer needs some axing of useless messages
> > and/or we need a MSG_DEBUG to have an additional level below
> > MSG_NOTE.
> 
> Sounds good.
> 
> One of the worst sources of excess noise seems to be vect_is_simple_use:
> just looking at an operand usually produces three lines of text.
> I did wonder about suggesting we axe that, but I suppose there are
> times when it might be useful.

It's easy to prune "obvious" parts, let me do that.

The following doesn't make it very much less noisy, it even adds
(useful) information and makes printing more consistent.

We now get for example

/space/rguenther/src/gcc-slpcost/gcc/testsuite/gcc.dg/vect/pr24300.c:23:3: 
note:  vect_is_simple_use: operand n_77 = PHI <n_10(9), 0(11)>, type of 
def: reduction
/space/rguenther/src/gcc-slpcost/gcc/testsuite/gcc.dg/vect/pr24300.c:23:3: 
note:  vect_is_simple_use: vectype vector(4) int

or when called with the !vectype variant just

/space/rguenther/src/gcc-slpcost/gcc/testsuite/gcc.dg/vect/pr24300.c:23:3: 
note:   vect_is_simple_use: operand (long unsigned int) s_76, type of def: 
internal

I combined printing the operand together with its definition and
always print the resulting *def.

I believe the "unsupported defining stmt" should be "dead" in that
it's STMT_VINFO_DEF_TYPE should be vect_unknown_def_type already
but I'm not 100% sure so I kept it for now.

Bootstrap / regtest in progress.

Richard.

2018-07-03  Richard Biener  <rguenther@suse.de>

	* tree-vect-stmts.c (vect_is_simple_use): Consolidate dumping,
	always set *dt.  Dump vectype in vectype overload.
	* dumpfile.h (dump_gimple_expr): New function.
	(dump_gimple_expr_loc): Likewise.
	* dumpfile.c (dump_gimple_expr): New function.
	(dump_gimple_expr_loc): Likewise.

diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 5f69f9bd646..3296299e86b 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -492,6 +492,42 @@ dump_gimple_stmt_loc (dump_flags_t dump_kind, const dump_location_t &loc,
     }
 }
 
+/* Dump gimple statement GS with SPC indentation spaces and
+   EXTRA_DUMP_FLAGS on the dump streams if DUMP_KIND is enabled.
+   Do not terminate with a newline or semicolon.  */
+
+void
+dump_gimple_expr (dump_flags_t dump_kind, dump_flags_t extra_dump_flags,
+		  gimple *gs, int spc)
+{
+  if (dump_file && (dump_kind & pflags))
+    print_gimple_expr (dump_file, gs, spc, dump_flags | extra_dump_flags);
+
+  if (alt_dump_file && (dump_kind & alt_flags))
+    print_gimple_expr (alt_dump_file, gs, spc, dump_flags | extra_dump_flags);
+}
+
+/* Similar to dump_gimple_expr, except additionally print source location.  */
+
+void
+dump_gimple_expr_loc (dump_flags_t dump_kind, const dump_location_t &loc,
+		      dump_flags_t extra_dump_flags, gimple *gs, int spc)
+{
+  location_t srcloc = loc.get_location_t ();
+  if (dump_file && (dump_kind & pflags))
+    {
+      dump_loc (dump_kind, dump_file, srcloc);
+      print_gimple_expr (dump_file, gs, spc, dump_flags | extra_dump_flags);
+    }
+
+  if (alt_dump_file && (dump_kind & alt_flags))
+    {
+      dump_loc (dump_kind, alt_dump_file, srcloc);
+      print_gimple_expr (alt_dump_file, gs, spc, dump_flags | extra_dump_flags);
+    }
+}
+
+
 /* Dump expression tree T using EXTRA_DUMP_FLAGS on dump streams if
    DUMP_KIND is enabled.  */
 
diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
index 0e588a6dac6..a4172419c1d 100644
--- a/gcc/dumpfile.h
+++ b/gcc/dumpfile.h
@@ -431,6 +431,9 @@ extern void dump_generic_expr (dump_flags_t, dump_flags_t, tree);
 extern void dump_gimple_stmt_loc (dump_flags_t, const dump_location_t &,
 				  dump_flags_t, gimple *, int);
 extern void dump_gimple_stmt (dump_flags_t, dump_flags_t, gimple *, int);
+extern void dump_gimple_expr_loc (dump_flags_t, const dump_location_t &,
+				  dump_flags_t, gimple *, int);
+extern void dump_gimple_expr (dump_flags_t, dump_flags_t, gimple *, int);
 extern void print_combine_total_stats (void);
 extern bool enable_rtl_dump_file (void);
 
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index ab8cc8049a4..ae62fc36401 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -10036,61 +10036,53 @@ vect_is_simple_use (tree operand, vec_info *vinfo, enum vect_def_type *dt,
     {
       dump_printf_loc (MSG_NOTE, vect_location,
                        "vect_is_simple_use: operand ");
-      dump_generic_expr (MSG_NOTE, TDF_SLIM, operand);
-      dump_printf (MSG_NOTE, "\n");
+      if (TREE_CODE (operand) == SSA_NAME
+	  && !SSA_NAME_IS_DEFAULT_DEF (operand))
+	dump_gimple_expr (MSG_NOTE, TDF_SLIM, SSA_NAME_DEF_STMT (operand), 0);
+      else
+	dump_generic_expr (MSG_NOTE, TDF_SLIM, operand);
     }
 
   if (CONSTANT_CLASS_P (operand))
-    {
-      *dt = vect_constant_def;
-      return true;
-    }
-
-  if (is_gimple_min_invariant (operand))
-    {
-      *dt = vect_external_def;
-      return true;
-    }
-
-  if (TREE_CODE (operand) != SSA_NAME)
-    {
-      if (dump_enabled_p ())
-	dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
-			 "not ssa-name.\n");
-      return false;
-    }
-
-  if (SSA_NAME_IS_DEFAULT_DEF (operand))
-    {
-      *dt = vect_external_def;
-      return true;
-    }
-
-  gimple *def_stmt = SSA_NAME_DEF_STMT (operand);
-  if (dump_enabled_p ())
-    {
-      dump_printf_loc (MSG_NOTE, vect_location, "def_stmt: ");
-      dump_gimple_stmt (MSG_NOTE, TDF_SLIM, def_stmt, 0);
-    }
-
-  if (! vect_stmt_in_region_p (vinfo, def_stmt))
+    *dt = vect_constant_def;
+  else if (is_gimple_min_invariant (operand))
+    *dt = vect_external_def;
+  else if (TREE_CODE (operand) != SSA_NAME)
+    *dt = vect_unknown_def_type;
+  else if (SSA_NAME_IS_DEFAULT_DEF (operand))
     *dt = vect_external_def;
   else
     {
-      stmt_vec_info stmt_vinfo = vinfo_for_stmt (def_stmt);
-      if (STMT_VINFO_IN_PATTERN_P (stmt_vinfo))
+      gimple *def_stmt = SSA_NAME_DEF_STMT (operand);
+      if (! vect_stmt_in_region_p (vinfo, def_stmt))
+	*dt = vect_external_def;
+      else
 	{
-	  def_stmt = STMT_VINFO_RELATED_STMT (stmt_vinfo);
-	  stmt_vinfo = vinfo_for_stmt (def_stmt);
+	  stmt_vec_info stmt_vinfo = vinfo_for_stmt (def_stmt);
+	  if (STMT_VINFO_IN_PATTERN_P (stmt_vinfo))
+	    {
+	      def_stmt = STMT_VINFO_RELATED_STMT (stmt_vinfo);
+	      stmt_vinfo = vinfo_for_stmt (def_stmt);
+	    }
+	  switch (gimple_code (def_stmt))
+	    {
+	    case GIMPLE_PHI:
+	    case GIMPLE_ASSIGN:
+	    case GIMPLE_CALL:
+	      *dt = STMT_VINFO_DEF_TYPE (stmt_vinfo);
+	      break;
+	    default:
+	      *dt = vect_unknown_def_type;
+	      break;
+	    }
 	}
-      *dt = STMT_VINFO_DEF_TYPE (stmt_vinfo);
+      if (def_stmt_out)
+	*def_stmt_out = def_stmt;
     }
-  if (def_stmt_out)
-    *def_stmt_out = def_stmt;
 
   if (dump_enabled_p ())
     {
-      dump_printf_loc (MSG_NOTE, vect_location, "type of def: ");
+      dump_printf (MSG_NOTE, ", type of def: ");
       switch (*dt)
 	{
 	case vect_uninitialized_def:
@@ -10131,19 +10123,6 @@ vect_is_simple_use (tree operand, vec_info *vinfo, enum vect_def_type *dt,
       return false;
     }
 
-  switch (gimple_code (def_stmt))
-    {
-    case GIMPLE_PHI:
-    case GIMPLE_ASSIGN:
-    case GIMPLE_CALL:
-      break;
-    default:
-      if (dump_enabled_p ())
-        dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
-                         "unsupported defining stmt:\n");
-      return false;
-    }
-
   return true;
 }
 
@@ -10179,6 +10158,13 @@ vect_is_simple_use (tree operand, vec_info *vinfo, enum vect_def_type *dt,
       stmt_vec_info stmt_info = vinfo_for_stmt (def_stmt);
       *vectype = STMT_VINFO_VECTYPE (stmt_info);
       gcc_assert (*vectype != NULL_TREE);
+      if (dump_enabled_p ())
+	{
+	  dump_printf_loc (MSG_NOTE, vect_location,
+			   "vect_is_simple_use: vectype ");
+	  dump_generic_expr (MSG_NOTE, TDF_SLIM, *vectype);
+	  dump_printf (MSG_NOTE, "\n");
+	}
     }
   else if (*dt == vect_uninitialized_def
 	   || *dt == vect_constant_def

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]