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 3/5] OpenACC tile clause support, C/C++ front-end parts


On Thu, Nov 10, 2016 at 06:46:16PM +0800, Chung-Lin Tang wrote:
> 2016-XX-XX  Nathan Sidwell  <nathan@codesourcery.com>
> 
>         c/
>         * c-parser.c (c_parser_omp_clause_collapse): Disallow tile.
>         (c_parser_oacc_clause_tile): Disallow collapse. Fix parsing and
>         semantic checking.
>         * c-parser.c (c_parser_omp_for_loop): Accept tiling constructs.
> 
>         cp/
> 	* parser.c (cp_parser_oacc_clause_tile): Disallow collapse.  Fix
>         parsing.  Parse constant expression. Remove semantic checking.
>         (cp_parser_omp_clause_collapse): Disallow tile.
>         (cp_parser_omp_for_loop): Deal with tile clause.  Don't emit a

Similarly to the previous patch, some lines have spaces instead of tabs.

> 	parse error about missing for after already emitting one.
> 	Use more conventional for idiom for unbounded loop.
> 	* pt.c (tsubst_omp_clauses): Require integral constant expression
> 	for COLLAPSE and TILE.  Remove broken TILE subst.
> 	* semantics.c (finish_omp_clauses): Correct TILE semantic check.
> 	(finish_omp_for): Deal with tile clause.
> 
>         gcc/testsuite/
>         * c-c++-common/goacc/loop-auto-1.c: Adjust and add additional
>         case.
>         * c-c++-common/goacc/loop-auto-2.c: New.
>         * c-c++-common/goacc/tile.c: Include stdbool, fix expected errors.
>         * g++.dg/goacc/template.C: Test tile subst.  Adjust erroneous
>         uses.
> 	* g++.dg/goacc/tile-1.C: Check tile subst.
> 	* gcc.dg/goacc/loop-processing-1.c: Adjust dg-final pattern.

> +	  if (!INTEGRAL_TYPE_P (TREE_TYPE (expr))
> +	      || TREE_CODE (expr) != INTEGER_CST

No need to test for INTEGER_CST, tree_fits_shwi_p will test that.

> +	      || !tree_fits_shwi_p (expr)
> +	      || tree_to_shwi (expr) <= 0)
>  	    {
> -	      warning_at (expr_loc, 0,"%<tile%> value must be positive");
> -	      expr = integer_one_node;
> +	      error_at (expr_loc, "%<tile%> argument needs positive"
> +			" integral constant");
> +	      expr = integer_zero_node;
>  	    }
>  	}

> @@ -14713,6 +14713,7 @@ tsubst_omp_clauses (tree clauses, enum c_omp_regio
>        nc = copy_node (oc);
>        OMP_CLAUSE_CHAIN (nc) = new_clauses;
>        new_clauses = nc;
> +      bool needs_ice = false;
>  
>        switch (OMP_CLAUSE_CODE (nc))
>  	{
> @@ -14742,10 +14743,16 @@ tsubst_omp_clauses (tree clauses, enum c_omp_regio
>  	    = tsubst_omp_clause_decl (OMP_CLAUSE_DECL (oc), args, complain,
>  				      in_decl);
>  	  break;
> +	case OMP_CLAUSE_COLLAPSE:
> +	case OMP_CLAUSE_TILE:
> +	  /* These clauses really need a positive integral constant
> +	     expression, but we don't have a predicate for that
> +	     (yet).  */
> +	  needs_ice = true;
> +	  /* FALLTHRU */

As I said earlier on gcc-patches, no need to change anything for
OMP_CLAUSE_COLLAPSE, we require that the argument is a constant integer
already at parsing time, it can't be e.g. a template integral parameter.
And for OMP_CLAUSE_TILE, please avoid the needs_ice var, instead don't fall
through into the tsubst_expr and copy it over and change the argument there
instead, it is short enough.

> +		      if (TREE_CODE (t) != INTEGER_CST
> +			  || !tree_fits_shwi_p (t)

Again, no need to check for INTEGER_CST when tree_fits_shwi_p will do that.

	Jakub


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