diff mbox

Patch: Add #pragma ivdep support to the ME and C FE

Message ID 52566924.9080909@net-b.de
State New
Headers show

Commit Message

Tobias Burnus Oct. 10, 2013, 8:45 a.m. UTC
Richard: Could you review the Gimple part? Thanks!
Joseph: Could you have a look at the C part? Thanks!

Jakub Jelinek wrote:
>> +      if (loop->latch)
>> +	FOR_EACH_EDGE (e, ei, loop->latch->succs)
>> +	  {
>> +	    if (e->dest->flags & BB_RTL)
>> +	      break;
> I'd prefer Richard to review this (and probably Joseph the C FE part).
> You can't really have loop->latch in GIMPLE and the successors
> in RTL, so perhaps you can check that in the if (loop->latch) check
> already.
> GIMPLE_COND must be the last in the bb, can't be followed by debug 
> stmts, so you can safely use just gsi_last_bb (e->dest) instead.

I have done the two modifications you suggested, fixed the formatting 
glitches. And completed it by another all-language bootstrap and 
regtesting. (Including also the separately posted Fortran [1] and the 
C++ [2] patches.)
OK for the trunk?

[1] http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00517.html
[2] http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00656.html

>> +#define SET_ANNOTATE_EXPR_ID(NODE, ID) \
>> +  (ANNOTATE_EXPR_CHECK(NODE)->base.u.version = ID)
> Likewise.  Shouldn't it be = (ID) ?

Probably less relevant for assignments, but for safety, it cannot harm.

Tobias

Comments

Joseph Myers Oct. 11, 2013, 3:58 p.m. UTC | #1
On Thu, 10 Oct 2013, Tobias Burnus wrote:

> Joseph: Could you have a look at the C part? Thanks!

Your error "missing loop condition loop with IVDEP pragma" should, I 
think, say "loop condition in loop".  Since the pragma is "ivdep" 
(case-sensitive), it should also say %<ivdep%> not IVDEP.  This 
restriction should be clarified in the documentation, which could do with 
examples.  There should be at least one execution test for correct 
execution of loops with this pragma, and at least one test for this 
diagnostic.  The documentation refers to "safelen" which sounds like 
implementation terminology; this is the user manual and needs to describe 
things in user terms, not in terms of the implementation.
Frederic Riss Oct. 16, 2013, 1:20 p.m. UTC | #2
Hi,

Just one question. You describe the pragma in the doco patch as:

+This pragma tells the compiler that the immediately following @code{for}
+loop can be executed in any loop index order without affecting the result.
+The pragma aids optimization and in particular vectorization as the
+compiler can then assume a vectorization safelen of infinity.

I'm not a specialist, but I was always told that the 'original'
meaning of ivdep (which I believe was introduced by Cray), was that
the compiler could assume that there are only forward dependencies in
the loop, but not that it can be executed in any order. The Intel docs
give this example:

void ignore_vec_dep(int *a, int k, int c, int m)
{
  #pragma ivdep
  for (int i = 0; i < m; i++)
    a[i] = a[i + k] * c;
}

Given your description, this loop wouldn't be a candidate for ivdep,
as reversing the loop index order changes the semantics. I believe
that the way you interpret it (ie. setting vectorization safe length
to INT_MAX) is correct with respect to this other definition, though.

Is there any normative definition for such language extensions ?

Oh, and are there any plans to maintain this information in some way
till the back-end? Software pipelining could be another huge winner
for that kind of dependency analysis simplification.

Thanks,
Fred


On 10 October 2013 10:45, Tobias Burnus <burnus@net-b.de> wrote:
> Richard: Could you review the Gimple part? Thanks!
> Joseph: Could you have a look at the C part? Thanks!
>
> Jakub Jelinek wrote:
>>>
>>> +      if (loop->latch)
>>> +       FOR_EACH_EDGE (e, ei, loop->latch->succs)
>>> +         {
>>> +           if (e->dest->flags & BB_RTL)
>>> +             break;
>>
>> I'd prefer Richard to review this (and probably Joseph the C FE part).
>> You can't really have loop->latch in GIMPLE and the successors
>> in RTL, so perhaps you can check that in the if (loop->latch) check
>> already.
>> GIMPLE_COND must be the last in the bb, can't be followed by debug stmts,
>> so you can safely use just gsi_last_bb (e->dest) instead.
>
>
> I have done the two modifications you suggested, fixed the formatting
> glitches. And completed it by another all-language bootstrap and regtesting.
> (Including also the separately posted Fortran [1] and the C++ [2] patches.)
> OK for the trunk?
>
> [1] http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00517.html
> [2] http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00656.html
>
>>> +#define SET_ANNOTATE_EXPR_ID(NODE, ID) \
>>> +  (ANNOTATE_EXPR_CHECK(NODE)->base.u.version = ID)
>>
>> Likewise.  Shouldn't it be = (ID) ?
>
>
> Probably less relevant for assignments, but for safety, it cannot harm.
>
> Tobias
diff mbox

Patch

2013-08-10  Tobias Burnus  <burnus@net-b.de>

	PR other/33426
        * c-pragma.c (init_pragma) Add #pragma ivdep handling.
        * c-pragma.h (pragma_kind): Add PRAGMA_IVDEP.

	PR other/33426
        * c-parser.c (c_parser_pragma, c_parser_for_statement):
        Handle PRAGMA_IVDEP.
        (c_parser_statement_after_labels): Update call.

	PR other/33426
        * cfgloop.c (flow_loops_find): Search for IFN_ANNOTATE
        and set safelen.
        * gimplify.c (gimple_boolify, gimplify_expr): Handle ANNOTATE_EXPR.
        * internal-fn.c (expand_ANNOTATE): New function.
        * internal-fn.def (ANNOTATE): Define as new internal function.
        * tree-core.h (tree_node_kind): Add annot_expr_ivdep_kind.
        (tree_base) Update a comment.
        * tree-pretty-print.c (dump_generic_node): Handle ANNOTATE_EXPR.
        * tree.def (ANNOTATE_EXPR): New DEFTREECODE.
        * tree.h (ANNOTATE_EXPR_ID, SET_ANNOTATE_EXPR_ID): New macros.
	* doc/extend.texi (Pragmas): Document #pragma ivdep.

	PR other/33426
	* testsuite/gcc.dg/vect/vect-ivdep-1.c: New.

diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index 309859f..06dbf17 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -1353,6 +1353,8 @@  init_pragma (void)
     cpp_register_deferred_pragma (parse_in, "GCC", "pch_preprocess",
 				  PRAGMA_GCC_PCH_PREPROCESS, false, false);
 
+  cpp_register_deferred_pragma (parse_in, 0, "ivdep", PRAGMA_IVDEP, false,
+				false);
 #ifdef HANDLE_PRAGMA_PACK_WITH_EXPANSION
   c_register_pragma_with_expansion (0, "pack", handle_pragma_pack);
 #else
diff --git a/gcc/c-family/c-pragma.h b/gcc/c-family/c-pragma.h
index 41215db..c826fbd 100644
--- a/gcc/c-family/c-pragma.h
+++ b/gcc/c-family/c-pragma.h
@@ -46,6 +46,7 @@  typedef enum pragma_kind {
   PRAGMA_OMP_THREADPRIVATE,
 
   PRAGMA_GCC_PCH_PREPROCESS,
+  PRAGMA_IVDEP,
 
   PRAGMA_FIRST_EXTERNAL
 } pragma_kind;
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index b612e29..6bf9fbf 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -1150,7 +1150,7 @@  static void c_parser_if_statement (c_parser *);
 static void c_parser_switch_statement (c_parser *);
 static void c_parser_while_statement (c_parser *);
 static void c_parser_do_statement (c_parser *);
-static void c_parser_for_statement (c_parser *);
+static void c_parser_for_statement (c_parser *, bool);
 static tree c_parser_asm_statement (c_parser *);
 static tree c_parser_asm_operands (c_parser *);
 static tree c_parser_asm_goto_operands (c_parser *);
@@ -4495,7 +4495,7 @@  c_parser_statement_after_labels (c_parser *parser)
 	  c_parser_do_statement (parser);
 	  break;
 	case RID_FOR:
-	  c_parser_for_statement (parser);
+	  c_parser_for_statement (parser, false);
 	  break;
 	case RID_GOTO:
 	  c_parser_consume_token (parser);
@@ -4948,7 +4948,7 @@  c_parser_do_statement (c_parser *parser)
 */
 
 static void
-c_parser_for_statement (c_parser *parser)
+c_parser_for_statement (c_parser *parser, bool ivdep)
 {
   tree block, cond, incr, save_break, save_cont, body;
   /* The following are only used when parsing an ObjC foreach statement.  */
@@ -5054,8 +5054,17 @@  c_parser_for_statement (c_parser *parser)
 	{
 	  if (c_parser_next_token_is (parser, CPP_SEMICOLON))
 	    {
-	      c_parser_consume_token (parser);
-	      cond = NULL_TREE;
+	      if (ivdep)
+		{
+		  c_parser_error (parser, "missing loop condition loop with "
+				  "IVDEP pragma");
+		  cond = error_mark_node;
+		}
+	      else
+		{
+		  c_parser_consume_token (parser);
+		  cond = NULL_TREE;
+		}
 	    }
 	  else
 	    {
@@ -5069,6 +5078,12 @@  c_parser_for_statement (c_parser *parser)
 	      c_parser_skip_until_found (parser, CPP_SEMICOLON,
 					 "expected %<;%>");
 	    }
+	  if (ivdep)
+	    {
+	      cond = build1 (ANNOTATE_EXPR, TREE_TYPE (cond), cond);
+	      SET_ANNOTATE_EXPR_ID (cond, annot_expr_ivdep_kind);
+	    }
+
 	}
       /* Parse the increment expression (the third expression in a
 	 for-statement).  In the case of a foreach-statement, this is
@@ -8947,6 +8962,17 @@  c_parser_pragma (c_parser *parser, enum pragma_context context)
       c_parser_skip_until_found (parser, CPP_PRAGMA_EOL, NULL);
       return false;
 
+    case PRAGMA_IVDEP:
+      c_parser_consume_pragma (parser);
+      c_parser_skip_to_pragma_eol (parser);
+      if (!c_parser_next_token_is_keyword (parser, RID_FOR))
+	{
+	  c_parser_error (parser, "for statement expected");
+	  return false;
+	}
+      c_parser_for_statement (parser, true);
+      return false;
+
     case PRAGMA_GCC_PCH_PREPROCESS:
       c_parser_error (parser, "%<#pragma GCC pch_preprocess%> must be first");
       c_parser_skip_until_found (parser, CPP_PRAGMA_EOL, NULL);
diff --git a/gcc/cfgloop.c b/gcc/cfgloop.c
index 272a675..c3cea44 100644
--- a/gcc/cfgloop.c
+++ b/gcc/cfgloop.c
@@ -507,6 +507,35 @@  flow_loops_find (struct loops *loops)
 	      loop->latch = latch;
 	    }
 	}
+      /* 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.  */
+      if (loop->latch && !(loop->latch->flags & BB_RTL))
+	FOR_EACH_EDGE (e, ei, loop->latch->succs)
+	  {
+	    gimple_stmt_iterator gsi = gsi_last_bb (e->dest);
+	    gimple 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_low_cst (gimple_call_arg (stmt, 1),
+								     0)
+		    != annot_expr_ivdep_kind)
+		  continue;
+		stmt = gimple_build_assign (gimple_call_lhs (stmt),
+					    gimple_call_arg (stmt, 0));
+		gsi_replace (&gsi, stmt, true);
+		loop->safelen = INT_MAX;
+	      }
+	  }
     }
 
   larray.release ();
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 9b641b2..a6fe610 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -15473,6 +15473,7 @@  for further explanation.
 * Visibility Pragmas::
 * Push/Pop Macro Pragmas::
 * Function Specific Option Pragmas::
+* Loop-Specific Pragmas::
 @end menu
 
 @node ARM Pragmas
@@ -15995,6 +15996,20 @@  The @samp{#pragma GCC reset_options} pragma is not implemented in GCC
 versions earlier than 4.4.
 @end table
 
+@node Loop-Specific Pragmas
+@subsection Loop-Specific Pragmas
+
+@table @code
+@item #pragma ivdep
+@cindex pragma ivdep
+@end table
+
+This pragma tells the compiler that the immediately following @code{for}
+loop can be executed in any loop index order without affecting the result.
+The pragma aids optimization and in particular vectorization as the
+compiler can then assume a vectorization safelen of infinity.
+
+
 @node Unnamed Fields
 @section Unnamed struct/union fields within structs/unions
 @cindex @code{struct}
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 86bda77..187ce05 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3053,6 +3053,16 @@  gimple_boolify (tree expr)
 	TREE_TYPE (expr) = boolean_type_node;
       return expr;
 
+    case ANNOTATE_EXPR:
+      if (ANNOTATE_EXPR_ID (expr) == annot_expr_ivdep_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;
+	}
+      /* FALLTHRU */
+
     default:
       if (COMPARISON_CLASS_P (expr))
 	{
@@ -7378,6 +7388,22 @@  gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	  ret = gimplify_addr_expr (expr_p, pre_p, post_p);
 	  break;
 
+	case ANNOTATE_EXPR:
+	  {
+	    tree cond = TREE_OPERAND (*expr_p, 0);
+	    tree id = build_int_cst (integer_type_node,
+				     ANNOTATE_EXPR_ID (*expr_p));
+	    tree tmp = create_tmp_var_raw (TREE_TYPE(cond), NULL);
+	    gimplify_arg (&cond, pre_p, EXPR_LOCATION (*expr_p));
+	    gimple call = gimple_build_call_internal (IFN_ANNOTATE, 2,
+						      cond, id);
+	    gimple_call_set_lhs (call, tmp);
+	    gimplify_seq_add_stmt (pre_p, call);
+	    *expr_p = tmp;
+	    ret = GS_ALL_DONE;
+	    break;
+	  }
+
 	case VA_ARG_EXPR:
 	  ret = gimplify_va_arg_expr (expr_p, pre_p, post_p);
 	  break;
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 983efeb..a22f222 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -109,6 +109,12 @@  expand_STORE_LANES (gimple stmt)
   expand_insn (get_multi_vector_move (type, vec_store_lanes_optab), 2, ops);
 }
 
+static void
+expand_ANNOTATE (gimple stmt ATTRIBUTE_UNUSED)
+{
+  gcc_unreachable ();
+}
+
 /* This should get expanded in adjust_simduid_builtins.  */
 
 static void
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index 5427664..0f5cc3c 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -43,3 +43,4 @@  DEF_INTERNAL_FN (STORE_LANES, ECF_CONST | ECF_LEAF)
 DEF_INTERNAL_FN (GOMP_SIMD_LANE, ECF_NOVOPS | ECF_LEAF | ECF_NOTHROW)
 DEF_INTERNAL_FN (GOMP_SIMD_VF, ECF_CONST | ECF_LEAF | ECF_NOTHROW)
 DEF_INTERNAL_FN (GOMP_SIMD_LAST_LANE, ECF_CONST | ECF_LEAF | ECF_NOTHROW)
+DEF_INTERNAL_FN (ANNOTATE,  ECF_CONST | ECF_LEAF | ECF_NOTHROW)
diff --git a/gcc/testsuite/gcc.dg/vect/vect-ivdep-1.c b/gcc/testsuite/gcc.dg/vect/vect-ivdep-1.c
new file mode 100644
index 0000000..470fa18
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-ivdep-1.c
@@ -0,0 +1,19 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_float } */
+/* { dg-options "-O3 -fopt-info-vec-optimized" } */
+
+/* PR other/33426 */
+/* Testing whether #pragma ivdep is working.  */
+
+void foo(int n, int *a, int *b, int *c, int *d, int *e) {
+  int i, j;
+#pragma ivdep
+  for (i = 0; i < n; ++i) {
+    a[i] = b[i] + c[i];
+  }
+}
+
+/* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
+/* { dg-bogus "version" "" { target *-*-* } 0 } */
+/* { dg-bogus "alias" "" { target *-*-* } 0 } */
+/* { dg-final { cleanup-tree-dump "vect" } } */
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 0b3314b..2d07a45 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -593,6 +593,10 @@  enum tree_node_kind {
   all_kinds
 };
 
+enum annot_expr_kind {
+  annot_expr_ivdep_kind
+};
+
 
 /*---------------------------------------------------------------------------
                                 Type definitions
@@ -692,7 +696,8 @@  struct GTY(()) tree_base {
        make better use of the 4-byte sized word.  */
     /* VEC length.  This field is only used with TREE_VEC.  */
     int length;
-    /* SSA version number.  This field is only used with SSA_NAME.  */
+    /* SSA version number or the ID of an ANNOTATE_EXPR.  This field is only
+       used with SSA_NAME and ANNOTATE_EXPR.  */
     unsigned int version;
   } GTY((skip(""))) u;
 };
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index c357b06..071fc43 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -1924,6 +1924,18 @@  dump_generic_node (pretty_printer *buffer, tree node, int spc, int flags,
       pp_string (buffer, " predictor.");
       break;
 
+    case ANNOTATE_EXPR:
+      pp_string (buffer, "ANNOTATE_EXPR <");
+      switch (ANNOTATE_EXPR_ID (node))
+	{
+	case annot_expr_ivdep_kind:
+	  pp_string (buffer, "ivdep, ");
+	  break;
+	}
+      dump_generic_node (buffer, TREE_OPERAND (node, 0), spc, flags, false);
+      pp_greater (buffer);
+      break;
+
     case RETURN_EXPR:
       pp_string (buffer, "return");
       op0 = TREE_OPERAND (node, 0);
diff --git a/gcc/tree.def b/gcc/tree.def
index f825aad..040f46d 100644
--- a/gcc/tree.def
+++ b/gcc/tree.def
@@ -1231,6 +1231,12 @@  DEFTREECODE (OPTIMIZATION_NODE, "optimization_node", tcc_exceptional, 0)
 /* TARGET_OPTION_NODE.  Node to store the target specific options.  */
 DEFTREECODE (TARGET_OPTION_NODE, "target_option_node", tcc_exceptional, 0)
 
+/* ANNOTATE_EXPR.
+   Operand 0 is the expression.  ....
+   Operand 1 is the annotation id, FIXME */
+DEFTREECODE (ANNOTATE_EXPR, "annotate_expr", tcc_expression, 1)
+
+
 /*
 Local variables:
 mode:c
diff --git a/gcc/tree.h b/gcc/tree.h
index 0fdebfb..a8951c2 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -591,6 +591,11 @@  extern void omp_clause_range_check_failed (const_tree, const char *, int,
 #define PREDICT_EXPR_PREDICTOR(NODE) \
   ((enum br_predictor)tree_low_cst (TREE_OPERAND (PREDICT_EXPR_CHECK (NODE), 0), 0))
 
+#define ANNOTATE_EXPR_ID(NODE) \
+  ((enum annot_expr_kind) ANNOTATE_EXPR_CHECK (NODE)->base.u.version)
+#define SET_ANNOTATE_EXPR_ID(NODE, ID) \
+  (ANNOTATE_EXPR_CHECK (NODE)->base.u.version = (unsigned int) (ID))
+
 /* In a VAR_DECL, nonzero means allocate static storage.
    In a FUNCTION_DECL, nonzero if function has been defined.
    In a CONSTRUCTOR, nonzero means allocate static storage.  */