diff mbox

OMP_CLAUSES with clauses in operand 0

Message ID 871tj3roor.fsf@schwinge.name
State New
Headers show

Commit Message

Thomas Schwinge April 29, 2015, 2:31 p.m. UTC
Hi Jakub!

On Wed, 29 Apr 2015 13:43:55 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Apr 29, 2015 at 01:13:29PM +0200, Thomas Schwinge wrote:
> > On Wed, 29 Apr 2015 11:32:31 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> > > Yeah, it is a non-starter, it has unnecessary runtime overhead everywhere
> > > where it is used.
> > 
> > Huh.  OMP_CLAUSES is currently used in a dozen places in
> > cp/cp-gimplify.c, cp/pt.c, and gimplify.c.  I've been expecting my
> > changed code to be well-optimizable, for the compiler already knows
> > TREE_CODE(NODE) in a lot of these places.  Doing a quick before (1)/after
> > (2) comparison test with -g -O2 on x86_64 GNU/Linux using GCC 4.8.3, the
> > object file sizes are as follows:
> > 
> >        text    data     bss     dec     hex filename
> >       37027       0      16   37043    90b3 1/build-gcc/gcc/cp/cp-gimplify.o
> >       36307       0      16   36323    8de3 2/build-gcc/gcc/cp/cp-gimplify.o
> >        text    data     bss     dec     hex filename
> >      458742       0     136  458878   7007e 1/build-gcc/gcc/cp/pt.o
> >      458630       0     136  458766   7000e 2/build-gcc/gcc/cp/pt.o
> >        text    data     bss     dec     hex filename
> >      166056       0      48  166104   288d8 1/build-gcc/gcc/gimplify.o
> >      166200       0      48  166248   28968 2/build-gcc/gcc/gimplify.o
> > 
> > ..., so actually smaller for the first two files.  Admittedly, there is a
> > 144 bytes (0.0867 %) size increase in gimplify.o, and I have not measured
> > the actual runtime overhead (which I'm totally expecting to be "in the
> > noise", but...), so I'm assuming that my proposal to keep the interface
> > simple does not pay for the presumed runtime overhead, so I guess I'm
> > posting this patch just for the archives...
> 
> I really don't understand how you could get smaller code out of that, that
> doesn't make any sense.

I tried a quick -fdump-tree-all comparison but that doesn't lead
anywhere, because a vast number of IDs change.  Any tricks how to tackle
such a thing?


> So yes, I really prefer OMP_STANDALONE_CLAUSES over OMP_CLAUSES for
> everything.

Like this (for trunk)?

commit 300e28fce192cb56d73cb61f787872643030f0bf
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Wed Apr 29 16:18:49 2015 +0200

    Add OMP_STANDALONE_CLAUSES.
    
    	gcc/
    	* tree.h (OACC_CACHE_CLAUSES, OACC_DECLARE_CLAUSES)
    	(OACC_ENTER_DATA_CLAUSES, OACC_EXIT_DATA_CLAUSES)
    	(OACC_UPDATE_CLAUSES, OMP_TARGET_UPDATE_CLAUSES): Merge into...
    	(OMP_STANDALONE_CLAUSES): ... this new macro.  Adjust all users.
---
 gcc/c/c-parser.c        |   11 ++++-------
 gcc/cp/parser.c         |   11 ++++-------
 gcc/cp/pt.c             |    4 ++--
 gcc/gimplify.c          |   18 ++++++++----------
 gcc/tree-pretty-print.c |   12 ++++++------
 gcc/tree.def            |   12 ++++++------
 gcc/tree.h              |   19 ++-----------------
 7 files changed, 32 insertions(+), 55 deletions(-)



Grüße,
 Thomas

Comments

Jakub Jelinek April 29, 2015, 2:36 p.m. UTC | #1
On Wed, Apr 29, 2015 at 04:31:32PM +0200, Thomas Schwinge wrote:
> > So yes, I really prefer OMP_STANDALONE_CLAUSES over OMP_CLAUSES for
> > everything.
> 
> Like this (for trunk)?
> 
> commit 300e28fce192cb56d73cb61f787872643030f0bf
> Author: Thomas Schwinge <thomas@codesourcery.com>
> Date:   Wed Apr 29 16:18:49 2015 +0200
> 
>     Add OMP_STANDALONE_CLAUSES.
>     
>     	gcc/
>     	* tree.h (OACC_CACHE_CLAUSES, OACC_DECLARE_CLAUSES)
>     	(OACC_ENTER_DATA_CLAUSES, OACC_EXIT_DATA_CLAUSES)
>     	(OACC_UPDATE_CLAUSES, OMP_TARGET_UPDATE_CLAUSES): Merge into...
>     	(OMP_STANDALONE_CLAUSES): ... this new macro.  Adjust all users.

I would keep the specific *_CLAUSES macros, just add
OMP_STANDALONE_CLAUSES and change the uses only if you are dealing with
multiple different codes.  That will match OMP_CLAUSES vs. OMP_FOR_CLAUSES,
OMP_PARALLEL_CLAUSES etc.

> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -11987,7 +11987,7 @@ c_parser_oacc_cache (location_t loc, c_parser *parser)
>  
>    stmt = make_node (OACC_CACHE);
>    TREE_TYPE (stmt) = void_type_node;
> -  OACC_CACHE_CLAUSES (stmt) = clauses;
> +  OMP_STANDALONE_CLAUSES (stmt) = clauses;
>    SET_EXPR_LOCATION (stmt, loc);
>    add_stmt (stmt);
>  

So, drop hunks like this.

> @@ -12155,10 +12155,7 @@ c_parser_oacc_enter_exit_data (c_parser *parser, bool enter)
>  
>    stmt = enter ? make_node (OACC_ENTER_DATA) : make_node (OACC_EXIT_DATA);
>    TREE_TYPE (stmt) = void_type_node;
> -  if (enter)
> -    OACC_ENTER_DATA_CLAUSES (stmt) = clauses;
> -  else
> -    OACC_EXIT_DATA_CLAUSES (stmt) = clauses;
> +  OMP_STANDALONE_CLAUSES (stmt) = clauses;
>    SET_EXPR_LOCATION (stmt, loc);
>    add_stmt (stmt);
>  }

And just keep ones like this.

	Jakub
diff mbox

Patch

diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index f5e2ac2c..86b77f3 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -11987,7 +11987,7 @@  c_parser_oacc_cache (location_t loc, c_parser *parser)
 
   stmt = make_node (OACC_CACHE);
   TREE_TYPE (stmt) = void_type_node;
-  OACC_CACHE_CLAUSES (stmt) = clauses;
+  OMP_STANDALONE_CLAUSES (stmt) = clauses;
   SET_EXPR_LOCATION (stmt, loc);
   add_stmt (stmt);
 
@@ -12155,10 +12155,7 @@  c_parser_oacc_enter_exit_data (c_parser *parser, bool enter)
 
   stmt = enter ? make_node (OACC_ENTER_DATA) : make_node (OACC_EXIT_DATA);
   TREE_TYPE (stmt) = void_type_node;
-  if (enter)
-    OACC_ENTER_DATA_CLAUSES (stmt) = clauses;
-  else
-    OACC_EXIT_DATA_CLAUSES (stmt) = clauses;
+  OMP_STANDALONE_CLAUSES (stmt) = clauses;
   SET_EXPR_LOCATION (stmt, loc);
   add_stmt (stmt);
 }
@@ -12285,7 +12282,7 @@  c_parser_oacc_update (c_parser *parser)
 
   tree stmt = make_node (OACC_UPDATE);
   TREE_TYPE (stmt) = void_type_node;
-  OACC_UPDATE_CLAUSES (stmt) = clauses;
+  OMP_STANDALONE_CLAUSES (stmt) = clauses;
   SET_EXPR_LOCATION (stmt, loc);
   add_stmt (stmt);
 }
@@ -13858,7 +13855,7 @@  c_parser_omp_target_update (location_t loc, c_parser *parser,
 
   tree stmt = make_node (OMP_TARGET_UPDATE);
   TREE_TYPE (stmt) = void_type_node;
-  OMP_TARGET_UPDATE_CLAUSES (stmt) = clauses;
+  OMP_STANDALONE_CLAUSES (stmt) = clauses;
   SET_EXPR_LOCATION (stmt, loc);
   add_stmt (stmt);
   return false;
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 4ea2ca2..61fd34f 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -31386,7 +31386,7 @@  cp_parser_omp_target_update (cp_parser *parser, cp_token *pragma_tok,
 
   tree stmt = make_node (OMP_TARGET_UPDATE);
   TREE_TYPE (stmt) = void_type_node;
-  OMP_TARGET_UPDATE_CLAUSES (stmt) = clauses;
+  OMP_STANDALONE_CLAUSES (stmt) = clauses;
   SET_EXPR_LOCATION (stmt, pragma_tok->location);
   add_stmt (stmt);
   return false;
@@ -31496,7 +31496,7 @@  cp_parser_oacc_cache (cp_parser *parser, cp_token *pragma_tok)
 
   stmt = make_node (OACC_CACHE);
   TREE_TYPE (stmt) = void_type_node;
-  OACC_CACHE_CLAUSES (stmt) = clauses;
+  OMP_STANDALONE_CLAUSES (stmt) = clauses;
   SET_EXPR_LOCATION (stmt, pragma_tok->location);
   add_stmt (stmt);
 
@@ -31606,10 +31606,7 @@  cp_parser_oacc_enter_exit_data (cp_parser *parser, cp_token *pragma_tok,
 
   stmt = enter ? make_node (OACC_ENTER_DATA) : make_node (OACC_EXIT_DATA);
   TREE_TYPE (stmt) = void_type_node;
-  if (enter)
-    OACC_ENTER_DATA_CLAUSES (stmt) = clauses;
-  else
-    OACC_EXIT_DATA_CLAUSES (stmt) = clauses;
+  OMP_STANDALONE_CLAUSES (stmt) = clauses;
   SET_EXPR_LOCATION (stmt, pragma_tok->location);
   add_stmt (stmt);
   return stmt;
@@ -31746,7 +31743,7 @@  cp_parser_oacc_update (cp_parser *parser, cp_token *pragma_tok)
 
   stmt = make_node (OACC_UPDATE);
   TREE_TYPE (stmt) = void_type_node;
-  OACC_UPDATE_CLAUSES (stmt) = clauses;
+  OMP_STANDALONE_CLAUSES (stmt) = clauses;
   SET_EXPR_LOCATION (stmt, pragma_tok->location);
   add_stmt (stmt);
   return stmt;
diff --git gcc/cp/pt.c gcc/cp/pt.c
index 129517a..9e83c22 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -14203,10 +14203,10 @@  tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
       break;
 
     case OMP_TARGET_UPDATE:
-      tmp = tsubst_omp_clauses (OMP_TARGET_UPDATE_CLAUSES (t), false,
+      tmp = tsubst_omp_clauses (OMP_STANDALONE_CLAUSES (t), false,
 				args, complain, in_decl);
       t = copy_node (t);
-      OMP_TARGET_UPDATE_CLAUSES (t) = tmp;
+      OMP_STANDALONE_CLAUSES (t) = tmp;
       add_stmt (t);
       break;
 
diff --git gcc/gimplify.c gcc/gimplify.c
index c68bd47..a68748a 100644
--- gcc/gimplify.c
+++ gcc/gimplify.c
@@ -6799,8 +6799,9 @@  gimplify_oacc_cache (tree *expr_p, gimple_seq *pre_p)
 {
   tree expr = *expr_p;
 
-  gimplify_scan_omp_clauses (&OACC_CACHE_CLAUSES (expr), pre_p, ORT_WORKSHARE);
-  gimplify_adjust_omp_clauses (pre_p, &OACC_CACHE_CLAUSES (expr));
+  gimplify_scan_omp_clauses (&OMP_STANDALONE_CLAUSES (expr), pre_p,
+			     ORT_WORKSHARE);
+  gimplify_adjust_omp_clauses (pre_p, &OMP_STANDALONE_CLAUSES (expr));
 
   /* TODO: Do something sensible with this information.  */
 
@@ -7427,34 +7428,31 @@  gimplify_omp_workshare (tree *expr_p, gimple_seq *pre_p)
 static void
 gimplify_omp_target_update (tree *expr_p, gimple_seq *pre_p)
 {
-  tree expr = *expr_p, clauses;
+  tree expr = *expr_p;
   int kind;
   gomp_target *stmt;
 
   switch (TREE_CODE (expr))
     {
     case OACC_ENTER_DATA:
-      clauses = OACC_ENTER_DATA_CLAUSES (expr);
       kind = GF_OMP_TARGET_KIND_OACC_ENTER_EXIT_DATA;
       break;
     case OACC_EXIT_DATA:
-      clauses = OACC_EXIT_DATA_CLAUSES (expr);
       kind = GF_OMP_TARGET_KIND_OACC_ENTER_EXIT_DATA;
       break;
     case OACC_UPDATE:
-      clauses = OACC_UPDATE_CLAUSES (expr);
       kind = GF_OMP_TARGET_KIND_OACC_UPDATE;
       break;
     case OMP_TARGET_UPDATE:
-      clauses = OMP_TARGET_UPDATE_CLAUSES (expr);
       kind = GF_OMP_TARGET_KIND_UPDATE;
       break;
     default:
       gcc_unreachable ();
     }
-  gimplify_scan_omp_clauses (&clauses, pre_p, ORT_WORKSHARE);
-  gimplify_adjust_omp_clauses (pre_p, &clauses);
-  stmt = gimple_build_omp_target (NULL, kind, clauses);
+  gimplify_scan_omp_clauses (&OMP_STANDALONE_CLAUSES (expr), pre_p,
+			     ORT_WORKSHARE);
+  gimplify_adjust_omp_clauses (pre_p, &OMP_STANDALONE_CLAUSES (expr));
+  stmt = gimple_build_omp_target (NULL, kind, OMP_STANDALONE_CLAUSES (expr));
 
   gimplify_seq_add_stmt (pre_p, stmt);
   *expr_p = NULL_TREE;
diff --git gcc/tree-pretty-print.c gcc/tree-pretty-print.c
index d7c049f..26f517f 100644
--- gcc/tree-pretty-print.c
+++ gcc/tree-pretty-print.c
@@ -2585,27 +2585,27 @@  dump_generic_node (pretty_printer *pp, tree node, int spc, int flags,
 
     case OACC_DECLARE:
       pp_string (pp, "#pragma acc declare");
-      dump_omp_clauses (pp, OACC_DECLARE_CLAUSES (node), spc, flags);
+      dump_omp_clauses (pp, OMP_STANDALONE_CLAUSES (node), spc, flags);
       break;
 
     case OACC_UPDATE:
       pp_string (pp, "#pragma acc update");
-      dump_omp_clauses (pp, OACC_UPDATE_CLAUSES (node), spc, flags);
+      dump_omp_clauses (pp, OMP_STANDALONE_CLAUSES (node), spc, flags);
       break;
 
     case OACC_ENTER_DATA:
       pp_string (pp, "#pragma acc enter data");
-      dump_omp_clauses (pp, OACC_ENTER_DATA_CLAUSES (node), spc, flags);
+      dump_omp_clauses (pp, OMP_STANDALONE_CLAUSES (node), spc, flags);
       break;
 
     case OACC_EXIT_DATA:
       pp_string (pp, "#pragma acc exit data");
-      dump_omp_clauses (pp, OACC_EXIT_DATA_CLAUSES (node), spc, flags);
+      dump_omp_clauses (pp, OMP_STANDALONE_CLAUSES (node), spc, flags);
       break;
 
     case OACC_CACHE:
       pp_string (pp, "#pragma acc cache");
-      dump_omp_clauses (pp, OACC_CACHE_CLAUSES (node), spc, flags);
+      dump_omp_clauses (pp, OMP_STANDALONE_CLAUSES (node), spc, flags);
       break;
 
     case OMP_PARALLEL:
@@ -2673,7 +2673,7 @@  dump_generic_node (pretty_printer *pp, tree node, int spc, int flags,
 
     case OMP_TARGET_UPDATE:
       pp_string (pp, "#pragma omp target update");
-      dump_omp_clauses (pp, OMP_TARGET_UPDATE_CLAUSES (node), spc, flags);
+      dump_omp_clauses (pp, OMP_STANDALONE_CLAUSES (node), spc, flags);
       is_expr = false;
       break;
 
diff --git gcc/tree.def gcc/tree.def
index b4b4164..2dd70b9 100644
--- gcc/tree.def
+++ gcc/tree.def
@@ -1157,28 +1157,28 @@  DEFTREECODE (OMP_ORDERED, "omp_ordered", tcc_statement, 1)
 DEFTREECODE (OMP_CRITICAL, "omp_critical", tcc_statement, 2)
 
 /* OpenACC - #pragma acc cache (variable1 ... variableN)
-   Operand 0: OACC_CACHE_CLAUSES: List of variables (transformed into
+   Operand 0: OMP_STANDALONE_CLAUSES: List of variables (transformed into
 	OMP_CLAUSE__CACHE_ clauses).  */
 DEFTREECODE (OACC_CACHE, "oacc_cache", tcc_statement, 1)
 
 /* OpenACC - #pragma acc declare [clause1 ... clauseN]
-   Operand 0: OACC_DECLARE_CLAUSES: List of clauses.  */
+   Operand 0: OMP_STANDALONE_CLAUSES: List of clauses.  */
 DEFTREECODE (OACC_DECLARE, "oacc_declare", tcc_statement, 1)
 
 /* OpenACC - #pragma acc enter data [clause1 ... clauseN]
-   Operand 0: OACC_ENTER_DATA_CLAUSES: List of clauses.  */
+   Operand 0: OMP_STANDALONE_CLAUSES: List of clauses.  */
 DEFTREECODE (OACC_ENTER_DATA, "oacc_enter_data", tcc_statement, 1)
 
 /* OpenACC - #pragma acc exit data [clause1 ... clauseN]
-   Operand 0: OACC_EXIT_DATA_CLAUSES: List of clauses.  */
+   Operand 0: OMP_STANDALONE_CLAUSES: List of clauses.  */
 DEFTREECODE (OACC_EXIT_DATA, "oacc_exit_data", tcc_statement, 1)
 
 /* OpenACC - #pragma acc update [clause1 ... clauseN]
-   Operand 0: OACC_UPDATE_CLAUSES: List of clauses.  */
+   Operand 0: OMP_STANDALONE_CLAUSES: List of clauses.  */
 DEFTREECODE (OACC_UPDATE, "oacc_update", tcc_statement, 1)
 
 /* OpenMP - #pragma omp target update [clause1 ... clauseN]
-   Operand 0: OMP_TARGET_UPDATE_CLAUSES: List of clauses.  */
+   Operand 0: OMP_STANDALONE_CLAUSES: List of clauses.  */
 DEFTREECODE (OMP_TARGET_UPDATE, "omp_target_update", tcc_statement, 1)
 
 /* OMP_ATOMIC through OMP_ATOMIC_CAPTURE_NEW must be consecutive,
diff --git gcc/tree.h gcc/tree.h
index 2ec9708..3c4633b 100644
--- gcc/tree.h
+++ gcc/tree.h
@@ -1222,21 +1222,6 @@  extern void protected_set_expr_location (tree, location_t);
 #define OACC_HOST_DATA_CLAUSES(NODE) \
   TREE_OPERAND (OACC_HOST_DATA_CHECK (NODE), 1)
 
-#define OACC_CACHE_CLAUSES(NODE) \
-  TREE_OPERAND (OACC_CACHE_CHECK (NODE), 0)
-
-#define OACC_DECLARE_CLAUSES(NODE) \
-  TREE_OPERAND (OACC_DECLARE_CHECK (NODE), 0)
-
-#define OACC_ENTER_DATA_CLAUSES(NODE) \
-  TREE_OPERAND (OACC_ENTER_DATA_CHECK (NODE), 0)
-
-#define OACC_EXIT_DATA_CLAUSES(NODE) \
-  TREE_OPERAND (OACC_EXIT_DATA_CHECK (NODE), 0)
-
-#define OACC_UPDATE_CLAUSES(NODE) \
-  TREE_OPERAND (OACC_UPDATE_CHECK (NODE), 0)
-
 #define OMP_PARALLEL_BODY(NODE)    TREE_OPERAND (OMP_PARALLEL_CHECK (NODE), 0)
 #define OMP_PARALLEL_CLAUSES(NODE) TREE_OPERAND (OMP_PARALLEL_CHECK (NODE), 1)
 
@@ -1283,8 +1268,8 @@  extern void protected_set_expr_location (tree, location_t);
 #define OMP_TARGET_BODY(NODE)	   TREE_OPERAND (OMP_TARGET_CHECK (NODE), 0)
 #define OMP_TARGET_CLAUSES(NODE)   TREE_OPERAND (OMP_TARGET_CHECK (NODE), 1)
 
-#define OMP_TARGET_UPDATE_CLAUSES(NODE)\
-  TREE_OPERAND (OMP_TARGET_UPDATE_CHECK (NODE), 0)
+#define OMP_STANDALONE_CLAUSES(NODE) \
+  TREE_OPERAND (TREE_RANGE_CHECK (NODE, OACC_CACHE, OMP_TARGET_UPDATE), 0)
 
 #define OMP_CLAUSE_SIZE(NODE)						\
   OMP_CLAUSE_OPERAND (OMP_CLAUSE_RANGE_CHECK (OMP_CLAUSE_CHECK (NODE),	\