Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic
Seems as if I missed a 'git add -u' yesterday evening + missed this when
rechecking this morning.
Now included as separate patch :-/
Unless there are comments, I intent to commit it very soon.
Namely, the actual c-parse.cc update was missing and only the updated
tests were included. In particular missing:
On 12.09.23 09:04, Tobias Burnus wrote:
+ error_at (OMP_CLAUSE_LOCATION (nl),
+"allocator variable %qD must be declared before
%qD",
+allocator, var);
...
...
I bet we can't catch everything, but perhaps e.g. doing the first
diagnostics from within walk_tree might be better.
Done now.
(Or only via the attach patch.)
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
OpenMP (C only): For 'omp allocate', really walk tree for 'alloctor' check
Walk expression tree of the 'allocator' clause of 'omp allocate' to
detect more cases where the allocator expression depends on code between
a variable declaration and its associated '#pragma omp allocate'.
This commit was supposed to be part of
r14-3863-g35f498d8dfc8e579eaba2ff2d2b96769c632fd58
OpenMP (C only): omp allocate - extend parsing support, improve diagnostic
which also contains the associated testcase changes (oops!).
gcc/c/ChangeLog:
* c-parser.cc (struct c_omp_loc_tree): New.
(c_check_omp_allocate_allocator_r): New; checking moved from ...
(c_parser_omp_allocate): ... here. Call it via walk_tree.
gcc/c/c-parser.cc | 102 --
1 file changed, 61 insertions(+), 41 deletions(-)
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 643ec02706b..b9a1b75ca43 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -19343,6 +19343,61 @@ c_parser_oacc_wait (location_t loc, c_parser *parser, char *p_name)
return stmt;
}
+struct c_omp_loc_tree
+{
+ location_t loc;
+ tree var;
+};
+
+/* Check whether the expression used in the allocator clause is declared or
+ modified between the variable declaration and its allocate directive. */
+static tree
+c_check_omp_allocate_allocator_r (tree *tp, int *, void *data)
+{
+ tree var = ((struct c_omp_loc_tree *) data)->var;
+ location_t loc = ((struct c_omp_loc_tree *) data)->loc;
+ if (TREE_CODE (*tp) == VAR_DECL && c_check_in_current_scope (*tp))
+{
+ if (linemap_location_before_p (line_table, DECL_SOURCE_LOCATION (var),
+ DECL_SOURCE_LOCATION (*tp)))
+ {
+ error_at (loc, "variable %qD used in the % clause must "
+ "be declared before %qD", *tp, var);
+ inform (DECL_SOURCE_LOCATION (*tp), "declared here");
+ inform (DECL_SOURCE_LOCATION (var),
+ "to be allocated variable declared here");
+ return *tp;
+ }
+ else
+ {
+ gcc_assert (cur_stmt_list
+ && TREE_CODE (cur_stmt_list) == STATEMENT_LIST);
+
+ tree_stmt_iterator l = tsi_last (cur_stmt_list);
+ while (!tsi_end_p (l))
+ {
+ if (linemap_location_before_p (line_table, EXPR_LOCATION (*l),
+ DECL_SOURCE_LOCATION (var)))
+ break;
+ if (TREE_CODE (*l) == MODIFY_EXPR
+ && TREE_OPERAND (*l, 0) == *tp)
+ {
+ error_at (loc,
+ "variable %qD used in the % clause "
+ "must not be modified between declaration of %qD "
+ "and its % directive", *tp, var);
+ inform (EXPR_LOCATION (*l), "modified here");
+ inform (DECL_SOURCE_LOCATION (var),
+ "to be allocated variable declared here");
+ return *tp;
+ }
+ --l;
+ }
+ }
+}
+ return NULL_TREE;
+}
+
/* OpenMP 5.x:
# pragma omp allocate (list) clauses
@@ -19465,8 +19520,8 @@ c_parser_omp_allocate (c_parser *parser)
error_at (loc, "% clause required for "
"static variable %qD", var);
else if (allocator
- && (tree_int_cst_sgn (allocator) != 1
- || tree_to_shwi (allocator) > 8))
+ && (wi::to_widest (allocator) < 1
+ || wi::to_widest (allocator) > 8))
/* 8 = largest predefined memory allocator. */
error_at (allocator_loc,
"% clause requires a predefined allocator as "
@@ -19477,46 +19532,11 @@ c_parser_omp_allocate (c_parser *parser)
"%qD not yet supported", var);
continue;
}
- if (allocator
- && TREE_CODE (allocator) == VAR_DECL
- && c_check_in_current_scope (var))
+ if (allocator)
{
- if (linemap_location_before_p (line_table, DECL_SOURCE_LOCATION (var),
- DECL_SOURCE_LOCATION (allocator)))
- {
- error_at (OMP_CLAUSE_LOCATION (nl),
- "allocator variable %qD must be declared before %qD",
- allocator, var);
- inform (DECL_SOURCE_LOCATION (allocator),
- "allocator declared here");
- inform (DECL_SOURCE_LOCATION (var), "declared here");
- }
- else
- {
- gcc_assert (cur_stmt_list
- && TREE_CODE (cur_stmt_list) ==
Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic
On Tue, Sep 12, 2023 at 09:04:16AM +0200, Tobias Burnus wrote: > Done now. What's not caught is, e.g., a value change by calling a > function which modifies its parameter: > > omp_allocator_t a = ...; int v; foo(a); #pragma omp allocate(v) allocator(a) > > as the current check is only whether 'a' is declared before 'v' or > whether 'a' is assigned to between v's declaration and the pragma. > > Any additional comments or suggestions? As I said, we can't catch all the mistakes, the unfortunate thing is that the syntax allows them. I'll try to make omp::decl attribute working soon and that will make that problem less severe when using that syntax. Jakub
Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic
Hi Jakub,
thanks for the further suggestions; updated patch attached.
On 11.09.23 15:34, Jakub Jelinek wrote:
On Mon, Sep 11, 2023 at 03:21:54PM +0200, Tobias Burnus wrote:
+ if (TREE_STATIC (var))
+{
+ if (allocator == NULL_TREE && allocator_loc == UNKNOWN_LOCATION)
+error_at (loc, "% clause required for "
+ "static variable %qD", var);
+ else if (allocator
+ && (tree_int_cst_sgn (allocator) != 1
+ || tree_to_shwi (allocator) > 8))
Has anything checked that in this case allocator is actually INTEGER_CST
which fits into shwi? Otherwise tree_to_shwi will ICE.
Consider say allocator argument of
329857234985743598347598437598347594835743895743wb
or (((unsigned __int128) 0x123456789abcdef0) << 64)
Either tree_fits_shwi_p (allocator) check would do it, or perhaps
else if (allocator
&& TREE_CODE (allocator) == INTEGER_CST
&& wi::to_widest (allocator) > 0
&& wi::to_widest (allocator) <= 8)
?
I have now used the latter. Using _BitInt and __int128 fails because the
type-check fails. I have not added them to the testsuite as not all
targets support the two. Using a casted -1 did ICE before I used to
wi::to_widest.
+ error_at (OMP_CLAUSE_LOCATION (nl),
+"allocator variable %qD must be declared before %qD",
+allocator, var);
...
+ error_at (EXPR_LOCATION (*l),
+ "allocator variable %qD, used in the "
+ "% directive for %qD, must not be "
+ "modified between declaration of %qD and its "
+ "% directive",
+ allocator, var, var);
BTW, it doesn't necessarily have to be just the simple case which you catch
here, namely that allocator is a VAR_DECL defined after var in current
scope.
...
I bet we can't catch everything, but perhaps e.g. doing the first
diagnostics from within walk_tree might be better.
Done now. What's not caught is, e.g., a value change by calling a
function which modifies its parameter:
omp_allocator_t a = ...; int v; foo(a); #pragma omp allocate(v) allocator(a)
as the current check is only whether 'a' is declared before 'v' or
whether 'a' is assigned to between v's declaration and the pragma.
Any additional comments or suggestions?
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
OpenMP (C only): omp allocate - extend parsing support, improve diagnostic
The 'allocate' directive can be used for both stack and static variables.
While the parser in C and C++ was pre-existing, it missed several
diagnostics, which this commit adds - for now only for C.
While the "sorry, unimplemented" for static variables is still issues
during parsing, the sorry for stack variables is now issued in the
middle end, preparing for the actual implementation. (Again: only for C.)
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_construct): Move call to
c_parser_omp_allocate to ...
(c_parser_pragma): ... here.
(c_parser_omp_allocate): Avoid ICE is allocator could not be
parsed; set 'omp allocate' attribute for stack/automatic variables
and only reject static variables; add several additional
restriction checks.
* c-tree.h (c_mark_decl_jump_unsafe_in_current_scope): New prototype.
* c-decl.cc (decl_jump_unsafe): Return true for omp-allocated decls.
(c_mark_decl_jump_unsafe_in_current_scope): New.
(warn_about_goto, c_check_switch_jump_warnings): Add error for
omp-allocated decls.
gcc/ChangeLog:
* gimplify.cc (gimplify_bind_expr): Check for
insertion after variable cleanup. Convert 'omp allocate'
var-decl attribute to GOMP_alloc/GOMP_free calls.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/allocate-5.c: Fix testcase; make some
dg-messages for 'sorry' as c++, only.
* c-c++-common/gomp/directive-1.c: Make a 'sorry' c++ only.
* c-c++-common/gomp/allocate-9.c: New test.
* c-c++-common/gomp/allocate-11.c: New test.
* c-c++-common/gomp/allocate-12.c: New test.
* c-c++-common/gomp/allocate-14.c: New test.
* c-c++-common/gomp/allocate-15.c: New test.
* c-c++-common/gomp/allocate-16.c: New test.
gcc/c/c-decl.cc | 26 ++
gcc/c/c-parser.cc | 115 +++---
gcc/c/c-tree.h| 1 +
gcc/gimplify.cc | 40 +
gcc/testsuite/c-c++-common/gomp/allocate-11.c | 40 +
gcc/testsuite/c-c++-common/gomp/allocate-12.c | 49 +++
gcc/testsuite/c-c++-common/gomp/allocate-14.c | 26 ++
gcc/testsuite/c-c++-common/gomp/allocate-15.c | 28 +++
gcc/testsuite/c-c++-common/gomp/allocate-16.c | 38 +
Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic)
On Mon, Sep 11, 2023 at 03:21:54PM +0200, Tobias Burnus wrote:
> + if (TREE_STATIC (var))
> + {
> + if (allocator == NULL_TREE && allocator_loc == UNKNOWN_LOCATION)
> + error_at (loc, "% clause required for "
> +"static variable %qD", var);
> + else if (allocator
> +&& (tree_int_cst_sgn (allocator) != 1
> +|| tree_to_shwi (allocator) > 8))
Has anything checked that in this case allocator is actually INTEGER_CST
which fits into shwi? Otherwise tree_to_shwi will ICE.
Consider say allocator argument of
329857234985743598347598437598347594835743895743wb
or (((unsigned __int128) 0x123456789abcdef0) << 64)
Either tree_fits_shwi_p (allocator) check would do it, or perhaps
else if (allocator
&& TREE_CODE (allocator) == INTEGER_CST
&& wi::to_widest (allocator) > 0
&& wi::to_widest (allocator) <= 8)
?
> + if (allocator
> + && TREE_CODE (allocator) == VAR_DECL
> + && c_check_in_current_scope (var))
> + {
> + if (linemap_location_before_p (line_table, DECL_SOURCE_LOCATION (var),
> + DECL_SOURCE_LOCATION (allocator)))
> + {
> + error_at (OMP_CLAUSE_LOCATION (nl),
> + "allocator variable %qD must be declared before %qD",
> + allocator, var);
> + inform (DECL_SOURCE_LOCATION (allocator),
> + "allocator declared here");
> + inform (DECL_SOURCE_LOCATION (var), "declared here");
> + }
> + else
> +{
> + gcc_assert (cur_stmt_list
> + && TREE_CODE (cur_stmt_list) == STATEMENT_LIST);
> + tree_stmt_iterator l = tsi_last (cur_stmt_list);
> + while (!tsi_end_p (l))
> +{
> + if (linemap_location_before_p (line_table, EXPR_LOCATION (*l),
> + DECL_SOURCE_LOCATION (var)))
> +break;
> + if (TREE_CODE (*l) == MODIFY_EXPR
> + && TREE_OPERAND (*l, 0) == allocator)
> +{
> + error_at (EXPR_LOCATION (*l),
> +"allocator variable %qD, used in the "
> +"% directive for %qD, must not be "
> +"modified between declaration of %qD and its "
> +"% directive",
> +allocator, var, var);
> + inform (DECL_SOURCE_LOCATION (var), "declared here");
> + inform (OMP_CLAUSE_LOCATION (nl), "used here");
> + break;
> + }
> + --l;
> + }
> +}
BTW, it doesn't necessarily have to be just the simple case which you catch
here, namely that allocator is a VAR_DECL defined after var in current
scope.
One can have an expression which uses those other vars, say
int v;
int a = 1;
int b[n]; // VLA
b[a] = 5;
#pragma omp allocate (v) allocator (foo (a, [a]))
where foo would be some function which returns omp_allocator_handle_t.
Or it could be e.g. lambda declared later, etc.
I bet we can't catch everything, but perhaps e.g. doing the first
diagnostics from within walk_tree might be better.
Jakub
Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic)
Hi,
thanks for the comments and for the line-number thing. (I wanted to
re-check it myself but then totally forgot about it.)
Attached is the updated patch, fixing the line-number comparison, the
C++ addition to the enum decl in two of the testcases, and adding
"allocator" to the one inform (both to the code and to one testcase).
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
OpenMP (C only): omp allocate - extend parsing support, improve diagnostic
The 'allocate' directive can be used for both stack and static variables.
While the parser in C and C++ was pre-existing, it missed several
diagnostics, which this commit adds - for now only for C.
While the "sorry, unimplemented" for static variables is still issues
during parsing, the sorry for stack variables is now issued in the
middle end, preparing for the actual implementation. (Again: only for C.)
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_construct): Move call to
c_parser_omp_allocate to ...
(c_parser_pragma): ... here.
(c_parser_omp_allocate): Avoid ICE is allocator could not be
parsed; set 'omp allocate' attribute for stack/automatic variables
and only reject static variables; add several additional
restriction checks.
* c-tree.h (c_mark_decl_jump_unsafe_in_current_scope): New prototype.
* c-decl.cc (decl_jump_unsafe): Return true for omp-allocated decls.
(c_mark_decl_jump_unsafe_in_current_scope): New.
(warn_about_goto, c_check_switch_jump_warnings): Add error for
omp-allocated decls.
gcc/ChangeLog:
* gimplify.cc (gimplify_bind_expr): Check for
insertion after variable cleanup. Convert 'omp allocate'
var-decl attribute to GOMP_alloc/GOMP_free calls.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/allocate-5.c: Fix testcase; make some
dg-messages for 'sorry' as c++, only.
* c-c++-common/gomp/directive-1.c: Make a 'sorry' c++ only.
* c-c++-common/gomp/allocate-9.c: New test.
* c-c++-common/gomp/allocate-11.c: New test.
* c-c++-common/gomp/allocate-12.c: New test.
* c-c++-common/gomp/allocate-14.c: New test.
* c-c++-common/gomp/allocate-15.c: New test.
gcc/c/c-decl.cc | 26 ++
gcc/c/c-parser.cc | 115 +++---
gcc/c/c-tree.h| 1 +
gcc/gimplify.cc | 40 +
gcc/testsuite/c-c++-common/gomp/allocate-11.c | 40 +
gcc/testsuite/c-c++-common/gomp/allocate-12.c | 49 +++
gcc/testsuite/c-c++-common/gomp/allocate-14.c | 26 ++
gcc/testsuite/c-c++-common/gomp/allocate-15.c | 28 +++
gcc/testsuite/c-c++-common/gomp/allocate-5.c | 60 +++---
gcc/testsuite/c-c++-common/gomp/allocate-9.c | 99 ++
gcc/testsuite/c-c++-common/gomp/directive-1.c | 2 +-
11 files changed, 444 insertions(+), 42 deletions(-)
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 649c5ae66c2..5822faf01b4 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -681,6 +681,11 @@ decl_jump_unsafe (tree decl)
if (VAR_P (decl) && C_DECL_COMPOUND_LITERAL_P (decl))
return false;
+ if (flag_openmp
+ && VAR_P (decl)
+ && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl)))
+return true;
+
/* Always warn about crossing variably modified types. */
if ((VAR_P (decl) || TREE_CODE (decl) == TYPE_DECL)
&& c_type_variably_modified_p (TREE_TYPE (decl)))
@@ -724,6 +729,15 @@ c_print_identifier (FILE *file, tree node, int indent)
c_binding_oracle = save;
}
+/* Establish that the scope contains declarations that are sensitive to
+ jumps that cross a binding. Together with decl_jump_unsafe, this is
+ used to diagnose such jumps. */
+void
+c_mark_decl_jump_unsafe_in_current_scope ()
+{
+ current_scope->has_jump_unsafe_decl = 1;
+}
+
/* Establish a binding between NAME, an IDENTIFIER_NODE, and DECL,
which may be any of several kinds of DECL or TYPE or error_mark_node,
in the scope SCOPE. */
@@ -3974,6 +3988,9 @@ warn_about_goto (location_t goto_loc, tree label, tree decl)
if (c_type_variably_modified_p (TREE_TYPE (decl)))
error_at (goto_loc,
"jump into scope of identifier with variably modified type");
+ else if (flag_openmp
+ && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl)))
+error_at (goto_loc, "jump skips OpenMP % allocation");
else
if (!warning_at (goto_loc, OPT_Wjump_misses_init,
"jump skips variable initialization"))
@@ -4253,6 +4270,15 @@ c_check_switch_jump_warnings (struct c_spot_bindings *switch_bindings,
"variably modified type"));
emitted = true;
}
+ else if (flag_openmp
+ && lookup_attribute ("omp allocate",
+ DECL_ATTRIBUTES (b->decl)))
+ {
+ saw_error = true;
+ error_at
Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic)
On Mon, 2023-09-11 at 13:54 +0200, Jakub Jelinek wrote:
> Hi!
>
> One question to David below, CCed.
>
> On Mon, Sep 11, 2023 at 01:44:07PM +0200, Tobias Burnus wrote:
[...]
>
> > +
> > + if (DECL_SOURCE_LOCATION (allocator) >
> > DECL_SOURCE_LOCATION (var))
> > + {
> > + error_at (OMP_CLAUSE_LOCATION (nl),
> > + "allocator variable %qD must be declared
> > before %qD",
> > + allocator, var);
> > + inform (DECL_SOURCE_LOCATION (allocator), "declared
> > here");
> > + inform (DECL_SOURCE_LOCATION (var), "declared here");
>
> I think this will be confusing to users when the inform is the same
> in both
> cases. I'd use "allocator declared here" in the first case.
>
> And, am really not sure if one can just simply compare location_t
> like that.
> Isn't there some function which determines what source location is
> before
> another one? David?
Indeed, the numerical ordering of location_t values doesn't fully
correspond to declaration order.
Please use
linemap_compare_locations
or
linemap_location_before_p
>
> > + if (EXPR_LOCATION (*l) < DECL_SOURCE_LOCATION
> > (var))
> > + break;
>
> Likewise.
>
Dave
>
Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic)
Hi!
One question to David below, CCed.
On Mon, Sep 11, 2023 at 01:44:07PM +0200, Tobias Burnus wrote:
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -681,6 +681,11 @@ decl_jump_unsafe (tree decl)
>if (VAR_P (decl) && C_DECL_COMPOUND_LITERAL_P (decl))
> return false;
>
> + if (flag_openmp
> + && VAR_P (decl)
> + && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl)))
> +return true;
> +
>/* Always warn about crossing variably modified types. */
>if ((VAR_P (decl) || TREE_CODE (decl) == TYPE_DECL)
>&& c_type_variably_modified_p (TREE_TYPE (decl)))
> @@ -724,6 +729,12 @@ c_print_identifier (FILE *file, tree node, int indent)
>c_binding_oracle = save;
> }
>
I think we want a function comment here.
> +void
> +c_mark_decl_jump_unsafe_in_current_scope ()
> +{
> + current_scope->has_jump_unsafe_decl = 1;
> +}
> +
> + if (DECL_SOURCE_LOCATION (allocator) > DECL_SOURCE_LOCATION (var))
> + {
> + error_at (OMP_CLAUSE_LOCATION (nl),
> + "allocator variable %qD must be declared before %qD",
> + allocator, var);
> + inform (DECL_SOURCE_LOCATION (allocator), "declared here");
> + inform (DECL_SOURCE_LOCATION (var), "declared here");
I think this will be confusing to users when the inform is the same in both
cases. I'd use "allocator declared here" in the first case.
And, am really not sure if one can just simply compare location_t like that.
Isn't there some function which determines what source location is before
another one? David?
> + if (EXPR_LOCATION (*l) < DECL_SOURCE_LOCATION (var))
> +break;
Likewise.
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/gomp/allocate-12.c
> @@ -0,0 +1,46 @@
> +/* TODO: enable for C++ once implemented. */
> +/* { dg-do compile { target c } } */
> +
> +typedef enum omp_allocator_handle_t
I think all the c-c++-common tests declaring
omp_allocator_handle_t should have
#if __cplusplus >= 201103L
: __UINTPTR_TYPE__
#endif
here (even if they are just { target c } for now, we'd certainly
forget to adjust them).
Jakub
[Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic)
The patch adds more check and fixes some minor FE bits, but it now has a
'sorry' for automatic/stack variables in the middle end.
I think it is useful by itself as it completes the C FE part and adds
diagnostic, even if the actual code generation is disabled.
Comments, remarks, suggestions?
The actual code generation works fine (see previous patch) but it fails
with OpenMP privatization and mapping; I have locally an incomplete WIP
patch to fix this. But until it works, the 'sorry' makes more sense.
Besides solving that issue, C++ needs to be updated to be on par with C
and my (not yet posted) Fortran patch also needs some changes for the C
and ME changes.
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
OpenMP (C only): omp allocate - extend parsing support, improve diagnostic
The 'allocate' directive can be used for both stack and static variables.
While the parser in C and C++ was pre-existing, it missed several
diagnostics, which this commit adds - for now only for C.
While the "sorry, unimplemented" for static variables is still issues
during parsing, the sorry for stack variables is now issued in the
middle end, preparing for the actual implementation. (Again: only for C.)
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_construct): Move call to
c_parser_omp_allocate to ...
(c_parser_pragma): ... here.
(c_parser_omp_allocate): Avoid ICE is allocator could not be
parsed; set 'omp allocate' attribute for stack/automatic variables
and only reject static variables; add several additional
restriction checks.
* c-tree.h (c_mark_decl_jump_unsafe_in_current_scope): New prototype.
* c-decl.cc (decl_jump_unsafe): Return true for omp-allocated decls.
(c_mark_decl_jump_unsafe_in_current_scope): New.
(warn_about_goto, c_check_switch_jump_warnings): Add error for
omp-allocated decls.
gcc/ChangeLog:
* gimplify.cc (gimplify_bind_expr): Check for
insertion after variable cleanup. Convert 'omp allocate'
var-decl attribute to GOMP_alloc/GOMP_free calls.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/allocate-5.c: Fix testcase; make some
dg-messages for 'sorry' as c++, only.
* c-c++-common/gomp/directive-1.c: Make a 'sorry' c++ only.
* c-c++-common/gomp/allocate-9.c: New test.
* c-c++-common/gomp/allocate-11.c: New test.
* c-c++-common/gomp/allocate-12.c: New test.
* c-c++-common/gomp/allocate-14.c: New test.
* c-c++-common/gomp/allocate-15.c: New test.
gcc/c/c-decl.cc | 23 ++
gcc/c/c-parser.cc | 112 +++---
gcc/c/c-tree.h| 1 +
gcc/gimplify.cc | 40 +
gcc/testsuite/c-c++-common/gomp/allocate-11.c | 40 +
gcc/testsuite/c-c++-common/gomp/allocate-12.c | 46 +++
gcc/testsuite/c-c++-common/gomp/allocate-14.c | 26 ++
gcc/testsuite/c-c++-common/gomp/allocate-15.c | 28 +++
gcc/testsuite/c-c++-common/gomp/allocate-5.c | 60 +++---
gcc/testsuite/c-c++-common/gomp/allocate-9.c | 96 ++
gcc/testsuite/c-c++-common/gomp/directive-1.c | 2 +-
11 files changed, 432 insertions(+), 42 deletions(-)
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 649c5ae66c2..05fdb9240ae 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -681,6 +681,11 @@ decl_jump_unsafe (tree decl)
if (VAR_P (decl) && C_DECL_COMPOUND_LITERAL_P (decl))
return false;
+ if (flag_openmp
+ && VAR_P (decl)
+ && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl)))
+return true;
+
/* Always warn about crossing variably modified types. */
if ((VAR_P (decl) || TREE_CODE (decl) == TYPE_DECL)
&& c_type_variably_modified_p (TREE_TYPE (decl)))
@@ -724,6 +729,12 @@ c_print_identifier (FILE *file, tree node, int indent)
c_binding_oracle = save;
}
+void
+c_mark_decl_jump_unsafe_in_current_scope ()
+{
+ current_scope->has_jump_unsafe_decl = 1;
+}
+
/* Establish a binding between NAME, an IDENTIFIER_NODE, and DECL,
which may be any of several kinds of DECL or TYPE or error_mark_node,
in the scope SCOPE. */
@@ -3974,6 +3985,9 @@ warn_about_goto (location_t goto_loc, tree label, tree decl)
if (c_type_variably_modified_p (TREE_TYPE (decl)))
error_at (goto_loc,
"jump into scope of identifier with variably modified type");
+ else if (flag_openmp
+ && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl)))
+error_at (goto_loc, "jump skips OpenMP % allocation");
else
if (!warning_at (goto_loc, OPT_Wjump_misses_init,
"jump skips variable initialization"))
@@ -4253,6 +4267,15 @@ c_check_switch_jump_warnings (struct c_spot_bindings *switch_bindings,
"variably modified type"));
emitted = true;
}
+
