Re: OMP_CLAUSES with clauses in operand 0

[Thomas Schwinge <thomas@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2508 bytes --]

Hi!

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...

--- gcc/tree.h
+++ gcc/tree.h
@@ -1200,7 +1200,9 @@ extern void protected_set_expr_location (tree, location_t);
 #define OMP_BODY(NODE) \
   TREE_OPERAND (TREE_RANGE_CHECK (NODE, OACC_PARALLEL, OMP_CRITICAL), 0)
 #define OMP_CLAUSES(NODE) \
-  TREE_OPERAND (TREE_RANGE_CHECK (NODE, OACC_PARALLEL, OMP_SINGLE), 1)
+  ((TREE_CODE (NODE) <= OMP_SINGLE) \
+   ? TREE_OPERAND (TREE_RANGE_CHECK (NODE, OACC_PARALLEL, OMP_SINGLE), 1) \
+   : TREE_OPERAND (TREE_RANGE_CHECK (NODE, OACC_CACHE, OMP_TARGET_UPDATE), 0))
 
 #define OACC_PARALLEL_BODY(NODE) \
   TREE_OPERAND (OACC_PARALLEL_CHECK (NODE), 0)

If that's not been obvious, I favor code readability (one generic
OMP_CLAUSES interface instead of having both OMP_CLAUSES and
OMP_CLAUSES_STANDALONE) over a tiny code size regression, but you seem to
think differently, set a different policy for code changes, which I'll
have to yield to.


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]