On Sun, May 01, 2016 at 08:11:24PM -0700, Cesar Philippidis wrote:
> On 04/29/2016 12:39 AM, Jakub Jelinek wrote:
>
> >> -extern tree c_finish_omp_clauses (tree, bool, bool = false, bool = false);
> >> +extern tree c_finish_omp_clauses (tree, unsigned int);
> >
> > I think it would be better to assign an enum value also for the
> > C_ORT_OMP | C_ORT_DECLARE_SIMD (C_ORT_OMP_DECLARE_SIMD), and just
> > use the enum type instead of unsigned int as the type, both in the proto
> > and in c_finish_omp_clauses definition.
>
> This patch goes back to using a enum argument for *finish_omp_clauses.
> One thing to note was that I was seeing this warning
>
> x86_64-none-linux-gnu-g++ declare-simd-1.C -fopenmp -ffat-lto-objects -S
> declare-simd-1.C:279:48: warning: ignoring large linear step
> #pragma omp declare simd simdlen (N) linear (a : sizeof (a) + sizeof
> (d) + sizeof (this) + sizeof (this->d))
>
> when the second call to finsh_omp_clauses in
>
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -9563,7 +9563,8 @@ can_complete_type_without_circularity (tree type)
> return 1;
> }
>
> -static tree tsubst_omp_clauses (tree, bool, bool, tree, tsubst_flags_t,
> tree);
> +static tree tsubst_omp_clauses (tree, enum c_omp_region_type, tree,
> + tsubst_flags_t, tree);
>
> /* Instantiate a single dependent attribute T (a TREE_LIST), and return
> either
> T or a new TREE_LIST, possibly a chain in the case of a pack
> expansion. */
> @@ -9582,10 +9583,10 @@ tsubst_attribute (tree t, tree *decl_p, tree args,
> get_attribute_name (t)))
> {
> tree clauses = TREE_VALUE (val);
> - clauses = tsubst_omp_clauses (clauses, true, false, args,
> + clauses = tsubst_omp_clauses (clauses, C_ORT_OMP_DECLARE_SIMD, args,
> complain, in_decl);
> c_omp_declare_simd_clauses_to_decls (*decl_p, clauses);
> - clauses = finish_omp_clauses (clauses, false, true);
> + clauses = finish_omp_clauses (clauses, C_ORT_OMP_DECLARE_SIMD);
> tree parms = DECL_ARGUMENTS (*decl_p);
> clauses
> = c_omp_declare_simd_clauses_to_numbers (parms, clauses);
>
> was passing C_ORT_OMP instead of C_ORT_OMP_DECLARE_SIMD. I didn't look
> into this too deeply though.
Sure, that is either C_ORT_OMP_DECLARE_SIMD or C_ORT_CILK_DECLARE_SIMD.
It would be good to differentiate between the two,
in c*_parser_omp_all_clauses it is easy to determine between the two
- Cilk+ "declare simd" aka. vector attribute will have
e.g. PRAGMA_CILK_CLAUSE_MASK bit set in the mask (or NOMASK, or
VECTORLENGTH), and both OpenMP and Cilk+ declare simd will have
PRAGMA_OMP_CLAUSE_UNIFORM in there (no other OpenMP or Cilk+ construct will).
During template instantiation it is possible to differentiate those two
by looking if the "cilk simd function" attribute is present or not.
But, guess I'm ok with your patch as is for now and the above be changed
incrementally (though, you'll need in the enum for that
C_ORT_DECLARE_SIMD = (1 << 3),
C_ORT_OMP_DECLARE_SIMD = C_ORT_DECLARE_SIMD | C_ORT_OMP,
C_ORT_CILK_DECLARE_SIMD = C_ORT_DECLARE_SIMD | C_ORT_CILK
and testing ort & C_ORT_DECLARE_SIMD instead of ort == C_ORT_OMP_DECLARE_SIMD
when you want to test for both OpenMP and Cilk+ declare simd.
Jakub
