This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix for PR c++/60269


On Thu, Feb 19, 2015 at 03:39:19AM +0000, Iyer, Balaji V wrote:
> Attached, please find a patch that is a fix for PR c++/60269. Tested on x86_64 and have no regression issues. Is this OK for trunk?

> +2015-02-18  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +       PR c++/60269
> +       * parser.c (cp_parser_cilk_simd_vectorlength): Added a check for
> +       template handling.  If so, then defer the validity checks to pt.c.
> +       * pt.c (tsubst_omp_clauses): Added a check for invalid vectorlength
> +       for Cilk Plus SIMD loops.

That looks wrong to me.

> +	  if (flag_cilkplus && OMP_CLAUSE_CODE (nc) == OMP_CLAUSE_SAFELEN)
> +	    {
> +	      tree new_expr = OMP_CLAUSE_OPERAND (nc, 0);
> +	      if (!new_expr || new_expr == error_mark_node)
> +		;
> +	      else if (!TREE_TYPE (new_expr) || !TREE_CONSTANT (new_expr)
> +		  || !INTEGRAL_TYPE_P (TREE_TYPE (new_expr)))
> +		error ("vectorlength must be an integer constant");
> +	      else if (TREE_CONSTANT (new_expr) 
> +		       && exact_log2 (TREE_INT_CST_LOW (new_expr)) == -1)
> +		error ("vectorlength must be a power of 2");
> +	    }   

This won't affect just #pragma simd vectorlength (x) though, but also
#pragma omp simd safelen(x).  And for the latter it is certainly
undesirable, the checking is done in finish_omp_clauses.  flag_cilkplus
can be true even if it is an OpenMP construct.

So, there are 2 ways out of this IMHO.  Either you add some special flag
on the OMP_CLAUSE_SAFELEN clause, which will tell you it is a vectorlength,
and put this diagnostics, reworked to check for type_dependent_*, into
finish_omp_clauses (then you can also move the
cp_parser_cilk_simd_vectorlength diagnostics there).
Or, if you really want to do this in tsubst_omp_clauses, you'd need some
extra argument from the caller to tell you e.g. tree_code of the tree
containing the clauses (you could use the declare_simd argument for that
turned into enum tree_code, and pass FUNCTION_DECL for declare_simd?).
But of course you need to arrange for the case where you report invalid
vectorlength for the clause to be removed from the list of clauses,
otherwise finish_omp_clauses will complain on it again and might break error
recovery.

Testcase coverage should also include testcases for where the vectorlength
clause is invalid (both the conditions you report), both outside and inside
of templates.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]