diff mbox

Add support for pragma Loop_Optimize ([No_]Vector)

Message ID 1411548.4VpsVizfpc@polaris
State New
Headers show

Commit Message

Eric Botcazou April 14, 2014, 2:45 p.m. UTC 
Hi,

this adds support for 2 optimization hints pertaining to loops in Ada, namely 
Loop_Optimize (No_Vector) and Loop_Optimize (Vector), by reusing the Ivdep 
approach in the middle-end (ANNOTATE_EXPR node) and directly setting the 
dont_vectorize and force_vectorize bits of the 'loop' structure.

Tested on x86_64-suse-linux, OK for the mainline?


2014-04-14  Eric Botcazou  <ebotcazou@adacore.com>

	* cfgloop.h (struct loop): Move force_vectorize down.
	* gimplify.c (gimple_boolify) <ANNOTATE_EXPR>: Handle new kinds.
	(gimplify_expr) <ANNOTATE_EXPR>: Minor tweak.
	* lto-streamer-in.c (input_cfg): Read dont_vectorize field.
	* lto-streamer-out.c (output_cfg): Write dont_vectorize field.
	* tree-cfg.c (replace_loop_annotate): Revamp and handle new kinds.
	* tree-core.h (enum annot_expr_kind): Add new kind values.
	* tree-inline.c (copy_loops): Copy dont_vectorize field and reorder.
	* tree-pretty-print.c (dump_generic_node) <ANNOTATE_EXPR>: Handle new
	kinds.
	* tree.def (ANNOTATE_EXPR): Tweak comment.
ada/
	* gcc-interface/trans.c (gnat_gimplify_stmt): Propagate loop hints.


2014-04-14  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/vect12.ad[sb]: New test.
	* gnat.dg/vect13.ad[sb]: Likewise.

Comments

Richard Biener April 15, 2014, 7:49 a.m. UTC | #1 
On Mon, Apr 14, 2014 at 4:45 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this adds support for 2 optimization hints pertaining to loops in Ada, namely
> Loop_Optimize (No_Vector) and Loop_Optimize (Vector), by reusing the Ivdep
> approach in the middle-end (ANNOTATE_EXPR node) and directly setting the
> dont_vectorize and force_vectorize bits of the 'loop' structure.
>
> Tested on x86_64-suse-linux, OK for the mainline?

The loop flags copying should go into copy_loop_info instead of only to
copy_loops.  Jakub - I see you remap simduid on copy - you have to do
sth in copy_loop_info instead I suppose.  See the other callers.

Otherwise I'm fine with this patch.

Thanks,
Richard.

>
> 2014-04-14  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * cfgloop.h (struct loop): Move force_vectorize down.
>         * gimplify.c (gimple_boolify) <ANNOTATE_EXPR>: Handle new kinds.
>         (gimplify_expr) <ANNOTATE_EXPR>: Minor tweak.
>         * lto-streamer-in.c (input_cfg): Read dont_vectorize field.
>         * lto-streamer-out.c (output_cfg): Write dont_vectorize field.
>         * tree-cfg.c (replace_loop_annotate): Revamp and handle new kinds.
>         * tree-core.h (enum annot_expr_kind): Add new kind values.
>         * tree-inline.c (copy_loops): Copy dont_vectorize field and reorder.
>         * tree-pretty-print.c (dump_generic_node) <ANNOTATE_EXPR>: Handle new
>         kinds.
>         * tree.def (ANNOTATE_EXPR): Tweak comment.
> ada/
>         * gcc-interface/trans.c (gnat_gimplify_stmt): Propagate loop hints.
>
>
> 2014-04-14  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/vect12.ad[sb]: New test.
>         * gnat.dg/vect13.ad[sb]: Likewise.
>
>
> --
> Eric Botcazou
Eric Botcazou April 15, 2014, 8:01 a.m. UTC | #2 
> The loop flags copying should go into copy_loop_info instead of only to
> copy_loops.  Jakub - I see you remap simduid on copy - you have to do
> sth in copy_loop_info instead I suppose.  See the other callers.

That also occurred to me, but IMO it's not crystal clear; for example, ivdep 
(aka safelen) is not in copy_loop_info either.  So I think this needs to be 
further discussed.

> Otherwise I'm fine with this patch.

Thanks, I have applied it as-is for now.
Richard Biener April 15, 2014, 8:11 a.m. UTC | #3 
On Tue, Apr 15, 2014 at 10:01 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> The loop flags copying should go into copy_loop_info instead of only to
>> copy_loops.  Jakub - I see you remap simduid on copy - you have to do
>> sth in copy_loop_info instead I suppose.  See the other callers.
>
> That also occurred to me, but IMO it's not crystal clear; for example, ivdep
> (aka safelen) is not in copy_loop_info either.  So I think this needs to be
> further discussed.

Well, there are passes that can end up duplicating loops and thus you lose
no_vectorize on the copy for example.  Clearly that's undesired, no?  For
safelen and simduid not copying them is erroring on the safe side at least,
likewise for force_vectorize.  But we do have the copy_loop_info abstraction
for a reason.  Otherwise we should simply discard it.

Richard.

>> Otherwise I'm fine with this patch.
>
> Thanks, I have applied it as-is for now.
>
> --
> Eric Botcazou
Eric Botcazou April 15, 2014, 8:36 a.m. UTC | #4 
> Well, there are passes that can end up duplicating loops and thus you lose
> no_vectorize on the copy for example.  Clearly that's undesired, no?  For
> safelen and simduid not copying them is erroring on the safe side at least,
> likewise for force_vectorize.  But we do have the copy_loop_info abstraction
> for a reason.  Otherwise we should simply discard it.

That's not very clear, even for dont_vectorize, and I'm a bit uncomfortable 
special-casing it.  For example, copy_loop_info is called for loop unswitching 
and loop versioning and one can wonder what should happen to loop hints for
unswitched and versioned instances of loops.  I'll further think about it.
diff mbox

Patch

Index: tree-pretty-print.c
===================================================================
--- tree-pretty-print.c	(revision 209334)
+++ tree-pretty-print.c	(working copy)
@@ -2105,13 +2105,21 @@  dump_generic_node (pretty_printer *buffe
 
     case ANNOTATE_EXPR:
       pp_string (buffer, "ANNOTATE_EXPR <");
+      dump_generic_node (buffer, TREE_OPERAND (node, 0), spc, flags, false);
       switch ((enum annot_expr_kind) TREE_INT_CST_LOW (TREE_OPERAND (node, 1)))
 	{
 	case annot_expr_ivdep_kind:
-	  pp_string (buffer, "ivdep, ");
+	  pp_string (buffer, ", ivdep");
+	  break;
+	case annot_expr_no_vector_kind:
+	  pp_string (buffer, ", no-vector");
 	  break;
+	case annot_expr_vector_kind:
+	  pp_string (buffer, ", vector");
+	  break;
+	default:
+	  gcc_unreachable ();
 	}
-      dump_generic_node (buffer, TREE_OPERAND (node, 0), spc, flags, false);
       pp_greater (buffer);
       break;
 
Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 209362)
+++ lto-streamer-out.c	(working copy)
@@ -1693,6 +1693,7 @@  output_cfg (struct output_block *ob, str
 
       /* Write OMP SIMD related info.  */
       streamer_write_hwi (ob, loop->safelen);
+      streamer_write_hwi (ob, loop->dont_vectorize);
       streamer_write_hwi (ob, loop->force_vectorize);
       stream_write_tree (ob, loop->simduid, true);
     }
Index: ada/gcc-interface/trans.c
===================================================================
--- ada/gcc-interface/trans.c	(revision 209375)
+++ ada/gcc-interface/trans.c	(working copy)
@@ -7761,6 +7761,15 @@  gnat_gimplify_stmt (tree *stmt_p)
 				 build_int_cst (integer_type_node,
 						annot_expr_ivdep_kind));
 
+	    if (LOOP_STMT_NO_VECTOR (stmt))
+	      gnu_cond = build2 (ANNOTATE_EXPR, TREE_TYPE (gnu_cond), gnu_cond,
+				 build_int_cst (integer_type_node,
+						annot_expr_no_vector_kind));
+	    if (LOOP_STMT_VECTOR (stmt))
+	      gnu_cond = build2 (ANNOTATE_EXPR, TREE_TYPE (gnu_cond), gnu_cond,
+				 build_int_cst (integer_type_node,
+						annot_expr_vector_kind));
+
 	    gnu_cond
 	      = build3 (COND_EXPR, void_type_node, gnu_cond, NULL_TREE,
 			build1 (GOTO_EXPR, void_type_node, gnu_end_label));
Index: lto-streamer-in.c
===================================================================
--- lto-streamer-in.c	(revision 209362)
+++ lto-streamer-in.c	(working copy)
@@ -718,6 +718,7 @@  input_cfg (struct lto_input_block *ib, s
 
       /* Read OMP SIMD related info.  */
       loop->safelen = streamer_read_hwi (ib);
+      loop->dont_vectorize = streamer_read_hwi (ib);
       loop->force_vectorize = streamer_read_hwi (ib);
       loop->simduid = stream_read_tree (ib, data_in);
 
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 209334)
+++ gimplify.c	(working copy)
@@ -2813,15 +2813,18 @@  gimple_boolify (tree expr)
       return expr;
 
     case ANNOTATE_EXPR:
-      if ((enum annot_expr_kind) TREE_INT_CST_LOW (TREE_OPERAND (expr, 1))
-	  == annot_expr_ivdep_kind)
+      switch ((enum annot_expr_kind) TREE_INT_CST_LOW (TREE_OPERAND (expr, 1)))
 	{
+	case annot_expr_ivdep_kind:
+	case annot_expr_no_vector_kind:
+	case annot_expr_vector_kind:
 	  TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
 	  if (TREE_CODE (type) != BOOLEAN_TYPE)
 	    TREE_TYPE (expr) = boolean_type_node;
 	  return expr;
+	default:
+	  gcc_unreachable ();
 	}
-      /* FALLTHRU */
 
     default:
       if (COMPARISON_CLASS_P (expr))
@@ -7528,7 +7531,7 @@  gimplify_expr (tree *expr_p, gimple_seq
 	case ANNOTATE_EXPR:
 	  {
 	    tree cond = TREE_OPERAND (*expr_p, 0);
-	    tree id = TREE_OPERAND (*expr_p, 1);
+	    tree kind = TREE_OPERAND (*expr_p, 1);
 	    tree type = TREE_TYPE (cond);
 	    if (!INTEGRAL_TYPE_P (type))
 	      {
@@ -7538,8 +7541,8 @@  gimplify_expr (tree *expr_p, gimple_seq
 	      }
 	    tree tmp = create_tmp_var (type, NULL);
 	    gimplify_arg (&cond, pre_p, EXPR_LOCATION (*expr_p));
-	    gimple call = gimple_build_call_internal (IFN_ANNOTATE, 2,
-						      cond, id);
+	    gimple call
+	      = gimple_build_call_internal (IFN_ANNOTATE, 2, cond, kind);
 	    gimple_call_set_lhs (call, tmp);
 	    gimplify_seq_add_stmt (pre_p, call);
 	    *expr_p = tmp;
Index: tree.def
===================================================================
--- tree.def	(revision 209334)
+++ tree.def	(working copy)
@@ -1280,7 +1280,7 @@  DEFTREECODE (TARGET_OPTION_NODE, "target
 
 /* ANNOTATE_EXPR.
    Operand 0 is the expression to be annotated.
-   Operand 1 is the annotation id. */
+   Operand 1 is the annotation kind.  */
 DEFTREECODE (ANNOTATE_EXPR, "annotate_expr", tcc_expression, 2)
 
 /* Cilk spawn statement
Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 209362)
+++ tree-inline.c	(working copy)
@@ -2349,17 +2349,18 @@  copy_loops (copy_body_data *id,
 	  place_new_loop (cfun, dest_loop);
 	  flow_loop_tree_node_add (dest_parent, dest_loop);
 
-	  if (src_loop->simduid)
-	    {
-	      dest_loop->simduid = remap_decl (src_loop->simduid, id);
-	      cfun->has_simduid_loops = true;
-	    }
+	  dest_loop->safelen = src_loop->safelen;
+	  dest_loop->dont_vectorize = src_loop->dont_vectorize;
 	  if (src_loop->force_vectorize)
 	    {
 	      dest_loop->force_vectorize = true;
 	      cfun->has_force_vectorize_loops = true;
 	    }
-	  dest_loop->safelen = src_loop->safelen;
+	  if (src_loop->simduid)
+	    {
+	      dest_loop->simduid = remap_decl (src_loop->simduid, id);
+	      cfun->has_simduid_loops = true;
+	    }
 
 	  /* Recurse.  */
 	  copy_loops (id, dest_loop, src_loop);
Index: cfgloop.h
===================================================================
--- cfgloop.h	(revision 209362)
+++ cfgloop.h	(working copy)
@@ -173,12 +173,12 @@  struct GTY ((chain_next ("%h.next"))) lo
      of the loop can be safely evaluated concurrently.  */
   int safelen;
 
-  /* True if we should try harder to vectorize this loop.  */
-  bool force_vectorize;
-
   /* True if this loop should never be vectorized.  */
   bool dont_vectorize;
 
+  /* True if we should try harder to vectorize this loop.  */
+  bool force_vectorize;
+
   /* For SIMD loops, this is a unique identifier of the loop, referenced
      by IFN_GOMP_SIMD_VF, IFN_GOMP_SIMD_LANE and IFN_GOMP_SIMD_LAST_LANE
      builtins.  */
Index: tree-core.h
===================================================================
--- tree-core.h	(revision 209334)
+++ tree-core.h	(working copy)
@@ -657,7 +657,10 @@  enum tree_node_kind {
 };
 
 enum annot_expr_kind {
-  annot_expr_ivdep_kind
+  annot_expr_ivdep_kind,
+  annot_expr_no_vector_kind,
+  annot_expr_vector_kind,
+  annot_expr_kind_last
 };
 
 
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 209362)
+++ tree-cfg.c	(working copy)
@@ -250,9 +250,9 @@  build_gimple_cfg (gimple_seq seq)
 }
 
 
-/* Search for ANNOTATE call with annot_expr_ivdep_kind; if found, remove
-   it and set loop->safelen to INT_MAX.  We assume that the annotation
-   comes immediately before the condition.  */
+/* Look for ANNOTATE calls with loop annotation kind; if found, remove
+   them and propagate the information to the loop.  We assume that the
+   annotations come immediately before the condition of the loop.  */
 
 static void
 replace_loop_annotate ()
@@ -266,50 +266,62 @@  replace_loop_annotate ()
     {
       gsi = gsi_last_bb (loop->header);
       stmt = gsi_stmt (gsi);
-      if (stmt && gimple_code (stmt) == GIMPLE_COND)
+      if (!(stmt && gimple_code (stmt) == GIMPLE_COND))
+	continue;
+      for (gsi_prev_nondebug (&gsi); !gsi_end_p (gsi); gsi_prev (&gsi))
 	{
-	  gsi_prev_nondebug (&gsi);
-	  if (gsi_end_p (gsi))
-	    continue;
 	  stmt = gsi_stmt (gsi);
 	  if (gimple_code (stmt) != GIMPLE_CALL)
-		continue;
+	    break;
 	  if (!gimple_call_internal_p (stmt)
-		  || gimple_call_internal_fn (stmt) != IFN_ANNOTATE)
-	    continue;
-	  if ((annot_expr_kind) tree_to_shwi (gimple_call_arg (stmt, 1))
-	      != annot_expr_ivdep_kind)
-	    continue;
+	      || gimple_call_internal_fn (stmt) != IFN_ANNOTATE)
+	    break;
+	  switch ((annot_expr_kind) tree_to_shwi (gimple_call_arg (stmt, 1)))
+	    {
+	    case annot_expr_ivdep_kind:
+	      loop->safelen = INT_MAX;
+	      break;
+	    case annot_expr_no_vector_kind:
+	      loop->dont_vectorize = true;
+	      break;
+	    case annot_expr_vector_kind:
+	      loop->force_vectorize = true;
+	      cfun->has_force_vectorize_loops = true;
+	      break;
+	    default:
+	      gcc_unreachable ();
+	    }
 	  stmt = gimple_build_assign (gimple_call_lhs (stmt),
 				      gimple_call_arg (stmt, 0));
 	  gsi_replace (&gsi, stmt, true);
-	  loop->safelen = INT_MAX;
 	}
     }
 
-  /* Remove IFN_ANNOTATE. Safeguard for the case loop->latch == NULL.  */
+  /* Remove IFN_ANNOTATE.  Safeguard for the case loop->latch == NULL.  */
   FOR_EACH_BB_FN (bb, cfun)
     {
-      gsi = gsi_last_bb (bb);
-      stmt = gsi_stmt (gsi);
-      if (stmt && gimple_code (stmt) == GIMPLE_COND)
-	gsi_prev_nondebug (&gsi);
-      if (gsi_end_p (gsi))
-	continue;
-      stmt = gsi_stmt (gsi);
-      if (gimple_code (stmt) != GIMPLE_CALL)
-	continue;
-      if (!gimple_call_internal_p (stmt)
-	  || gimple_call_internal_fn (stmt) != IFN_ANNOTATE)
-	continue;
-      if ((annot_expr_kind) tree_to_shwi (gimple_call_arg (stmt, 1))
-	  != annot_expr_ivdep_kind)
-	continue;
-      warning_at (gimple_location (stmt), 0, "ignoring %<GCC ivdep%> "
-		  "annotation");
-      stmt = gimple_build_assign (gimple_call_lhs (stmt),
-				  gimple_call_arg (stmt, 0));
-      gsi_replace (&gsi, stmt, true);
+      for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi))
+	{
+	  stmt = gsi_stmt (gsi);
+	  if (gimple_code (stmt) != GIMPLE_CALL)
+	    break;
+	  if (!gimple_call_internal_p (stmt)
+	      || gimple_call_internal_fn (stmt) != IFN_ANNOTATE)
+	    break;
+	  switch ((annot_expr_kind) tree_to_shwi (gimple_call_arg (stmt, 1)))
+	    {
+	    case annot_expr_ivdep_kind:
+	    case annot_expr_no_vector_kind:
+	    case annot_expr_vector_kind:
+	      break;
+	    default:
+	      gcc_unreachable ();
+	    }
+	  warning_at (gimple_location (stmt), 0, "ignoring loop annotation");
+	  stmt = gimple_build_assign (gimple_call_lhs (stmt),
+				      gimple_call_arg (stmt, 0));
+	  gsi_replace (&gsi, stmt, true);
+	}
     }
 }