On Fri, Oct 04, 2024 at 03:54:53PM +0100, Kwok Cheung Yeung wrote:
> +/* Returns a tree expression containing the total iteration count of the
> +   iterator clause decl T.  */

s/iterator/OpenMP iterator/

> +
> +static tree
> +compute_iterator_count (tree t, gimple_seq *pre_p)

s/_iterator/_omp_iterator/

gimplify.cc isn't OpenMP/OpenACC specific, so it is really helpful
to make it clear that some code is OpenMP specific; people without
OpenMP knowledge might not know at all what kind of iterators it talks
about.

> +/* Build loops iterating over the space defined by the iterators in clause C.

similarly, s/clause/OpenMP clause/

> +   Returns a pointer to the BIND_EXPR_BODY in the innermost loop body.
> +   LAST_BIND is set to point to the BIND_EXPR containing the whole loop.  */
> +
> +static tree *
> +build_iterator_loop (tree c, gimple_seq *pre_p, tree *last_bind)

and s/_iterator/_omp_iterator/

> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -2156,6 +2156,12 @@ class auto_suppress_location_wrappers
>  #define OMP_CLAUSE_OPERAND(NODE, I)                          \
>       OMP_CLAUSE_ELT_CHECK (NODE, I)
>  
> +/* True if the clause decl NODE contains an iterator.  */
> +#define OMP_ITERATOR_DECL_P(NODE)                            \
> +     (TREE_CODE (NODE) == TREE_LIST                          \
> +      && TREE_PURPOSE (NODE)                                 \
> +      && TREE_CODE (TREE_PURPOSE (NODE)) == TREE_VEC)

This is not how multiline macro indentation should look like.
The correct indentation is
#define OMP_ITERATOR_DECL_P(NODE) \
  (TREE_CODE (NODE) == TREE_LIST                                \
   && TREE_PURPOSE (NODE)                                       \
   && TREE_CODE (TREE_PURPOSE (NODE)) == TREE_VEC)
I know the few preceding #defines don't follow that, but IMHO we should
just fix that and not add another weird case.

The patch is ok for trunk with those nits fixed.

        Jakub

Reply via email to