public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Frederik Harwath <frederik@codesourcery.com>
To: <gcc-patches@gcc.gnu.org>
Cc: Sandra Loosemore <sandra@codesourcery.com>,
	<thomas@codesourcery.com>, <joseph@codesourcery.com>,
	<jason@redhat.com>, <nathan@acm.org>
Subject: [PATCH 12/40] Relax some restrictions on the loop bound in kernels loop annotation.
Date: Wed, 15 Dec 2021 16:54:19 +0100	[thread overview]
Message-ID: <20211215155447.19379-13-frederik@codesourcery.com> (raw)
In-Reply-To: <20211215155447.19379-1-frederik@codesourcery.com>

From: Sandra Loosemore <sandra@codesourcery.com>

OpenACC loop semantics require that the loop bound be computable
before entering the loop, rather than the C/C++ semantics where the
end test is evaluated on every iteration.  Formerly the kernels loop
annotater permitted only constants and variables not modified in the
loop body in the loop bound expression.  This patch relaxes those
restrictions somewhat to allow many forms of expressions involving
such constants and variables, including calls to constant functions.

2020-08-30  Sandra Loosemore  <sandra@codesourcery.com>

        gcc/c-family/
        * c-omp.c (end_test_ok_for_annotation_r): New.
        (end_test_ok_for_annotation): New.
        (check_and_annotate_for_loop): Use the new helper function.

        gcc/testsuite/
        * c-c++-common/goacc/kernels-loop-annotation-21.c: New.
        * c-c++-common/goacc/kernels-loop-annotation-22.c: New.
---
 gcc/c-family/c-omp.c                          | 120 ++++++++++++++++--
 .../goacc/kernels-loop-annotation-21.c        |  42 ++++++
 .../goacc/kernels-loop-annotation-22.c        |  41 ++++++
 3 files changed, 194 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/goacc/kernels-loop-annotation-21.c
 create mode 100644 gcc/testsuite/c-c++-common/goacc/kernels-loop-annotation-22.c

diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index e73fb5d01f7e..dc63d304ca67 100644
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -3165,6 +3165,116 @@ is_local_var (tree decl)
          && !TREE_ADDRESSABLE (decl));
 }

+/* EXP is a loop bound expression for a comparison against local
+   variable DECL.  Check whether this is potentially valid in an OpenACC loop
+   context, namely that it can be precomputed when entering the loop
+   construct per the OpenACC specification.  Local variables referenced
+   in both DECL and EXP that may not be modified in the body of the loop
+   are added to the list in INFO to be checked later.
+
+   FIXME: Ideally we would like to make this test permissive rather than
+   restrictive, and allow the later conversion of the "auto" attribute to
+   either "seq" or "independent" to make the determination using dataflow,
+   alias analysis, etc rather than a tree traversal.  But presently it does
+   not do that and always just hoists the loop bound expression.  So the
+   current implementation only considers expressions involving unmodified
+   local variables and constants, using a tree walk.  */
+
+static tree
+end_test_ok_for_annotation_r (tree *tp, int *walk_subtrees,
+                             void *data)
+{
+  tree exp = *tp;
+  struct annotation_info *info = (struct annotation_info *) data;
+
+  switch (TREE_CODE_CLASS (TREE_CODE (exp)))
+    {
+    case tcc_constant:
+      /* Constants are trivially known to be invariant.  */
+      return NULL_TREE;
+
+    case tcc_declaration:
+      if (is_local_var (exp))
+       {
+         tree t;
+         /* Add it to the list of variables that can't be modified in the
+            loop, only if not already present.  */
+         for (t = info->vars; t && TREE_VALUE (t) != exp;
+              t = TREE_CHAIN (t))
+           ;
+         if (!t)
+           info->vars = tree_cons (NULL_TREE, exp, info->vars);
+         return NULL_TREE;
+       }
+      else if (TREE_CODE (exp) == VAR_DECL && TREE_READONLY (exp))
+       return NULL_TREE;
+      else if (TREE_CODE (exp) == FUNCTION_DECL)
+       return NULL_TREE;
+      break;
+
+    case tcc_unary:
+    case tcc_binary:
+    case tcc_comparison:
+      /* Allow arithmetic expressions and comparisons provided
+        that the operands are good.  */
+      return NULL_TREE;
+
+    default:
+      /* Handle some special cases.  */
+      switch (TREE_CODE (exp))
+       {
+       case COND_EXPR:
+       case TRUTH_ANDIF_EXPR:
+       case TRUTH_ORIF_EXPR:
+       case TRUTH_AND_EXPR:
+       case TRUTH_OR_EXPR:
+       case TRUTH_XOR_EXPR:
+       case TRUTH_NOT_EXPR:
+         /* ?: and boolean operators are OK.  */
+         return NULL_TREE;
+
+       case CALL_EXPR:
+         /* Allow calls to constant functions with invariant operands.  */
+         {
+           tree fndecl = get_callee_fndecl (exp);
+           if (fndecl && TREE_READONLY (fndecl))
+             return NULL_TREE;
+         }
+         break;
+
+       case ADDR_EXPR:
+         /* We can expect addresses of things to be invariant.  */
+         return NULL_TREE;
+
+       default:
+         break;
+       }
+    }
+
+  /* Reject anything else.  */
+  *walk_subtrees = 0;
+  return exp;
+}
+
+static bool
+end_test_ok_for_annotation (tree decl, tree exp,
+                           struct annotation_info *info)
+{
+  /* Traversal returns NULL_TREE if all is well.  */
+  if (!walk_tree (&exp, end_test_ok_for_annotation_r, info, NULL))
+    {
+      /* So far, so good.  Check the decl against any variables collected
+        in the exp.  */
+      tree t;
+      for (t = info->vars; t; t = TREE_CHAIN (t))
+       if (TREE_VALUE (t) == decl)
+         return false;
+      info->vars = tree_cons (NULL_TREE, decl, info->vars);
+      return true;
+    }
+  return false;
+}
+
 /* The initializer for a FOR_STMT is sometimes wrapped in various other
    language-specific tree structures.  We need a hook to unwrap them.
    This function takes a tree argument and should return either a
@@ -3333,16 +3443,8 @@ check_and_annotate_for_loop (tree *nodeptr, tree_stmt_iterator *prev_tsi,
        limit_exp = TREE_OPERAND (cond, 0);

       if (!limit_exp
-         || (!is_local_var (limit_exp)
-             && (TREE_CODE_CLASS (TREE_CODE (limit_exp)) != tcc_constant)))
+         || !end_test_ok_for_annotation (decl, limit_exp, &loop_info))
        do_not_annotate_loop (&loop_info, as_invalid_predicate, cond);
-      else
-       {
-         /* These variables must not be assigned to in the loop.  */
-         loop_info.vars = tree_cons (NULL_TREE, decl, loop_info.vars);
-         if (TREE_CODE_CLASS (TREE_CODE (limit_exp)) != tcc_constant)
-           loop_info.vars = tree_cons (NULL_TREE, limit_exp, loop_info.vars);
-       }
     }

   /* Walk the body.  This will process any nested loops, so we have to do it
diff --git a/gcc/testsuite/c-c++-common/goacc/kernels-loop-annotation-21.c b/gcc/testsuite/c-c++-common/goacc/kernels-loop-annotation-21.c
new file mode 100644
index 000000000000..f87444ede4b4
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/goacc/kernels-loop-annotation-21.c
@@ -0,0 +1,42 @@
+/* { dg-additional-options "-fopenacc -fopenacc-kernels-annotate-loops" } */
+/* { dg-additional-options "-Wopenacc-kernels-annotate-loops" } */
+/* { dg-additional-options "-fdump-tree-original" } */
+/* { dg-do compile } */
+
+/* Test for rejecting annotation on loops that have various subexpressions
+   in the loop end test that are not loop-invariant.  */
+
+extern int g (int);
+extern int x;
+extern int gg (int, int) __attribute__ ((const));
+
+void f (float *a, float *b, int n)
+{
+
+  int j;
+#pragma acc kernels
+  {
+    /* Non-constant function call.  */
+    for (int i = 0; i < g(n); i++)     /* { dg-warning "loop cannot be annotated" } */
+      a[i] = b[i];
+
+    /* Global variable.  */
+    for (int i = x; i < n + x; i++)    /* { dg-warning "loop cannot be annotated" } */
+      a[i] = b[i];
+
+    /* Explicit reference to the loop variable.  */
+    for (int i = 0; i < gg (i, n); i++)        /* { dg-warning "loop cannot be annotated" } */
+      a[i] = b[i];
+
+    /* Reference to a variable that is modified in the body of the loop.  */
+    j = 0;
+    for (int i = 0; i < gg (j, n); i++)        /* { dg-warning "loop cannot be annotated" } */
+      {
+       a[i] = b[i];
+       j = i;
+      }
+
+  }
+}
+
+/* { dg-final { scan-tree-dump-times "acc loop auto" 0 "original" } } */
diff --git a/gcc/testsuite/c-c++-common/goacc/kernels-loop-annotation-22.c b/gcc/testsuite/c-c++-common/goacc/kernels-loop-annotation-22.c
new file mode 100644
index 000000000000..6a5099d2ff9d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/goacc/kernels-loop-annotation-22.c
@@ -0,0 +1,41 @@
+/* { dg-additional-options "-fopenacc -fopenacc-kernels-annotate-loops" } */
+/* { dg-additional-options "-Wopenacc-kernels-annotate-loops" } */
+/* { dg-additional-options "-fdump-tree-original" } */
+/* { dg-do compile } */
+
+/* Test for accepting annotation on loops that have various forms of
+   loop-invariant expressions in their end test.  */
+
+extern const int x;
+extern int g (int) __attribute__ ((const));
+
+void f (float *a, float *b, int n)
+{
+
+  int j;
+#pragma acc kernels
+  {
+    /* Reversed form of comparison.  */
+    for (int i = 0; n >= i; i++)
+      a[i] = b[i];
+
+    /* Constant function call.  */
+    for (int i = 0; i < g(n); i++)
+      a[i] = b[i];
+
+    /* Constant global variable.  */
+    for (int i = 0; i < x; i++)
+      a[i] = b[i];
+
+    /* Complicated expression involving conditionals, etc. */
+    for (int i = 0; i < ((x == 4) ? (n << 2) : (n << 3)); i++)
+      a[i] = b[i];
+
+    /* Reference to a local variable not modified in the loop.  */
+    j = ((x == 4) ? (n << 2) : (n << 3));
+    for (int i = 0; i < j; i++)
+      a[i] = b[i];
+  }
+}
+
+/* { dg-final { scan-tree-dump-times "acc loop auto" 5 "original" } } */
--
2.33.0

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

  parent reply	other threads:[~2021-12-15 15:55 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-15 15:54 [PATCH 00/40] OpenACC "kernels" Improvements Frederik Harwath
2021-12-15 15:54 ` [PATCH 01/40] Kernels loops annotation: C and C++ Frederik Harwath
2021-12-15 15:54 ` [PATCH 02/40] Add -fno-openacc-kernels-annotate-loops option to more testcases Frederik Harwath
2021-12-15 15:54 ` [PATCH 03/40] Kernels loops annotation: Fortran Frederik Harwath
2021-12-15 15:54 ` [PATCH 04/40] Additional Fortran testsuite fixes for kernels loops annotation pass Frederik Harwath
2021-12-15 15:54 ` [PATCH 05/40] Fix bug in processing of array dimensions in data clauses Frederik Harwath
2021-12-15 15:54 ` [PATCH 06/40] Add a "combined" flag for "acc kernels loop" etc directives Frederik Harwath
2021-12-15 15:54 ` [PATCH 07/40] Annotate inner loops in "acc kernels loop" directives (C/C++) Frederik Harwath
2021-12-15 15:54 ` [PATCH 08/40] Annotate inner loops in "acc kernels loop" directives (Fortran) Frederik Harwath
2021-12-15 15:54 ` [PATCH 09/40] Permit calls to builtins and intrinsics in kernels loops Frederik Harwath
2021-12-15 15:54 ` [PATCH 10/40] Fix patterns in Fortran tests for kernels loop annotation Frederik Harwath
2021-12-15 15:54 ` [PATCH 11/40] Clean up loop variable extraction in OpenACC " Frederik Harwath
2021-12-15 15:54 ` Frederik Harwath [this message]
2021-12-15 15:54 ` [PATCH 13/40] Fortran: Delinearize array accesses Frederik Harwath
2021-12-15 15:54 ` [PATCH 14/40] openacc: Move pass_oacc_device_lower after pass_graphite Frederik Harwath
2021-12-15 15:54 ` [PATCH 15/40] graphite: Extend SCoP detection dump output Frederik Harwath
2022-05-16 12:49   ` Tobias Burnus
2022-05-17  8:21     ` Richard Biener
2022-05-18 12:19       ` Harwath, Frederik
2022-05-18 12:21         ` Richard Biener
2021-12-15 15:54 ` [PATCH 16/40] graphite: Rename isl_id_for_ssa_name Frederik Harwath
2022-05-16 12:49   ` Tobias Burnus
2022-05-17  8:22     ` Richard Biener
2021-12-15 15:54 ` [PATCH 17/40] graphite: Fix minor mistakes in comments Frederik Harwath
2022-05-16 12:49   ` Tobias Burnus
2022-05-17  8:22     ` Richard Biener
2021-12-15 15:54 ` [PATCH 18/40] Move compute_alias_check_pairs to tree-data-ref.c Frederik Harwath
2021-12-15 15:54 ` [PATCH 19/40] graphite: Add runtime alias checking Frederik Harwath
2021-12-15 15:54 ` [PATCH 20/40] openacc: Use Graphite for dependence analysis in "kernels" regions Frederik Harwath
2021-12-15 15:54 ` [PATCH 21/40] openacc: Add "can_be_parallel" flag info to "graph" dumps Frederik Harwath
2021-12-15 15:54 ` [PATCH 22/40] openacc: Remove unused partitioning in "kernels" regions Frederik Harwath
2021-12-15 15:54 ` [PATCH 23/40] Add function for printing a single OMP_CLAUSE Frederik Harwath
2021-12-15 15:54 ` [PATCH 24/40] openacc: Add data optimization pass Frederik Harwath
2021-12-15 15:54 ` [PATCH 25/40] openacc: Add runtime alias checking for OpenACC kernels Frederik Harwath
2021-12-15 15:54 ` [PATCH 26/40] openacc: Warn about "independent" "kernels" loops with data-dependences Frederik Harwath
2021-12-15 15:54 ` [PATCH 27/40] openacc: Handle internal function calls in pass_lim Frederik Harwath
2021-12-15 15:54 ` [PATCH 28/40] openacc: Disable pass_pre on outlined functions analyzed by Graphite Frederik Harwath
2021-12-15 15:54 ` [PATCH 29/40] graphite: Tune parameters for OpenACC use Frederik Harwath
2021-12-15 15:54 ` [PATCH 30/40] graphite: Adjust scop loop-nest choice Frederik Harwath
2021-12-15 15:54 ` [PATCH 31/40] graphite: Accept loops without data references Frederik Harwath
2021-12-15 15:54 ` [PATCH 32/40] Reference reduction localization Frederik Harwath
2021-12-15 15:54 ` [PATCH 33/40] Fix tree check failure with " Frederik Harwath
2021-12-15 15:54 ` [PATCH 34/40] Use more appropriate var in localize_reductions call Frederik Harwath
2021-12-15 15:54 ` [PATCH 35/40] Handle references in OpenACC "private" clauses Frederik Harwath
2021-12-15 15:54 ` [PATCH 36/40] openacc: Enable reduction variable localization for "kernels" Frederik Harwath
2021-12-15 15:54 ` [PATCH 37/40] Fix for is_gimple_reg vars to 'data kernels' Frederik Harwath
2021-12-15 15:54 ` [PATCH 38/40] openacc: fix privatization of by-reference arrays Frederik Harwath
2021-12-15 15:54 ` [PATCH 39/40] openacc: Check type for references in reduction lowering Frederik Harwath
2021-12-16 12:00 ` [PATCH 40/40] openacc: Adjust testsuite to new "kernels" handling Frederik Harwath

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211215155447.19379-13-frederik@codesourcery.com \
    --to=frederik@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=nathan@acm.org \
    --cc=sandra@codesourcery.com \
    --cc=thomas@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).