This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch 3/5] OpenACC tile clause support, C/C++ front-end parts
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Chung-Lin Tang <chunglin_tang at mentor dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>, Nathan Sidwell <nathan at acm dot org>, Cesar Philippidis <cesar at codesourcery dot com>
- Date: Fri, 11 Nov 2016 11:30:38 +0100
- Subject: Re: [Patch 3/5] OpenACC tile clause support, C/C++ front-end parts
- Authentication-results: sourceware.org; auth=none
- References: <61e744b3-3ae8-1709-df7b-52d65bbfb676@mentor.com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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