* [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic @ 2023-08-29 16:12 Tobias Burnus 2023-08-29 16:28 ` Jakub Jelinek 0 siblings, 1 reply; 15+ messages in thread From: Tobias Burnus @ 2023-08-29 16:12 UTC (permalink / raw) To: gcc-patches, Jakub Jelinek [-- Attachment #1: Type: text/plain, Size: 1190 bytes --] This adds support for #pragma omp allocate(var-list) [allocator(..) align(..)] While the spec permits stack and static variables, this patch only adds support for stack variables - keeping the 'sorry' for static variables. It is also only C as I wanted to get this out before updating C++ parsing. However, if disable the 'sorry' + add the attribute, it will also work for C++. For Fortran, there is no expression associated with the declaration such that it will not work with the gimplify.cc patch (it cannot find the place to add it); however, I have a mostly working version for FE generated support for stack _and_ static variables. The main RFC question for this patch is whether to generate the GOMP_alloc/free calls also for !TREE_USED() or not. This patch avoids this to aid removal of such stack variables, but one could also argue otherwise. Thoughts, comments, remarks? 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 [-- Attachment #2: omp-allocate-c.diff --] [-- Type: text/x-patch, Size: 30713 bytes --] OpenMP (C only): omp allocate - handle stack vars, 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. Additionally, it stopped with a sorry after parsing. For C only, the sorry is now restricted to static variables, the stack variable declarations are now tagged by the 'omp allocate' attribute and in gimplify_bind_expr the GOMP_alloc/GOMP_free allocation will now be added. Follow up: Add the same parser additions for C++ and update the testcases. And add Fortran support, where also parsing support exists, where also diagnostic updates are required. 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 variables and only reject stack variables; add several additional restriction checks. gcc/ChangeLog: * gimplify.cc (gimplify_bind_expr): Convert 'omp allocate' var-decl attribute to GOMP_alloc/GOMP_free calls. libgomp/ChangeLog: * libgomp.texi (Impl.Status): Mark allocate directive as 'P'. (OMP_ALLOCATOR): Fix name of ICV variable. * testsuite/libgomp.c-c++-common/allocate-4.c: New test. 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-10.c: New test. * c-c++-common/gomp/allocate-9.c: New test. gcc/c/c-parser.cc | 73 ++++++++++++++--- gcc/gimplify.cc | 82 +++++++++++++++---- gcc/testsuite/c-c++-common/gomp/allocate-10.c | 46 +++++++++++ gcc/testsuite/c-c++-common/gomp/allocate-5.c | 58 ++++++------- gcc/testsuite/c-c++-common/gomp/allocate-9.c | 94 ++++++++++++++++++++++ gcc/testsuite/c-c++-common/gomp/directive-1.c | 2 +- libgomp/libgomp.texi | 7 +- .../testsuite/libgomp.c-c++-common/allocate-4.c | 75 +++++++++++++++++ 8 files changed, 377 insertions(+), 60 deletions(-) diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index cae10ba9c80..a04fd474a28 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -1679,6 +1679,7 @@ static bool c_parser_omp_declare (c_parser *, enum pragma_context); static void c_parser_omp_requires (c_parser *); static bool c_parser_omp_error (c_parser *, enum pragma_context); static void c_parser_omp_assumption_clauses (c_parser *, bool); +static void c_parser_omp_allocate (c_parser *); static void c_parser_omp_assumes (c_parser *); static bool c_parser_omp_ordered (c_parser *, enum pragma_context, bool *); static void c_parser_oacc_routine (c_parser *, enum pragma_context); @@ -13620,6 +13621,10 @@ c_parser_pragma (c_parser *parser, enum pragma_context context, bool *if_p) c_parser_omp_requires (parser); return false; + case PRAGMA_OMP_ALLOCATE: + c_parser_omp_allocate (parser); + return false; + case PRAGMA_OMP_ASSUMES: if (context != pragma_external) { @@ -19316,10 +19321,13 @@ c_parser_oacc_wait (location_t loc, c_parser *parser, char *p_name) align (constant-expression)] */ static void -c_parser_omp_allocate (location_t loc, c_parser *parser) +c_parser_omp_allocate (c_parser *parser) { tree alignment = NULL_TREE; tree allocator = NULL_TREE; + c_parser_consume_pragma (parser); + location_t loc = c_parser_peek_token (parser)->location; + location_t allocator_loc = UNKNOWN_LOCATION; tree nl = c_parser_omp_var_list_parens (parser, OMP_CLAUSE_ALLOCATE, NULL_TREE); do { @@ -19344,7 +19352,9 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) c_expr expr = c_parser_expr_no_commas (parser, NULL); expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); expr_loc = c_parser_peek_token (parser)->location; - if (p[2] == 'i' && alignment) + if (expr.value == error_mark_node) + ; + else if (p[2] == 'i' && alignment) { error_at (expr_loc, "too many %qs clauses", "align"); break; @@ -19371,6 +19381,7 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) else { allocator = c_fully_fold (expr.value, false, NULL); + allocator_loc = expr_loc; tree orig_type = expr.original_type ? expr.original_type : TREE_TYPE (allocator); orig_type = TYPE_MAIN_VARIANT (orig_type); @@ -19390,14 +19401,53 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) } while (true); c_parser_skip_to_pragma_eol (parser); - if (allocator || alignment) - for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) - { - OMP_CLAUSE_ALLOCATE_ALLOCATOR (c) = allocator; - OMP_CLAUSE_ALLOCATE_ALIGN (c) = alignment; - } - - sorry_at (loc, "%<#pragma omp allocate%> not yet supported"); + for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) + { + tree var = OMP_CLAUSE_DECL (c); + if (TREE_CODE (var) == PARM_DECL) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "function parameter %qD may not appear as list item in an " + "%<allocate%> directive", var); + continue; + } + if (!c_check_in_current_scope (var)) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%<allocate%> directive must be in the same scope as %qD", + var); + inform (DECL_SOURCE_LOCATION (var), "declared here"); + continue; + } + if (lookup_attribute ("omp allocate", DECL_ATTRIBUTES (var))) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%qD already appeared as list item in an " + "%<allocate%> directive", var); + continue; + } + if (TREE_STATIC (var)) + { + if (allocator == NULL_TREE && allocator_loc == UNKNOWN_LOCATION) + error_at (loc, "%<allocator%> clause required for " + "static variable %qD", var); + else if (allocator + && (tree_int_cst_sgn (allocator) != 1 + || tree_to_shwi (allocator) > 8)) + /* 8 = largest predefined memory allocator. */ + error_at (allocator_loc, + "%<allocator%> clause requires a predefined allocator as " + "%qD is static", var); + else + sorry_at (OMP_CLAUSE_LOCATION (nl), + "%<#pragma omp allocate%> for static variables like " + "%qD not yet supported", var); + continue; + } + DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"), + build_tree_list (allocator, alignment), + DECL_ATTRIBUTES (var)); + } } /* OpenMP 2.5: @@ -24894,9 +24944,6 @@ c_parser_omp_construct (c_parser *parser, bool *if_p) strcpy (p_name, "#pragma wait"); stmt = c_parser_oacc_wait (loc, parser, p_name); break; - case PRAGMA_OMP_ALLOCATE: - c_parser_omp_allocate (loc, parser); - return; case PRAGMA_OMP_ATOMIC: c_parser_omp_atomic (loc, parser, false); return; diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index a49b50bc857..e48eb404bcb 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -1364,6 +1364,44 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) { struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp; + tree attr; + if (flag_openmp + && !is_global_var (t) + && DECL_CONTEXT (t) == current_function_decl + && TREE_USED (t) + && (attr = lookup_attribute ("omp allocate", DECL_ATTRIBUTES (t))) + != NULL_TREE) + { + tree v = create_tmp_var_raw (build_pointer_type (TREE_TYPE (t))); + DECL_IGNORED_P (v) = 0; + tree tmp = build_fold_indirect_ref (v); + TREE_THIS_NOTRAP (tmp) = 1; + SET_DECL_VALUE_EXPR (t, tmp); + DECL_HAS_VALUE_EXPR_P (t) = 1; + attr = TREE_VALUE (attr); + tree sz = TYPE_SIZE_UNIT (TREE_TYPE (t)); + tree alloc = (TREE_PURPOSE (attr) + ? TREE_PURPOSE (attr) + : build_zero_cst (ptr_type_node)); + tree align = (TREE_VALUE (attr) + ? TREE_VALUE (attr) : build_zero_cst (size_type_node)); + tmp = builtin_decl_explicit (BUILT_IN_GOMP_ALLOC); + tmp = build_call_expr_loc (DECL_SOURCE_LOCATION (t), tmp, 3, align, sz, alloc); + tmp = fold_build2_loc (DECL_SOURCE_LOCATION (t), MODIFY_EXPR, TREE_TYPE (v), v, + fold_convert (TREE_TYPE (v), tmp)); + gcc_assert (BIND_EXPR_BODY (bind_expr) != NULL_TREE + && TREE_CODE (BIND_EXPR_BODY (bind_expr)) == STATEMENT_LIST); + tree_stmt_iterator e = tsi_start (BIND_EXPR_BODY (bind_expr)); + while (!tsi_end_p (e)) + { + if (EXPR_LOCATION (*e) == DECL_SOURCE_LOCATION (t)) + break; + ++e; + } + gcc_assert (!tsi_end_p (e)); + tsi_link_before (&e, tmp, TSI_SAME_STMT); + } + /* Mark variable as local. */ if (ctx && ctx->region_type != ORT_NONE && !DECL_EXTERNAL (t)) { @@ -1446,22 +1484,6 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) cleanup = NULL; stack_save = NULL; - /* If the code both contains VLAs and calls alloca, then we cannot reclaim - the stack space allocated to the VLAs. */ - if (gimplify_ctxp->save_stack && !gimplify_ctxp->keep_stack) - { - gcall *stack_restore; - - /* Save stack on entry and restore it on exit. Add a try_finally - block to achieve this. */ - build_stack_save_restore (&stack_save, &stack_restore); - - gimple_set_location (stack_save, start_locus); - gimple_set_location (stack_restore, end_locus); - - gimplify_seq_add_stmt (&cleanup, stack_restore); - } - /* Add clobbers for all variables that go out of scope. */ for (t = BIND_EXPR_VARS (bind_expr); t ; t = DECL_CHAIN (t)) { @@ -1469,6 +1491,17 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) && !is_global_var (t) && DECL_CONTEXT (t) == current_function_decl) { + if (flag_openmp + && DECL_HAS_VALUE_EXPR_P (t) + && TREE_USED (t) + && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (t))) + { + tree tmp = builtin_decl_explicit (BUILT_IN_GOMP_FREE); + tmp = build_call_expr_loc (end_locus, tmp, 2, + TREE_OPERAND (DECL_VALUE_EXPR (t), 0), + build_zero_cst (ptr_type_node)); + gimplify_and_add (tmp, &cleanup); + } if (!DECL_HARD_REGISTER (t) && !TREE_THIS_VOLATILE (t) && !DECL_HAS_VALUE_EXPR_P (t) @@ -1525,6 +1558,23 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) gimplify_ctxp->live_switch_vars->remove (t); } + /* If the code both contains VLAs and calls alloca, then we cannot reclaim + the stack space allocated to the VLAs. */ + if (gimplify_ctxp->save_stack && !gimplify_ctxp->keep_stack) + { + gcall *stack_restore; + + /* Save stack on entry and restore it on exit. Add a try_finally + block to achieve this. */ + build_stack_save_restore (&stack_save, &stack_restore); + + gimple_set_location (stack_save, start_locus); + gimple_set_location (stack_restore, end_locus); + + gimplify_seq_add_stmt (&cleanup, stack_restore); + } + + if (ret_clauses) { gomp_target *stmt; diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-10.c b/gcc/testsuite/c-c++-common/gomp/allocate-10.c new file mode 100644 index 00000000000..6fe16b1eb03 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-10.c @@ -0,0 +1,46 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ +/* { dg-additional-options "-Wall -fdump-tree-gimple" } */ + +typedef enum omp_allocator_handle_t +{ + omp_default_mem_alloc = 1, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + +void +f() +{ + int n; + int A[n]; /* { dg-warning "'n' is used uninitialized" } */ + /* { dg-warning "unused variable 'A'" "" { target *-*-* } .-1 } */ +} + +void +h1() +{ + omp_allocator_handle_t my_handle; + int B1[3]; /* { dg-warning "'my_handle' is used uninitialized" } */ + /* { dg-warning "variable 'B1' set but not used" "" { target *-*-* } .-1 } */ + #pragma omp allocate(B1) allocator(my_handle) + B1[0] = 5; + /* { dg-final { scan-tree-dump-times "__builtin_GOMP_alloc" 1 "gimple" } } */ + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(0, 12, my_handle\\);" 1 "gimple" } } */ + /* { dg-final { scan-tree-dump-times "__builtin_GOMP_free \\(D.\[0-9\]+, 0B\\);" 1 "gimple" } } */ +} + +void +h2() +{ + omp_allocator_handle_t my_handle; + int B2[3]; /* { dg-warning "unused variable 'B2'" } */ + #pragma omp allocate(B2) allocator(my_handle) /* No warning as 'B2' is unused */ +} + +void +h3() +{ + omp_allocator_handle_t my_handle; + int B3[3] = {1,2,3}; /* { dg-warning "unused variable 'B3'" } */ + #pragma omp allocate(B3) allocator(my_handle) /* No warning as 'B3' is unused */ +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c b/gcc/testsuite/c-c++-common/gomp/allocate-5.c index 8a9181205f7..de1efc6832d 100644 --- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c +++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c @@ -19,59 +19,63 @@ void foo () { int a, b; + static int c; omp_allocator_handle_t my_allocator; -#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ -#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ +#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate(c) align(32) + /* { dg-message "'allocator' clause required for static variable 'c'" "" { target c } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ } void bar () { - int a, b; + int a, a2, b; omp_allocator_handle_t my_allocator; #pragma omp allocate /* { dg-error "expected '\\(' before end of line" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate allocator(my_allocator) /* { dg-error "expected '\\(' before 'allocator'" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate(a) foo(my_allocator) /* { dg-error "expected 'allocator'" } */ /* { dg-error "expected end of line before '\\(' token" "" { target *-*-* } .-1 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-2 } */ -#pragma omp allocate(a) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ +#pragma omp allocate(a2) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } void align_test () { - int i; - #pragma omp allocate(i) allocator(omp_default_mem_alloc), align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ),allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i),allocator(omp_default_mem_alloc) align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ) allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + int i1,i2,i3,i4,i5,i6; + #pragma omp allocate(i1) allocator(omp_default_mem_alloc), align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 32 ),allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3),allocator(omp_default_mem_alloc) align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i4) align ( 32 ) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ - #pragma omp allocate(i) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) + #pragma omp allocate(i5) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) /* { dg-error "too many 'allocator' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ - #pragma omp allocate(i) align ( 32 ), align(32) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ + #pragma omp allocate(i6) align ( 32 ), align(32) allocator(omp_default_mem_alloc) /* { dg-error "too many 'align' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ } void align_test2 () { - int i; + int i, i2,i3; #pragma omp allocate(i) align (32.0) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-9.c b/gcc/testsuite/c-c++-common/gomp/allocate-9.c new file mode 100644 index 00000000000..e782b48bdf4 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-9.c @@ -0,0 +1,94 @@ +typedef enum omp_allocator_handle_t +{ + omp_null_allocator = 0, + omp_default_mem_alloc = 1, + omp_large_cap_mem_alloc = 2, + omp_const_mem_alloc = 3, + omp_high_bw_mem_alloc = 4, + omp_low_lat_mem_alloc = 5, + omp_cgroup_mem_alloc = 6, + omp_pteam_mem_alloc = 7, + omp_thread_mem_alloc = 8, + __ompx_last_mem_alloc = omp_thread_mem_alloc, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + + +static int A[5] = {1,2,3,4,5}; +int B, C, D; + +/* If the following fails bacause of added predefined allocators, please update + - c/c-parser.c's c_parser_omp_allocate + - fortran/openmp.cc's is_predefined_allocator + - libgomp/env.c's parse_allocator + - libgomp/libgomp.texi (document the new values - multiple locations) + + ensure that the memory-spaces are also up to date. */ + +#pragma omp allocate(A) align(32) allocator((omp_allocator_handle_t) 9) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + + +// typo in allocator name: +#pragma omp allocate(A) allocator(omp_low_latency_mem_alloc) +/* { dg-error "'omp_low_latency_mem_alloc' undeclared here \\(not in a function\\); did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c } .-1 } */ +/* { dg-error "'omp_low_latency_mem_alloc' was not declared in this scope; did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c++ } .-2 } */ +/* { dg-error "'allocator' clause required for static variable 'A'" "" { target c } .-3 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-4 } */ + +/* align be const multiple of 2 */ +#pragma omp allocate(A) align(31) allocator(omp_default_mem_alloc) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'A' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +/* allocator missing (required as A is static) */ +#pragma omp allocate(A) align(32) /* { dg-error "'allocator' clause required for static variable 'A'" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +/* "expression in the clause must be a constant expression that evaluates to one of the + predefined memory allocator values -> omp_low_lat_mem_alloc" */ +#pragma omp allocate(B) allocator((omp_allocator_handle_t) (omp_high_bw_mem_alloc+1)) align(32) /* OK: omp_low_lat_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'B' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(C) allocator((omp_allocator_handle_t) 2) /* OK: omp_large_cap_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(A) align(32) allocator(omp_null_allocator) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +#pragma omp allocate(C) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'C' already appeared as list item in an 'allocate' directive" "" { xfail *-*-* } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +// allocate directive in same TU +int f() +{ + #pragma omp allocate(D) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'allocate' directive must be in the same scope as 'D'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } 18 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return A[0]; +} + +int g() +{ + int a2=1, b2=2; + #pragma omp allocate(a2) +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(a2) /* { dg-error "'a2' already appeared as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + { + int c2=3; + #pragma omp allocate(c2, b2) /* { dg-error "'allocate' directive must be in the same scope as 'b2'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } .-8 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return c2+a2+b2; + } +} + +int h(int q) +{ + #pragma omp allocate(q) /* { dg-error "function parameter 'q' may not appear as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + return q; +} diff --git a/gcc/testsuite/c-c++-common/gomp/directive-1.c b/gcc/testsuite/c-c++-common/gomp/directive-1.c index fc441538778..21ca319b9f9 100644 --- a/gcc/testsuite/c-c++-common/gomp/directive-1.c +++ b/gcc/testsuite/c-c++-common/gomp/directive-1.c @@ -19,7 +19,7 @@ foo (void) int i, k = 0, l = 0; #pragma omp allocate, (i) /* { dg-error "expected '\\\(' before ',' token" } */ /* { dg-error "expected end of line before ',' token" "" { target c++ } .-1 } */ - /* { dg-message "not yet supported" "" { target *-*-* } .-2 } */ + /* { dg-message "not yet supported" "" { target c++ } .-2 } */ #pragma omp critical, (bar) /* { dg-error "expected an OpenMP clause before '\\\(' token" } */ ; #pragma omp flush, (k, l) /* { dg-error "expected '\\\(' or end of line before ',' token" "" { target c } } */ diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi index 4aad8cc52f4..6dab873337e 100644 --- a/libgomp/libgomp.texi +++ b/libgomp/libgomp.texi @@ -225,7 +225,7 @@ The OpenMP 4.5 specification is fully supported. @item Predefined memory spaces, memory allocators, allocator traits @tab Y @tab See also @ref{Memory allocation} @item Memory management routines @tab Y @tab -@item @code{allocate} directive @tab N @tab +@item @code{allocate} directive @tab P @tab Only C, only stack variables @item @code{allocate} clause @tab P @tab Initial support @item @code{use_device_addr} clause on @code{target data} @tab Y @tab @item @code{ancestor} modifier on @code{device} clause @tab Y @tab @@ -296,7 +296,8 @@ The OpenMP 4.5 specification is fully supported. @item Loop transformation constructs @tab N @tab @item @code{strict} modifier in the @code{grainsize} and @code{num_tasks} clauses of the @code{taskloop} construct @tab Y @tab -@item @code{align} clause in @code{allocate} directive @tab N @tab +@item @code{align} clause in @code{allocate} directive @tab P + @tab Only C (and only stack variables) @item @code{align} modifier in @code{allocate} clause @tab Y @tab @item @code{thread_limit} clause to @code{target} construct @tab Y @tab @item @code{has_device_addr} clause to @code{target} construct @tab Y @tab @@ -2204,7 +2205,7 @@ variable is not set. @section @env{OMP_ALLOCATOR} -- Set the default allocator @cindex Environment Variable @table @asis -@item @emph{ICV:} @var{available-devices-var} +@item @emph{ICV:} @var{def-allocator-var} @item @emph{Scope:} data environment @item @emph{Description}: Sets the default allocator that is used when no allocator has been specified diff --git a/libgomp/testsuite/libgomp.c-c++-common/allocate-4.c b/libgomp/testsuite/libgomp.c-c++-common/allocate-4.c new file mode 100644 index 00000000000..ebfdc78a56b --- /dev/null +++ b/libgomp/testsuite/libgomp.c-c++-common/allocate-4.c @@ -0,0 +1,75 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do run { target c } } */ +/* { dg-additional-options "-fdump-tree-gimple" } */ + +#include <omp.h> +#include <stdint.h> + +/* { dg-final { scan-tree-dump-times "__builtin_GOMP_free \\(D.\[0-9\]+, 0B\\);" 5 "gimple" } } */ + +int one () +{ + int sum = 0; + #pragma omp allocate(sum) + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(0, 4, 0B\\);" 1 "gimple" } } */ + + /* NOTE: Initializer cannot be omp_init_allocator - as 'A' is + in the same scope and the auto-omp_free comes later than + any omp_destroy_allocator. */ + omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; + int n = 25; + int A[n]; + #pragma omp allocate(A) align(128) allocator(my_allocator) + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(128, _\[0-9\]+, my_allocator\\);" 1 "gimple" } } */ + + if (((intptr_t)A) % 128 != 0) + __builtin_abort (); + for (int i = 0; i < n; ++i) + A[i] = i; + + omp_alloctrait_t traits[1] = { { omp_atk_alignment, 64 } }; + my_allocator = omp_init_allocator(omp_low_lat_mem_space,1,traits); + { + int B[n] = { }; + int C[5] = {1,2,3,4,5}; + #pragma omp allocate(B,C) allocator(my_allocator) + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(0, _\[0-9\]+, my_allocator\\);" 1 "gimple" } } */ + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(0, 20, my_allocator\\);" 1 "gimple" } } */ + + int D[5] = {11,22,33,44,55}; + #pragma omp allocate(D) align(256) + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(256, 20, 0B\\);" 1 "gimple" } } */ + + if (((intptr_t) B) % 64 != 0) + __builtin_abort (); + if (((intptr_t) C) % 64 != 0) + __builtin_abort (); + if (((intptr_t) D) % 64 != 0) + __builtin_abort (); + + for (int i = 0; i < 5; ++i) + { + if (C[i] != i+1) + __builtin_abort (); + if (D[i] != i+1 + 10*(i+1)) + __builtin_abort (); + } + + for (int i = 0; i < n; ++i) + { + if (B[i] != 0) + __builtin_abort (); + sum += A[i]+B[i]+C[i%5]+D[i%5]; + } + } + omp_destroy_allocator (my_allocator); + return sum; +} + +int +main () +{ + if (one () != 1200) + __builtin_abort (); + return 0; +} ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic 2023-08-29 16:12 [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic Tobias Burnus @ 2023-08-29 16:28 ` Jakub Jelinek 2023-08-29 16:56 ` Tobias Burnus 0 siblings, 1 reply; 15+ messages in thread From: Jakub Jelinek @ 2023-08-29 16:28 UTC (permalink / raw) To: Tobias Burnus; +Cc: gcc-patches On Tue, Aug 29, 2023 at 06:12:58PM +0200, Tobias Burnus wrote: > This adds support for > #pragma omp allocate(var-list) [allocator(..) align(..)] > > While the spec permits stack and static variables, this patch only > adds support for stack variables - keeping the 'sorry' for static > variables. > It is also only C as I wanted to get this out before updating C++ > parsing. However, if disable the 'sorry' + add the attribute, it > will also work for C++. > > For Fortran, there is no expression associated with the declaration > such that it will not work with the gimplify.cc patch (it cannot > find the place to add it); however, I have a mostly working version > for FE generated support for stack _and_ static variables. > > The main RFC question for this patch is whether to generate the > GOMP_alloc/free calls also for !TREE_USED() or not. This patch avoids > this to aid removal of such stack variables, but one could also argue > otherwise. > > Thoughts, comments, remarks? Don't have time to look through it in detail right now, so just 2 quick comments. One thing is that for C++ one needs to be careful about vars optimized by NRV by the FE. Dunno if we can simply disregard vars mentioned in allocate directive from FE NRV, or whether we instead should ignore the allocate directive, or sorry, etc. Because NRV optimized vars are turned into RESULT_DECL and live usually in the caller's stack frame rather than current function, so they can't be heap allocated... And, just from the gimplify_bind_expr function name, I'm guessing you are adding the allocations at the start of surrounding BIND_EXPR, is that where we generally want to allocate them? Other option is on DECL_EXPR, e.g. for C++ right before they are constructed, or for C initialized if they have initializer. Though, that can have a drawback, one can e.g. in C (and I think in C++ too as long as the vars don't need any kind of construction) jump across that initialization and one would then bypass this allocation. Though, what happens with say void foo (int i) { switch (i) { int j; #pragma omp allocate (j) ... case 42: j = 5; use (&j); break; default: j = 24; use (&j); break; } } Where will j be allocated and where will j be deallocated with your patch? If it is try/finally, then perhaps break; as jump out of the BIND_EXPR will destruct it fine, but I have no idea how it would work when jumping into the switch (or goto into middle of some compound statement etc.). Jakub ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic 2023-08-29 16:28 ` Jakub Jelinek @ 2023-08-29 16:56 ` Tobias Burnus 2023-08-29 17:14 ` Jakub Jelinek 0 siblings, 1 reply; 15+ messages in thread From: Tobias Burnus @ 2023-08-29 16:56 UTC (permalink / raw) To: Jakub Jelinek, gcc-patches On 29.08.23 18:28, Jakub Jelinek wrote: > One thing is that for C++ one needs to be careful about vars optimized > by NRV by the FE. Thanks for the warning. > And, just from the gimplify_bind_expr function name, (I forgot that one change in there which makes it look larger is that I moved the stack handling below the variable cleaning as it makes sense to GOMP_free before instead of after wiping the stack via __builtin_stack_restore.) > I'm guessing you are adding the allocations at the start of surrounding BIND_EXPR, is that where > we generally want to allocate them? I actually started with this in a FE implementation, but that does not work in general. I need to put the GOMP_alloc right before the associated DECL_EXPR, otherwise, it will break for code like: omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; int n = 5; pragma omp allocate(n) allocator(my_allocator) int B[n] where my_allocator strictly needs to be (initialized) before GOMP_alloc and the 'n = 5' - but that needs in turn to happen before the 'int B[n]' to make sure 'n' is allocated and has the value 5. > Other option is on DECL_EXPR, e.g. for C++ right before they are > constructed, or for C initialized if they have initializer. Though, that > can have a drawback, one can e.g. in C (and I think in C++ too as long as > the vars don't need any kind of construction) jump across that > initialization and one would then bypass this allocation. For your example, I get (the full gimple dump is below): foo.c:7:11: warning: statement will never be executed [-Wswitch-unreachable] 7 | int j; I wonder whether we should just error out as GCC does when using a VLA at that place: error: switch jumps into scope of identifier with variably modified type (with a similar but modified msg wording.) In a sense, the 'allocate' is very similar to VLA such that we should be able to piggyback on the VLA diagnostic. Tobias PS: The gimple dump for your program: void foo (int i) { switch (i) <default: <D.2896>, case 42: <D.2895>> { int j [value-expr: *D.2899]; try { D.2899 = __builtin_GOMP_alloc (0, 4, 0B); <D.2895>: *D.2899 = 5; use (D.2899); goto <D.2897>; <D.2896>: *D.2899 = 24; use (D.2899); goto <D.2897>; } finally { __builtin_GOMP_free (D.2899, 0B); } } <D.2897>: } ----------------- 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic 2023-08-29 16:56 ` Tobias Burnus @ 2023-08-29 17:14 ` Jakub Jelinek 2023-08-30 10:47 ` Tobias Burnus 0 siblings, 1 reply; 15+ messages in thread From: Jakub Jelinek @ 2023-08-29 17:14 UTC (permalink / raw) To: Tobias Burnus; +Cc: gcc-patches On Tue, Aug 29, 2023 at 06:56:40PM +0200, Tobias Burnus wrote: > On 29.08.23 18:28, Jakub Jelinek wrote: > > One thing is that for C++ one needs to be careful about vars optimized > > by NRV by the FE. > Thanks for the warning. > > And, just from the gimplify_bind_expr function name, > > (I forgot that one change in there which makes it look larger is that > I moved the stack handling below the variable cleaning as it makes sense > to GOMP_free before instead of after wiping the stack via __builtin_stack_restore.) > > > I'm guessing you are adding the allocations at the start of surrounding BIND_EXPR, is that where > > we generally want to allocate them? > > I actually started with this in a FE implementation, but that > does not work in general. I need to put the GOMP_alloc right > before the associated DECL_EXPR, otherwise, it will break for > code like: > > omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; > int n = 5; > pragma omp allocate(n) allocator(my_allocator) What about int n = 5; omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; #pragma omp allocate(n) allocator(my_allocator) ? What we do in that case? Is that invalid because my_allocator isn't in scope when n is defined? > For your example, I get (the full gimple dump is below): > > foo.c:7:11: warning: statement will never be executed [-Wswitch-unreachable] > 7 | int j; > > I wonder whether we should just error out as GCC does when using a VLA > at that place: > > error: switch jumps into scope of identifier with variably modified type > > (with a similar but modified msg wording.) In a sense, the 'allocate' is very > similar to VLA such that we should be able to piggyback on the VLA diagnostic. Well, in this case the warning is there just because the patch chose to put it at the start of the BIND_EXPR rather than right before DECL_EXPR. For switches, there is the case of the switch jumping across declaration of an automatic var which is not initialized/constructed (I think in that case there is normally no warning/error and happens a lot in the wild including GCC sources) but perhaps one could treat those cases with #pragma omp allocate as if they are actually constructed (though, I'd still raise it at OpenMP F2F), and another case with switch which doesn't even do that. Consider switch (i) { case 42: bar (); break; case 51: int j = 5; use (&j); break; } This is valid for both C and C++, one doesn't jump across any initialization in there. Yet if the j allocation is done at the start of the BIND_EXPR, it will jump across that initialization. So, to me this feels like we should treat for the crossing initialization stuff vars mentioned in allocate directive as having a non-trivial constructor. If you add default: break; before } above, C++ will reject it with an error, but C will accept it, it is up to the user to initialize the var again in C or not use it. And, as I said earlier, this isn't just about switch, but user gotos as well. Jakub ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic 2023-08-29 17:14 ` Jakub Jelinek @ 2023-08-30 10:47 ` Tobias Burnus 2023-08-30 11:13 ` Jakub Jelinek 2023-08-30 12:43 ` Tobias Burnus 0 siblings, 2 replies; 15+ messages in thread From: Tobias Burnus @ 2023-08-30 10:47 UTC (permalink / raw) To: Jakub Jelinek, gcc-patches [-- Attachment #1: Type: text/plain, Size: 4435 bytes --] Revised patch included - addresses part of the issues: * gimplify.cc: Fix placement of GOMP_alloc by really checking for DECL_EXPR (experimented with it before but settled for a different pattern) * c/ Add it to has_jump_unsafe_decl similar to VLA + added msg to the switch/goto error handling. + new c-c++-common/gomp/allocate-11.c testcase for it But not all discussion points are solved, yet. Namely, how to diagnose: omp_allocator_handle_t uninit; int var, var2; uninit = omp_low_lat_mem_alloc; omp_allocator_handle_t late_declared = omp_low_lat_mem_alloc; #pragma omp allocate(var) allocator(uninit) #pragma omp allocate(var) allocator(late_declared) (Currently, it is only diagnosed with -Wuninitialized (or -Wextra).) Otherwise, I think all is covered, but I might have missed something. Regarding: On 29.08.23 19:14, Jakub Jelinek wrote: > What about > int n = 5; > omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; > #pragma omp allocate(n) allocator(my_allocator) > ? What we do in that case? Is that invalid because my_allocator > isn't in scope when n is defined? IMHO yes - as we can always construct cases with loops in the execution-order dependence. The current implementation handles it as VLA, i.e. warning (VLA vs. allocator): foo.c:5:3: warning: ‘m’ is used uninitialized [-Wuninitialized] 5 | int A[m]; | ^~~ foo.c:7:7: warning: ‘my_allocator’ is used uninitialized [-Wuninitialized] 7 | int n = 5; As this is more surprising than VLA, it is probably worth an error and a more explicit error. The question is how to best detect that the allocator is either declared after the to-be-omp-allocated variable declaration or that it is declared before but the assignment happens only after the declaration (either before 'omp allocate' or not). I wonder how to best check for this. During parsing, we can check for TREE_USE and whether an initializer exists, for DECL_EXPR and for c_check_in_current_scope, but this is not sufficient. We could use DECL_SOURCE_LOCATION () with '<' comparisons, but I am not sure how it plays with #line and #include - nor will it catch: int var; allocator = ... #pragma omp allocate(var) allocator(allocator) where 'allocator' is TREE_USED and declared before 'var' but still wrong. The 'is used initialized' is done deep in the middle end, but I don't think we want to add some flag_openmp always-run special case there. [Admittedly, we do not need to catch all issues and -Wuninitialized, enabled by -Wextra, will also find this usage. Still, we should try to diagnose the most common issues.] > Well, in this case the warning is there just because the patch chose to put > it at the start of the BIND_EXPR rather than right before DECL_EXPR. Granted; the way the code before the DECL_EXPR was found wasn't ideal; I now changed it back to what I previously had - where for C++, I need to handle also a cleanup_point_expr around DECL_EXPR. > For switches, there is the case of the switch jumping across declaration > of an automatic var which is not initialized/constructed (I think in that > case there is normally no warning/error and happens a lot in the wild > including GCC sources) but perhaps one could treat those cases with > #pragma omp allocate as if they are actually constructed (though, I'd still > raise it at OpenMP F2F), Can you open an OpenMP spec issue? > and another case with switch which doesn't even do > that. > Consider > switch (i) > { > case 42: > bar (); > break; > case 51: > int j = 5; > use (&j); > break; > } > This is valid for both C and C++, one doesn't jump across any initialization > in there. Yet if the j allocation is done at the start of the BIND_EXPR, it > will jump across that initialization. With the previously posted patch (and also the current one), that yields: <D.2896>: D.2900 = __builtin_GOMP_alloc (0, 4, 0B); *D.2900 = 5; which looks fine to me. 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 [-- Attachment #2: omp-allocate-c-v2.diff --] [-- Type: text/x-patch, Size: 35251 bytes --] OpenMP (C only): omp allocate - handle stack vars, 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. Additionally, it stopped with a sorry after parsing. For C only, the sorry is now restricted to static variables, the stack variable declarations are now tagged by the 'omp allocate' attribute and in gimplify_bind_expr the GOMP_alloc/GOMP_free allocation will now be added. Follow up: Add the same parser additions for C++ and update the testcases. And add Fortran support, where also parsing support exists, where also diagnostic updates are required. 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 variables and only reject stack 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): Move __builtin_stack_restore insertion after variable cleanup. Convert 'omp allocate' var-decl attribute to GOMP_alloc/GOMP_free calls. libgomp/ChangeLog: * libgomp.texi (Impl.Status): Mark allocate directive as 'P'. (OMP_ALLOCATOR): Fix name of ICV variable. * testsuite/libgomp.c-c++-common/allocate-4.c: New test. 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-10.c: New test. * c-c++-common/gomp/allocate-11.c: New test. gcc/c/c-decl.cc | 23 ++++++ gcc/c/c-parser.cc | 74 ++++++++++++++--- gcc/c/c-tree.h | 1 + gcc/gimplify.cc | 86 ++++++++++++++++---- gcc/testsuite/c-c++-common/gomp/allocate-10.c | 46 +++++++++++ gcc/testsuite/c-c++-common/gomp/allocate-11.c | 37 +++++++++ gcc/testsuite/c-c++-common/gomp/allocate-5.c | 58 ++++++------- gcc/testsuite/c-c++-common/gomp/allocate-9.c | 94 ++++++++++++++++++++++ gcc/testsuite/c-c++-common/gomp/directive-1.c | 2 +- libgomp/libgomp.texi | 7 +- .../testsuite/libgomp.c-c++-common/allocate-4.c | 75 +++++++++++++++++ 11 files changed, 443 insertions(+), 60 deletions(-) diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index 1f9eb44dbaa..0e372c4189a 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 %<allocate%> 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; } + else if (flag_openmp + && lookup_attribute ("omp allocate", + DECL_ATTRIBUTES (b->decl))) + { + saw_error = true; + error_at (case_loc, + "switch jumps over OpenMP %<allocate%> allocation"); + emitted = true; + } else emitted = warning_at (case_loc, OPT_Wjump_misses_init, diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index cae10ba9c80..dcc5de7ad93 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -1679,6 +1679,7 @@ static bool c_parser_omp_declare (c_parser *, enum pragma_context); static void c_parser_omp_requires (c_parser *); static bool c_parser_omp_error (c_parser *, enum pragma_context); static void c_parser_omp_assumption_clauses (c_parser *, bool); +static void c_parser_omp_allocate (c_parser *); static void c_parser_omp_assumes (c_parser *); static bool c_parser_omp_ordered (c_parser *, enum pragma_context, bool *); static void c_parser_oacc_routine (c_parser *, enum pragma_context); @@ -13620,6 +13621,10 @@ c_parser_pragma (c_parser *parser, enum pragma_context context, bool *if_p) c_parser_omp_requires (parser); return false; + case PRAGMA_OMP_ALLOCATE: + c_parser_omp_allocate (parser); + return false; + case PRAGMA_OMP_ASSUMES: if (context != pragma_external) { @@ -19316,10 +19321,13 @@ c_parser_oacc_wait (location_t loc, c_parser *parser, char *p_name) align (constant-expression)] */ static void -c_parser_omp_allocate (location_t loc, c_parser *parser) +c_parser_omp_allocate (c_parser *parser) { tree alignment = NULL_TREE; tree allocator = NULL_TREE; + c_parser_consume_pragma (parser); + location_t loc = c_parser_peek_token (parser)->location; + location_t allocator_loc = UNKNOWN_LOCATION; tree nl = c_parser_omp_var_list_parens (parser, OMP_CLAUSE_ALLOCATE, NULL_TREE); do { @@ -19344,7 +19352,9 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) c_expr expr = c_parser_expr_no_commas (parser, NULL); expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); expr_loc = c_parser_peek_token (parser)->location; - if (p[2] == 'i' && alignment) + if (expr.value == error_mark_node) + ; + else if (p[2] == 'i' && alignment) { error_at (expr_loc, "too many %qs clauses", "align"); break; @@ -19371,6 +19381,7 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) else { allocator = c_fully_fold (expr.value, false, NULL); + allocator_loc = expr_loc; tree orig_type = expr.original_type ? expr.original_type : TREE_TYPE (allocator); orig_type = TYPE_MAIN_VARIANT (orig_type); @@ -19390,14 +19401,54 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) } while (true); c_parser_skip_to_pragma_eol (parser); - if (allocator || alignment) - for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) - { - OMP_CLAUSE_ALLOCATE_ALLOCATOR (c) = allocator; - OMP_CLAUSE_ALLOCATE_ALIGN (c) = alignment; - } - - sorry_at (loc, "%<#pragma omp allocate%> not yet supported"); + c_mark_decl_jump_unsafe_in_current_scope (); + for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) + { + tree var = OMP_CLAUSE_DECL (c); + if (TREE_CODE (var) == PARM_DECL) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "function parameter %qD may not appear as list item in an " + "%<allocate%> directive", var); + continue; + } + if (!c_check_in_current_scope (var)) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%<allocate%> directive must be in the same scope as %qD", + var); + inform (DECL_SOURCE_LOCATION (var), "declared here"); + continue; + } + if (lookup_attribute ("omp allocate", DECL_ATTRIBUTES (var))) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%qD already appeared as list item in an " + "%<allocate%> directive", var); + continue; + } + if (TREE_STATIC (var)) + { + if (allocator == NULL_TREE && allocator_loc == UNKNOWN_LOCATION) + error_at (loc, "%<allocator%> clause required for " + "static variable %qD", var); + else if (allocator + && (tree_int_cst_sgn (allocator) != 1 + || tree_to_shwi (allocator) > 8)) + /* 8 = largest predefined memory allocator. */ + error_at (allocator_loc, + "%<allocator%> clause requires a predefined allocator as " + "%qD is static", var); + else + sorry_at (OMP_CLAUSE_LOCATION (nl), + "%<#pragma omp allocate%> for static variables like " + "%qD not yet supported", var); + continue; + } + DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"), + build_tree_list (allocator, alignment), + DECL_ATTRIBUTES (var)); + } } /* OpenMP 2.5: @@ -24894,9 +24945,6 @@ c_parser_omp_construct (c_parser *parser, bool *if_p) strcpy (p_name, "#pragma wait"); stmt = c_parser_oacc_wait (loc, parser, p_name); break; - case PRAGMA_OMP_ALLOCATE: - c_parser_omp_allocate (loc, parser); - return; case PRAGMA_OMP_ATOMIC: c_parser_omp_atomic (loc, parser, false); return; diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index 7c5234e80fd..8026e1cfc4c 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -620,6 +620,7 @@ extern unsigned int start_underspecified_init (location_t, tree); extern void finish_underspecified_init (tree, unsigned int); extern void push_scope (void); extern tree pop_scope (void); +extern void c_mark_decl_jump_unsafe_in_current_scope (); extern void c_bindings_start_stmt_expr (struct c_spot_bindings *); extern void c_bindings_end_stmt_expr (struct c_spot_bindings *); diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index a49b50bc857..18da78c31ec 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -1364,6 +1364,48 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) { struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp; + tree attr; + if (flag_openmp + && !is_global_var (t) + && DECL_CONTEXT (t) == current_function_decl + && TREE_USED (t) + && (attr = lookup_attribute ("omp allocate", DECL_ATTRIBUTES (t))) + != NULL_TREE) + { + tree v = create_tmp_var_raw (build_pointer_type (TREE_TYPE (t))); + DECL_IGNORED_P (v) = 0; + tree tmp = build_fold_indirect_ref (v); + TREE_THIS_NOTRAP (tmp) = 1; + SET_DECL_VALUE_EXPR (t, tmp); + DECL_HAS_VALUE_EXPR_P (t) = 1; + attr = TREE_VALUE (attr); + tree sz = TYPE_SIZE_UNIT (TREE_TYPE (t)); + tree alloc = (TREE_PURPOSE (attr) + ? TREE_PURPOSE (attr) + : build_zero_cst (ptr_type_node)); + tree align = (TREE_VALUE (attr) + ? TREE_VALUE (attr) : build_zero_cst (size_type_node)); + tmp = builtin_decl_explicit (BUILT_IN_GOMP_ALLOC); + tmp = build_call_expr_loc (DECL_SOURCE_LOCATION (t), tmp, 3, align, sz, alloc); + tmp = fold_build2_loc (DECL_SOURCE_LOCATION (t), MODIFY_EXPR, TREE_TYPE (v), v, + fold_convert (TREE_TYPE (v), tmp)); + gcc_assert (BIND_EXPR_BODY (bind_expr) != NULL_TREE + && TREE_CODE (BIND_EXPR_BODY (bind_expr)) == STATEMENT_LIST); + tree_stmt_iterator e = tsi_start (BIND_EXPR_BODY (bind_expr)); + while (!tsi_end_p (e)) + { + if ((TREE_CODE (*e) == DECL_EXPR + && TREE_OPERAND (*e, 0) == t) + || (TREE_CODE (*e) == CLEANUP_POINT_EXPR + && TREE_CODE (TREE_OPERAND (*e, 0)) == DECL_EXPR + && TREE_OPERAND (TREE_OPERAND (*e, 0), 0) == t)) + break; + ++e; + } + gcc_assert (!tsi_end_p (e)); + tsi_link_before (&e, tmp, TSI_SAME_STMT); + } + /* Mark variable as local. */ if (ctx && ctx->region_type != ORT_NONE && !DECL_EXTERNAL (t)) { @@ -1446,22 +1488,6 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) cleanup = NULL; stack_save = NULL; - /* If the code both contains VLAs and calls alloca, then we cannot reclaim - the stack space allocated to the VLAs. */ - if (gimplify_ctxp->save_stack && !gimplify_ctxp->keep_stack) - { - gcall *stack_restore; - - /* Save stack on entry and restore it on exit. Add a try_finally - block to achieve this. */ - build_stack_save_restore (&stack_save, &stack_restore); - - gimple_set_location (stack_save, start_locus); - gimple_set_location (stack_restore, end_locus); - - gimplify_seq_add_stmt (&cleanup, stack_restore); - } - /* Add clobbers for all variables that go out of scope. */ for (t = BIND_EXPR_VARS (bind_expr); t ; t = DECL_CHAIN (t)) { @@ -1469,6 +1495,17 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) && !is_global_var (t) && DECL_CONTEXT (t) == current_function_decl) { + if (flag_openmp + && DECL_HAS_VALUE_EXPR_P (t) + && TREE_USED (t) + && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (t))) + { + tree tmp = builtin_decl_explicit (BUILT_IN_GOMP_FREE); + tmp = build_call_expr_loc (end_locus, tmp, 2, + TREE_OPERAND (DECL_VALUE_EXPR (t), 0), + build_zero_cst (ptr_type_node)); + gimplify_and_add (tmp, &cleanup); + } if (!DECL_HARD_REGISTER (t) && !TREE_THIS_VOLATILE (t) && !DECL_HAS_VALUE_EXPR_P (t) @@ -1525,6 +1562,23 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) gimplify_ctxp->live_switch_vars->remove (t); } + /* If the code both contains VLAs and calls alloca, then we cannot reclaim + the stack space allocated to the VLAs. */ + if (gimplify_ctxp->save_stack && !gimplify_ctxp->keep_stack) + { + gcall *stack_restore; + + /* Save stack on entry and restore it on exit. Add a try_finally + block to achieve this. */ + build_stack_save_restore (&stack_save, &stack_restore); + + gimple_set_location (stack_save, start_locus); + gimple_set_location (stack_restore, end_locus); + + gimplify_seq_add_stmt (&cleanup, stack_restore); + } + + if (ret_clauses) { gomp_target *stmt; diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-10.c b/gcc/testsuite/c-c++-common/gomp/allocate-10.c new file mode 100644 index 00000000000..6fe16b1eb03 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-10.c @@ -0,0 +1,46 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ +/* { dg-additional-options "-Wall -fdump-tree-gimple" } */ + +typedef enum omp_allocator_handle_t +{ + omp_default_mem_alloc = 1, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + +void +f() +{ + int n; + int A[n]; /* { dg-warning "'n' is used uninitialized" } */ + /* { dg-warning "unused variable 'A'" "" { target *-*-* } .-1 } */ +} + +void +h1() +{ + omp_allocator_handle_t my_handle; + int B1[3]; /* { dg-warning "'my_handle' is used uninitialized" } */ + /* { dg-warning "variable 'B1' set but not used" "" { target *-*-* } .-1 } */ + #pragma omp allocate(B1) allocator(my_handle) + B1[0] = 5; + /* { dg-final { scan-tree-dump-times "__builtin_GOMP_alloc" 1 "gimple" } } */ + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(0, 12, my_handle\\);" 1 "gimple" } } */ + /* { dg-final { scan-tree-dump-times "__builtin_GOMP_free \\(D.\[0-9\]+, 0B\\);" 1 "gimple" } } */ +} + +void +h2() +{ + omp_allocator_handle_t my_handle; + int B2[3]; /* { dg-warning "unused variable 'B2'" } */ + #pragma omp allocate(B2) allocator(my_handle) /* No warning as 'B2' is unused */ +} + +void +h3() +{ + omp_allocator_handle_t my_handle; + int B3[3] = {1,2,3}; /* { dg-warning "unused variable 'B3'" } */ + #pragma omp allocate(B3) allocator(my_handle) /* No warning as 'B3' is unused */ +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-11.c b/gcc/testsuite/c-c++-common/gomp/allocate-11.c new file mode 100644 index 00000000000..c052da488e3 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-11.c @@ -0,0 +1,37 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +void bar(); +void use (int*); + +void +f (int i) +{ + switch (i) /* { dg-note "switch starts here" } */ + { + int j; /* { dg-note "'j' declared here" } */ + /* { dg-warning "statement will never be executed \\\[-Wswitch-unreachable\\\]" "" { target *-*-* } .-1 } */ + #pragma omp allocate(j) + case 42: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" } */ + bar (); + break; + case 51: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" } */ + use (&j); + break; + } +} + +int +h (int i2) +{ + if (i2 == 5) + goto label; /* { dg-error "jump skips OpenMP 'allocate' allocation" } */ + return 5; + + int k2; /* { dg-note "'k2' declared here" } */ + int j2 = 4; /* { dg-note "'j2' declared here" } */ + #pragma omp allocate(k2, j2) +label: /* { dg-note "label 'label' defined here" } */ + k2 = 4; + return j2 + k2; +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c b/gcc/testsuite/c-c++-common/gomp/allocate-5.c index 8a9181205f7..de1efc6832d 100644 --- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c +++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c @@ -19,59 +19,63 @@ void foo () { int a, b; + static int c; omp_allocator_handle_t my_allocator; -#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ -#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ +#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate(c) align(32) + /* { dg-message "'allocator' clause required for static variable 'c'" "" { target c } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ } void bar () { - int a, b; + int a, a2, b; omp_allocator_handle_t my_allocator; #pragma omp allocate /* { dg-error "expected '\\(' before end of line" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate allocator(my_allocator) /* { dg-error "expected '\\(' before 'allocator'" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate(a) foo(my_allocator) /* { dg-error "expected 'allocator'" } */ /* { dg-error "expected end of line before '\\(' token" "" { target *-*-* } .-1 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-2 } */ -#pragma omp allocate(a) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ +#pragma omp allocate(a2) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } void align_test () { - int i; - #pragma omp allocate(i) allocator(omp_default_mem_alloc), align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ),allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i),allocator(omp_default_mem_alloc) align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ) allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + int i1,i2,i3,i4,i5,i6; + #pragma omp allocate(i1) allocator(omp_default_mem_alloc), align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 32 ),allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3),allocator(omp_default_mem_alloc) align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i4) align ( 32 ) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ - #pragma omp allocate(i) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) + #pragma omp allocate(i5) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) /* { dg-error "too many 'allocator' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ - #pragma omp allocate(i) align ( 32 ), align(32) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ + #pragma omp allocate(i6) align ( 32 ), align(32) allocator(omp_default_mem_alloc) /* { dg-error "too many 'align' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ } void align_test2 () { - int i; + int i, i2,i3; #pragma omp allocate(i) align (32.0) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-9.c b/gcc/testsuite/c-c++-common/gomp/allocate-9.c new file mode 100644 index 00000000000..e782b48bdf4 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-9.c @@ -0,0 +1,94 @@ +typedef enum omp_allocator_handle_t +{ + omp_null_allocator = 0, + omp_default_mem_alloc = 1, + omp_large_cap_mem_alloc = 2, + omp_const_mem_alloc = 3, + omp_high_bw_mem_alloc = 4, + omp_low_lat_mem_alloc = 5, + omp_cgroup_mem_alloc = 6, + omp_pteam_mem_alloc = 7, + omp_thread_mem_alloc = 8, + __ompx_last_mem_alloc = omp_thread_mem_alloc, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + + +static int A[5] = {1,2,3,4,5}; +int B, C, D; + +/* If the following fails bacause of added predefined allocators, please update + - c/c-parser.c's c_parser_omp_allocate + - fortran/openmp.cc's is_predefined_allocator + - libgomp/env.c's parse_allocator + - libgomp/libgomp.texi (document the new values - multiple locations) + + ensure that the memory-spaces are also up to date. */ + +#pragma omp allocate(A) align(32) allocator((omp_allocator_handle_t) 9) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + + +// typo in allocator name: +#pragma omp allocate(A) allocator(omp_low_latency_mem_alloc) +/* { dg-error "'omp_low_latency_mem_alloc' undeclared here \\(not in a function\\); did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c } .-1 } */ +/* { dg-error "'omp_low_latency_mem_alloc' was not declared in this scope; did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c++ } .-2 } */ +/* { dg-error "'allocator' clause required for static variable 'A'" "" { target c } .-3 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-4 } */ + +/* align be const multiple of 2 */ +#pragma omp allocate(A) align(31) allocator(omp_default_mem_alloc) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'A' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +/* allocator missing (required as A is static) */ +#pragma omp allocate(A) align(32) /* { dg-error "'allocator' clause required for static variable 'A'" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +/* "expression in the clause must be a constant expression that evaluates to one of the + predefined memory allocator values -> omp_low_lat_mem_alloc" */ +#pragma omp allocate(B) allocator((omp_allocator_handle_t) (omp_high_bw_mem_alloc+1)) align(32) /* OK: omp_low_lat_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'B' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(C) allocator((omp_allocator_handle_t) 2) /* OK: omp_large_cap_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(A) align(32) allocator(omp_null_allocator) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +#pragma omp allocate(C) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'C' already appeared as list item in an 'allocate' directive" "" { xfail *-*-* } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +// allocate directive in same TU +int f() +{ + #pragma omp allocate(D) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'allocate' directive must be in the same scope as 'D'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } 18 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return A[0]; +} + +int g() +{ + int a2=1, b2=2; + #pragma omp allocate(a2) +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(a2) /* { dg-error "'a2' already appeared as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + { + int c2=3; + #pragma omp allocate(c2, b2) /* { dg-error "'allocate' directive must be in the same scope as 'b2'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } .-8 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return c2+a2+b2; + } +} + +int h(int q) +{ + #pragma omp allocate(q) /* { dg-error "function parameter 'q' may not appear as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + return q; +} diff --git a/gcc/testsuite/c-c++-common/gomp/directive-1.c b/gcc/testsuite/c-c++-common/gomp/directive-1.c index fc441538778..21ca319b9f9 100644 --- a/gcc/testsuite/c-c++-common/gomp/directive-1.c +++ b/gcc/testsuite/c-c++-common/gomp/directive-1.c @@ -19,7 +19,7 @@ foo (void) int i, k = 0, l = 0; #pragma omp allocate, (i) /* { dg-error "expected '\\\(' before ',' token" } */ /* { dg-error "expected end of line before ',' token" "" { target c++ } .-1 } */ - /* { dg-message "not yet supported" "" { target *-*-* } .-2 } */ + /* { dg-message "not yet supported" "" { target c++ } .-2 } */ #pragma omp critical, (bar) /* { dg-error "expected an OpenMP clause before '\\\(' token" } */ ; #pragma omp flush, (k, l) /* { dg-error "expected '\\\(' or end of line before ',' token" "" { target c } } */ diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi index 4aad8cc52f4..6dab873337e 100644 --- a/libgomp/libgomp.texi +++ b/libgomp/libgomp.texi @@ -225,7 +225,7 @@ The OpenMP 4.5 specification is fully supported. @item Predefined memory spaces, memory allocators, allocator traits @tab Y @tab See also @ref{Memory allocation} @item Memory management routines @tab Y @tab -@item @code{allocate} directive @tab N @tab +@item @code{allocate} directive @tab P @tab Only C, only stack variables @item @code{allocate} clause @tab P @tab Initial support @item @code{use_device_addr} clause on @code{target data} @tab Y @tab @item @code{ancestor} modifier on @code{device} clause @tab Y @tab @@ -296,7 +296,8 @@ The OpenMP 4.5 specification is fully supported. @item Loop transformation constructs @tab N @tab @item @code{strict} modifier in the @code{grainsize} and @code{num_tasks} clauses of the @code{taskloop} construct @tab Y @tab -@item @code{align} clause in @code{allocate} directive @tab N @tab +@item @code{align} clause in @code{allocate} directive @tab P + @tab Only C (and only stack variables) @item @code{align} modifier in @code{allocate} clause @tab Y @tab @item @code{thread_limit} clause to @code{target} construct @tab Y @tab @item @code{has_device_addr} clause to @code{target} construct @tab Y @tab @@ -2204,7 +2205,7 @@ variable is not set. @section @env{OMP_ALLOCATOR} -- Set the default allocator @cindex Environment Variable @table @asis -@item @emph{ICV:} @var{available-devices-var} +@item @emph{ICV:} @var{def-allocator-var} @item @emph{Scope:} data environment @item @emph{Description}: Sets the default allocator that is used when no allocator has been specified diff --git a/libgomp/testsuite/libgomp.c-c++-common/allocate-4.c b/libgomp/testsuite/libgomp.c-c++-common/allocate-4.c new file mode 100644 index 00000000000..ebfdc78a56b --- /dev/null +++ b/libgomp/testsuite/libgomp.c-c++-common/allocate-4.c @@ -0,0 +1,75 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do run { target c } } */ +/* { dg-additional-options "-fdump-tree-gimple" } */ + +#include <omp.h> +#include <stdint.h> + +/* { dg-final { scan-tree-dump-times "__builtin_GOMP_free \\(D.\[0-9\]+, 0B\\);" 5 "gimple" } } */ + +int one () +{ + int sum = 0; + #pragma omp allocate(sum) + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(0, 4, 0B\\);" 1 "gimple" } } */ + + /* NOTE: Initializer cannot be omp_init_allocator - as 'A' is + in the same scope and the auto-omp_free comes later than + any omp_destroy_allocator. */ + omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; + int n = 25; + int A[n]; + #pragma omp allocate(A) align(128) allocator(my_allocator) + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(128, _\[0-9\]+, my_allocator\\);" 1 "gimple" } } */ + + if (((intptr_t)A) % 128 != 0) + __builtin_abort (); + for (int i = 0; i < n; ++i) + A[i] = i; + + omp_alloctrait_t traits[1] = { { omp_atk_alignment, 64 } }; + my_allocator = omp_init_allocator(omp_low_lat_mem_space,1,traits); + { + int B[n] = { }; + int C[5] = {1,2,3,4,5}; + #pragma omp allocate(B,C) allocator(my_allocator) + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(0, _\[0-9\]+, my_allocator\\);" 1 "gimple" } } */ + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(0, 20, my_allocator\\);" 1 "gimple" } } */ + + int D[5] = {11,22,33,44,55}; + #pragma omp allocate(D) align(256) + /* { dg-final { scan-tree-dump-times "D.\[0-9\]+ = __builtin_GOMP_alloc \\(256, 20, 0B\\);" 1 "gimple" } } */ + + if (((intptr_t) B) % 64 != 0) + __builtin_abort (); + if (((intptr_t) C) % 64 != 0) + __builtin_abort (); + if (((intptr_t) D) % 64 != 0) + __builtin_abort (); + + for (int i = 0; i < 5; ++i) + { + if (C[i] != i+1) + __builtin_abort (); + if (D[i] != i+1 + 10*(i+1)) + __builtin_abort (); + } + + for (int i = 0; i < n; ++i) + { + if (B[i] != 0) + __builtin_abort (); + sum += A[i]+B[i]+C[i%5]+D[i%5]; + } + } + omp_destroy_allocator (my_allocator); + return sum; +} + +int +main () +{ + if (one () != 1200) + __builtin_abort (); + return 0; +} ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic 2023-08-30 10:47 ` Tobias Burnus @ 2023-08-30 11:13 ` Jakub Jelinek 2023-08-30 12:43 ` Tobias Burnus 1 sibling, 0 replies; 15+ messages in thread From: Jakub Jelinek @ 2023-08-30 11:13 UTC (permalink / raw) To: Tobias Burnus; +Cc: gcc-patches On Wed, Aug 30, 2023 at 12:47:42PM +0200, Tobias Burnus wrote: > > For switches, there is the case of the switch jumping across declaration > > of an automatic var which is not initialized/constructed (I think in that > > case there is normally no warning/error and happens a lot in the wild > > including GCC sources) but perhaps one could treat those cases with > > #pragma omp allocate as if they are actually constructed (though, I'd still > > raise it at OpenMP F2F), > Can you open an OpenMP spec issue? https://github.com/OpenMP/spec/issues/3676 Feel freeo to amend it. Jakub ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic 2023-08-30 10:47 ` Tobias Burnus 2023-08-30 11:13 ` Jakub Jelinek @ 2023-08-30 12:43 ` Tobias Burnus 2023-09-11 11:44 ` [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic) Tobias Burnus 1 sibling, 1 reply; 15+ messages in thread From: Tobias Burnus @ 2023-08-30 12:43 UTC (permalink / raw) To: Jakub Jelinek, gcc-patches [-- Attachment #1: Type: text/plain, Size: 800 bytes --] Attached is an incremental patch to add diagnostic for the in-between allocator issues, i.e. On 30.08.23 12:47, Tobias Burnus wrote: > omp_allocator_handle_t uninit; > int var, var2; > uninit = omp_low_lat_mem_alloc; > omp_allocator_handle_t late_declared = omp_low_lat_mem_alloc; > #pragma omp allocate(var) allocator(uninit) > #pragma omp allocate(var) allocator(late_declared) Further comments, remarks and suggestions to this patch - or the base patch (v2 in previous email) are highly welcome. 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 [-- Attachment #2: omp-allocate-c-diag.diff --] [-- Type: text/x-patch, Size: 4323 bytes --] OpenMP (C only): omp allocate - improve diagnostic regarding the allocator gcc/c/ChangeLog: * c-parser.cc (c_parser_omp_allocate): Diagnose when allocator is declared or modified between the declaration of a list item and the 'omp allocate' directive. gcc/testsuite/ChangeLog: * c-c++-common/gomp/allocate-5.c: Fix testcase. * c-c++-common/gomp/allocate-12.c: New test. diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index dcc5de7ad93..dfef61da082 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -19445,6 +19445,44 @@ 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 (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"); + } + 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 (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 " + "%<allocate%> directive for %qD, must not be " + "modified between declaration of %qD and its " + "%<allocate%> directive", + allocator, var, var); + inform (DECL_SOURCE_LOCATION (var), "declared here"); + inform (OMP_CLAUSE_LOCATION (nl), "used here"); + break; + } + --l; + } + } + } DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"), build_tree_list (allocator, alignment), DECL_ATTRIBUTES (var)); diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c b/gcc/testsuite/c-c++-common/gomp/allocate-5.c index de1efc6832d..2ca4786264f 100644 --- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c +++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c @@ -18,9 +18,9 @@ typedef enum omp_allocator_handle_t void foo () { + omp_allocator_handle_t my_allocator = omp_default_mem_alloc; int a, b; static int c; - omp_allocator_handle_t my_allocator; #pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ #pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ #pragma omp allocate(c) align(32) diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-12.c b/gcc/testsuite/c-c++-common/gomp/allocate-12.c new file mode 100644 index 00000000000..38836ef5089 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-12.c @@ -0,0 +1,43 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +typedef enum omp_allocator_handle_t +{ + omp_default_mem_alloc = 1, + omp_low_lat_mem_alloc = 5, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + +int +f () +{ + omp_allocator_handle_t my_allocator; + int n = 5; /* { dg-note "declared here" } */ + my_allocator = omp_default_mem_alloc; /* { dg-error "allocator variable 'my_allocator' must not be modified between declaration of 'n' and its 'allocate' directive" } */ + #pragma omp allocate(n) allocator(my_allocator) /* { dg-note "used here" } */ + n = 7; + return n; +} + + +int +g () +{ + int n = 5; /* { dg-note "declared here" } */ + omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; /* { dg-note "declared here" } */ + #pragma omp allocate(n) allocator(my_allocator) /* { dg-error "allocator variable 'my_allocator' must be declared before 'n'" } */ + n = 7; + return n; +} + +int +h () +{ + /* my_allocator uninitialized - but only diagnosed in the ME with -Wuninitialized; + see gomp/allocate-10.c. */ + omp_allocator_handle_t my_allocator; + int n = 5; + #pragma omp allocate(n) allocator(my_allocator) + n = 7; + return n; +} ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic) 2023-08-30 12:43 ` Tobias Burnus @ 2023-09-11 11:44 ` Tobias Burnus 2023-09-11 11:54 ` Jakub Jelinek 0 siblings, 1 reply; 15+ messages in thread From: Tobias Burnus @ 2023-09-11 11:44 UTC (permalink / raw) To: Jakub Jelinek, gcc-patches [-- Attachment #1: Type: text/plain, Size: 962 bytes --] 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 [-- Attachment #2: omp-allocate-c-v3.diff --] [-- Type: text/x-patch, Size: 31937 bytes --] 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 %<allocate%> 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; } + else if (flag_openmp + && lookup_attribute ("omp allocate", + DECL_ATTRIBUTES (b->decl))) + { + saw_error = true; + error_at (case_loc, + "switch jumps over OpenMP %<allocate%> allocation"); + emitted = true; + } else emitted = warning_at (case_loc, OPT_Wjump_misses_init, diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index c04962d67a4..fb4e69fffb7 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -1681,6 +1681,7 @@ static bool c_parser_omp_declare (c_parser *, enum pragma_context); static void c_parser_omp_requires (c_parser *); static bool c_parser_omp_error (c_parser *, enum pragma_context); static void c_parser_omp_assumption_clauses (c_parser *, bool); +static void c_parser_omp_allocate (c_parser *); static void c_parser_omp_assumes (c_parser *); static bool c_parser_omp_ordered (c_parser *, enum pragma_context, bool *); static void c_parser_oacc_routine (c_parser *, enum pragma_context); @@ -13649,6 +13650,10 @@ c_parser_pragma (c_parser *parser, enum pragma_context context, bool *if_p) c_parser_omp_requires (parser); return false; + case PRAGMA_OMP_ALLOCATE: + c_parser_omp_allocate (parser); + return false; + case PRAGMA_OMP_ASSUMES: if (context != pragma_external) { @@ -19348,10 +19353,13 @@ c_parser_oacc_wait (location_t loc, c_parser *parser, char *p_name) align (constant-expression)] */ static void -c_parser_omp_allocate (location_t loc, c_parser *parser) +c_parser_omp_allocate (c_parser *parser) { tree alignment = NULL_TREE; tree allocator = NULL_TREE; + c_parser_consume_pragma (parser); + location_t loc = c_parser_peek_token (parser)->location; + location_t allocator_loc = UNKNOWN_LOCATION; tree nl = c_parser_omp_var_list_parens (parser, OMP_CLAUSE_ALLOCATE, NULL_TREE); do { @@ -19376,7 +19384,9 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) c_expr expr = c_parser_expr_no_commas (parser, NULL); expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); expr_loc = c_parser_peek_token (parser)->location; - if (p[2] == 'i' && alignment) + if (expr.value == error_mark_node) + ; + else if (p[2] == 'i' && alignment) { error_at (expr_loc, "too many %qs clauses", "align"); break; @@ -19403,6 +19413,7 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) else { allocator = c_fully_fold (expr.value, false, NULL); + allocator_loc = expr_loc; tree orig_type = expr.original_type ? expr.original_type : TREE_TYPE (allocator); orig_type = TYPE_MAIN_VARIANT (orig_type); @@ -19422,14 +19433,92 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) } while (true); c_parser_skip_to_pragma_eol (parser); - if (allocator || alignment) - for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) - { - OMP_CLAUSE_ALLOCATE_ALLOCATOR (c) = allocator; - OMP_CLAUSE_ALLOCATE_ALIGN (c) = alignment; - } - - sorry_at (loc, "%<#pragma omp allocate%> not yet supported"); + c_mark_decl_jump_unsafe_in_current_scope (); + for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) + { + tree var = OMP_CLAUSE_DECL (c); + if (TREE_CODE (var) == PARM_DECL) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "function parameter %qD may not appear as list item in an " + "%<allocate%> directive", var); + continue; + } + if (!c_check_in_current_scope (var)) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%<allocate%> directive must be in the same scope as %qD", + var); + inform (DECL_SOURCE_LOCATION (var), "declared here"); + continue; + } + if (lookup_attribute ("omp allocate", DECL_ATTRIBUTES (var))) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%qD already appeared as list item in an " + "%<allocate%> directive", var); + continue; + } + if (TREE_STATIC (var)) + { + if (allocator == NULL_TREE && allocator_loc == UNKNOWN_LOCATION) + error_at (loc, "%<allocator%> clause required for " + "static variable %qD", var); + else if (allocator + && (tree_int_cst_sgn (allocator) != 1 + || tree_to_shwi (allocator) > 8)) + /* 8 = largest predefined memory allocator. */ + error_at (allocator_loc, + "%<allocator%> clause requires a predefined allocator as " + "%qD is static", var); + else + sorry_at (OMP_CLAUSE_LOCATION (nl), + "%<#pragma omp allocate%> for static variables like " + "%qD not yet supported", var); + continue; + } + if (allocator + && TREE_CODE (allocator) == VAR_DECL + && c_check_in_current_scope (var)) + { + 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"); + } + 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 (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 " + "%<allocate%> directive for %qD, must not be " + "modified between declaration of %qD and its " + "%<allocate%> directive", + allocator, var, var); + inform (DECL_SOURCE_LOCATION (var), "declared here"); + inform (OMP_CLAUSE_LOCATION (nl), "used here"); + break; + } + --l; + } + } + } + DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"), + build_tree_list (allocator, alignment), + DECL_ATTRIBUTES (var)); + } } /* OpenMP 2.5: @@ -24926,9 +25015,6 @@ c_parser_omp_construct (c_parser *parser, bool *if_p) strcpy (p_name, "#pragma wait"); stmt = c_parser_oacc_wait (loc, parser, p_name); break; - case PRAGMA_OMP_ALLOCATE: - c_parser_omp_allocate (loc, parser); - return; case PRAGMA_OMP_ATOMIC: c_parser_omp_atomic (loc, parser, false); return; diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index f928137c8d4..2664354337b 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -626,6 +626,7 @@ extern unsigned int start_underspecified_init (location_t, tree); extern void finish_underspecified_init (tree, unsigned int); extern void push_scope (void); extern tree pop_scope (void); +extern void c_mark_decl_jump_unsafe_in_current_scope (); extern void c_bindings_start_stmt_expr (struct c_spot_bindings *); extern void c_bindings_end_stmt_expr (struct c_spot_bindings *); diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index a49b50bc857..a0e8cc2199d 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -1363,6 +1363,46 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) if (VAR_P (t)) { struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp; + tree attr; + + if (flag_openmp + && !is_global_var (t) + && DECL_CONTEXT (t) == current_function_decl + && TREE_USED (t) + && (attr = lookup_attribute ("omp allocate", DECL_ATTRIBUTES (t))) + != NULL_TREE) + { + tree alloc = TREE_PURPOSE (TREE_VALUE (attr)); + tree align = TREE_VALUE (TREE_VALUE (attr)); + /* Allocate directives that appear in a target region must specify + an allocator clause unless a requires directive with the + dynamic_allocators clause is present in the same compilation + unit. */ + bool missing_dyn_alloc = false; + if (alloc == NULL_TREE + && ((omp_requires_mask & OMP_REQUIRES_DYNAMIC_ALLOCATORS) + == 0)) + { + /* This comes too early for omp_discover_declare_target..., + but should at least catch the most common cases. */ + missing_dyn_alloc + = cgraph_node::get (current_function_decl)->offloadable; + for (struct gimplify_omp_ctx *ctx2 = ctx; + ctx2 && !missing_dyn_alloc; ctx2 = ctx2->outer_context) + if (ctx2->code == OMP_TARGET) + missing_dyn_alloc = true; + } + if (missing_dyn_alloc) + error_at (DECL_SOURCE_LOCATION (t), + "%<allocate%> directive for %qD inside a target " + "region must specify an %<allocator%> clause", t); + else if (align != NULL_TREE + || alloc == NULL_TREE + || !integer_onep (alloc)) + sorry_at (DECL_SOURCE_LOCATION (t), + "OpenMP %<allocate%> directive, used for %qD, not " + "yet supported", t); + } /* Mark variable as local. */ if (ctx && ctx->region_type != ORT_NONE && !DECL_EXTERNAL (t)) diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-11.c b/gcc/testsuite/c-c++-common/gomp/allocate-11.c new file mode 100644 index 00000000000..f9ad50abb7f --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-11.c @@ -0,0 +1,40 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +void bar(); +void use (int*); + +void +f (int i) +{ + switch (i) /* { dg-note "switch starts here" } */ + { + int j; /* { dg-note "'j' declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + #pragma omp allocate(j) + case 42: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" } */ + bar (); + /* { dg-warning "statement will never be executed \\\[-Wswitch-unreachable\\\]" "" { target *-*-* } .-1 } */ + break; + case 51: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" } */ + use (&j); + break; + } +} + +int +h (int i2) +{ + if (i2 == 5) + goto label; /* { dg-error "jump skips OpenMP 'allocate' allocation" } */ + return 5; + + int k2; /* { dg-note "'k2' declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + int j2 = 4; /* { dg-note "'j2' declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + #pragma omp allocate(k2, j2) +label: /* { dg-note "label 'label' defined here" } */ + k2 = 4; + return j2 + k2; +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-12.c b/gcc/testsuite/c-c++-common/gomp/allocate-12.c new file mode 100644 index 00000000000..aeb38547ba5 --- /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 +{ + omp_default_mem_alloc = 1, + omp_low_lat_mem_alloc = 5, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + +int +f () +{ + omp_allocator_handle_t my_allocator; + int n = 5; /* { dg-note "declared here" } */ + my_allocator = omp_default_mem_alloc; /* { dg-error "allocator variable 'my_allocator', used in the 'allocate' directive for 'n', must not be modified between declaration of 'n' and its 'allocate' directive" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-2 } */ + #pragma omp allocate(n) allocator(my_allocator) /* { dg-note "used here" } */ + n = 7; + return n; +} + + +int +g () +{ + int n = 5; /* { dg-note "declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; /* { dg-note "declared here" } */ + #pragma omp allocate(n) allocator(my_allocator) /* { dg-error "allocator variable 'my_allocator' must be declared before 'n'" } */ + n = 7; + return n; +} + +int +h () +{ + /* my_allocator uninitialized - but only diagnosed in the ME with -Wuninitialized; + see gomp/allocate-10.c. */ + omp_allocator_handle_t my_allocator; + int n = 5; + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + #pragma omp allocate(n) allocator(my_allocator) + n = 7; + return n; +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-14.c b/gcc/testsuite/c-c++-common/gomp/allocate-14.c new file mode 100644 index 00000000000..b25da5497c5 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-14.c @@ -0,0 +1,26 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +#pragma omp begin declare target +void +f () +{ + + int var; /* { dg-error "'allocate' directive for 'var' inside a target region must specify an 'allocator' clause" } */ + #pragma omp allocate(var) + var = 5; +} +#pragma omp end declare target + +void +h () +{ + #pragma omp target + #pragma omp parallel + #pragma omp serial + { + int var2[5]; /* { dg-error "'allocate' directive for 'var2' inside a target region must specify an 'allocator' clause" } */ + #pragma omp allocate(var2) + var2[0] = 7; + } +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-15.c b/gcc/testsuite/c-c++-common/gomp/allocate-15.c new file mode 100644 index 00000000000..d9600f96c46 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-15.c @@ -0,0 +1,28 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +#pragma omp requires dynamic_allocators + +#pragma omp begin declare target +void +f () +{ + + int var; /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive, used for 'var', not yet supported" } */ + #pragma omp allocate(var) + var = 5; +} +#pragma omp end declare target + +void +h () +{ + #pragma omp target + #pragma omp parallel + #pragma omp serial + { + int var2[5]; /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive, used for 'var2', not yet supported" } */ + #pragma omp allocate(var2) + var2[0] = 7; + } +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c b/gcc/testsuite/c-c++-common/gomp/allocate-5.c index 8a9181205f7..2ca4786264f 100644 --- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c +++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c @@ -18,60 +18,64 @@ typedef enum omp_allocator_handle_t void foo () { + omp_allocator_handle_t my_allocator = omp_default_mem_alloc; int a, b; - omp_allocator_handle_t my_allocator; -#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ -#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ + static int c; +#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate(c) align(32) + /* { dg-message "'allocator' clause required for static variable 'c'" "" { target c } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ } void bar () { - int a, b; + int a, a2, b; omp_allocator_handle_t my_allocator; #pragma omp allocate /* { dg-error "expected '\\(' before end of line" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate allocator(my_allocator) /* { dg-error "expected '\\(' before 'allocator'" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate(a) foo(my_allocator) /* { dg-error "expected 'allocator'" } */ /* { dg-error "expected end of line before '\\(' token" "" { target *-*-* } .-1 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-2 } */ -#pragma omp allocate(a) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ +#pragma omp allocate(a2) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } void align_test () { - int i; - #pragma omp allocate(i) allocator(omp_default_mem_alloc), align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ),allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i),allocator(omp_default_mem_alloc) align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ) allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + int i1,i2,i3,i4,i5,i6; + #pragma omp allocate(i1) allocator(omp_default_mem_alloc), align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 32 ),allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3),allocator(omp_default_mem_alloc) align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i4) align ( 32 ) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ - #pragma omp allocate(i) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) + #pragma omp allocate(i5) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) /* { dg-error "too many 'allocator' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ - #pragma omp allocate(i) align ( 32 ), align(32) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ + #pragma omp allocate(i6) align ( 32 ), align(32) allocator(omp_default_mem_alloc) /* { dg-error "too many 'align' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ } void align_test2 () { - int i; + int i, i2,i3; #pragma omp allocate(i) align (32.0) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-9.c b/gcc/testsuite/c-c++-common/gomp/allocate-9.c new file mode 100644 index 00000000000..6b2b9bfb6b5 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-9.c @@ -0,0 +1,96 @@ +typedef enum omp_allocator_handle_t +{ + omp_null_allocator = 0, + omp_default_mem_alloc = 1, + omp_large_cap_mem_alloc = 2, + omp_const_mem_alloc = 3, + omp_high_bw_mem_alloc = 4, + omp_low_lat_mem_alloc = 5, + omp_cgroup_mem_alloc = 6, + omp_pteam_mem_alloc = 7, + omp_thread_mem_alloc = 8, + __ompx_last_mem_alloc = omp_thread_mem_alloc, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + + +static int A[5] = {1,2,3,4,5}; +int B, C, D; + +/* If the following fails bacause of added predefined allocators, please update + - c/c-parser.c's c_parser_omp_allocate + - fortran/openmp.cc's is_predefined_allocator + - libgomp/env.c's parse_allocator + - libgomp/libgomp.texi (document the new values - multiple locations) + + ensure that the memory-spaces are also up to date. */ + +#pragma omp allocate(A) align(32) allocator((omp_allocator_handle_t) 9) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + + +// typo in allocator name: +#pragma omp allocate(A) allocator(omp_low_latency_mem_alloc) +/* { dg-error "'omp_low_latency_mem_alloc' undeclared here \\(not in a function\\); did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c } .-1 } */ +/* { dg-error "'omp_low_latency_mem_alloc' was not declared in this scope; did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c++ } .-2 } */ +/* { dg-error "'allocator' clause required for static variable 'A'" "" { target c } .-3 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-4 } */ + +/* align be const multiple of 2 */ +#pragma omp allocate(A) align(31) allocator(omp_default_mem_alloc) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'A' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +/* allocator missing (required as A is static) */ +#pragma omp allocate(A) align(32) /* { dg-error "'allocator' clause required for static variable 'A'" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +/* "expression in the clause must be a constant expression that evaluates to one of the + predefined memory allocator values -> omp_low_lat_mem_alloc" */ +#pragma omp allocate(B) allocator((omp_allocator_handle_t) (omp_high_bw_mem_alloc+1)) align(32) /* OK: omp_low_lat_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'B' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(C) allocator((omp_allocator_handle_t) 2) /* OK: omp_large_cap_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(A) align(32) allocator(omp_null_allocator) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +#pragma omp allocate(C) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'C' already appeared as list item in an 'allocate' directive" "" { xfail *-*-* } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +// allocate directive in same TU +int f() +{ + #pragma omp allocate(D) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'allocate' directive must be in the same scope as 'D'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } 18 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return A[0]; +} + +int g() +{ + int a2=1, b2=2; + #pragma omp allocate(a2) +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(a2) /* { dg-error "'a2' already appeared as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + { + int c2=3; + #pragma omp allocate(c2, b2) /* { dg-error "'allocate' directive must be in the same scope as 'b2'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } .-8 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return c2+a2+b2; + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target c } .-5 } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target c } .-12 } */ + } +} + +int h(int q) +{ + #pragma omp allocate(q) /* { dg-error "function parameter 'q' may not appear as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + return q; +} diff --git a/gcc/testsuite/c-c++-common/gomp/directive-1.c b/gcc/testsuite/c-c++-common/gomp/directive-1.c index fc441538778..21ca319b9f9 100644 --- a/gcc/testsuite/c-c++-common/gomp/directive-1.c +++ b/gcc/testsuite/c-c++-common/gomp/directive-1.c @@ -19,7 +19,7 @@ foo (void) int i, k = 0, l = 0; #pragma omp allocate, (i) /* { dg-error "expected '\\\(' before ',' token" } */ /* { dg-error "expected end of line before ',' token" "" { target c++ } .-1 } */ - /* { dg-message "not yet supported" "" { target *-*-* } .-2 } */ + /* { dg-message "not yet supported" "" { target c++ } .-2 } */ #pragma omp critical, (bar) /* { dg-error "expected an OpenMP clause before '\\\(' token" } */ ; #pragma omp flush, (k, l) /* { dg-error "expected '\\\(' or end of line before ',' token" "" { target c } } */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic) 2023-09-11 11:44 ` [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic) Tobias Burnus @ 2023-09-11 11:54 ` Jakub Jelinek 2023-09-11 13:07 ` David Malcolm 2023-09-11 13:21 ` Tobias Burnus 0 siblings, 2 replies; 15+ messages in thread From: Jakub Jelinek @ 2023-09-11 11:54 UTC (permalink / raw) To: Tobias Burnus, David Malcolm; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic) 2023-09-11 11:54 ` Jakub Jelinek @ 2023-09-11 13:07 ` David Malcolm 2023-09-11 13:21 ` Tobias Burnus 1 sibling, 0 replies; 15+ messages in thread From: David Malcolm @ 2023-09-11 13:07 UTC (permalink / raw) To: Jakub Jelinek, Tobias Burnus; +Cc: gcc-patches 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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic) 2023-09-11 11:54 ` Jakub Jelinek 2023-09-11 13:07 ` David Malcolm @ 2023-09-11 13:21 ` Tobias Burnus 2023-09-11 13:34 ` Jakub Jelinek 1 sibling, 1 reply; 15+ messages in thread From: Tobias Burnus @ 2023-09-11 13:21 UTC (permalink / raw) To: Jakub Jelinek, David Malcolm; +Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 622 bytes --] 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 [-- Attachment #2: omp-allocate-c-v4.diff --] [-- Type: text/x-patch, Size: 32353 bytes --] 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 %<allocate%> 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 (case_loc, + "switch jumps over OpenMP %<allocate%> allocation"); + emitted = true; + } else emitted = warning_at (case_loc, OPT_Wjump_misses_init, diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index c04962d67a4..643ec02706b 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -1681,6 +1681,7 @@ static bool c_parser_omp_declare (c_parser *, enum pragma_context); static void c_parser_omp_requires (c_parser *); static bool c_parser_omp_error (c_parser *, enum pragma_context); static void c_parser_omp_assumption_clauses (c_parser *, bool); +static void c_parser_omp_allocate (c_parser *); static void c_parser_omp_assumes (c_parser *); static bool c_parser_omp_ordered (c_parser *, enum pragma_context, bool *); static void c_parser_oacc_routine (c_parser *, enum pragma_context); @@ -13649,6 +13650,10 @@ c_parser_pragma (c_parser *parser, enum pragma_context context, bool *if_p) c_parser_omp_requires (parser); return false; + case PRAGMA_OMP_ALLOCATE: + c_parser_omp_allocate (parser); + return false; + case PRAGMA_OMP_ASSUMES: if (context != pragma_external) { @@ -19348,10 +19353,13 @@ c_parser_oacc_wait (location_t loc, c_parser *parser, char *p_name) align (constant-expression)] */ static void -c_parser_omp_allocate (location_t loc, c_parser *parser) +c_parser_omp_allocate (c_parser *parser) { tree alignment = NULL_TREE; tree allocator = NULL_TREE; + c_parser_consume_pragma (parser); + location_t loc = c_parser_peek_token (parser)->location; + location_t allocator_loc = UNKNOWN_LOCATION; tree nl = c_parser_omp_var_list_parens (parser, OMP_CLAUSE_ALLOCATE, NULL_TREE); do { @@ -19376,7 +19384,9 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) c_expr expr = c_parser_expr_no_commas (parser, NULL); expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); expr_loc = c_parser_peek_token (parser)->location; - if (p[2] == 'i' && alignment) + if (expr.value == error_mark_node) + ; + else if (p[2] == 'i' && alignment) { error_at (expr_loc, "too many %qs clauses", "align"); break; @@ -19403,6 +19413,7 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) else { allocator = c_fully_fold (expr.value, false, NULL); + allocator_loc = expr_loc; tree orig_type = expr.original_type ? expr.original_type : TREE_TYPE (allocator); orig_type = TYPE_MAIN_VARIANT (orig_type); @@ -19422,14 +19433,95 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) } while (true); c_parser_skip_to_pragma_eol (parser); - if (allocator || alignment) - for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) - { - OMP_CLAUSE_ALLOCATE_ALLOCATOR (c) = allocator; - OMP_CLAUSE_ALLOCATE_ALIGN (c) = alignment; - } - - sorry_at (loc, "%<#pragma omp allocate%> not yet supported"); + c_mark_decl_jump_unsafe_in_current_scope (); + for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) + { + tree var = OMP_CLAUSE_DECL (c); + if (TREE_CODE (var) == PARM_DECL) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "function parameter %qD may not appear as list item in an " + "%<allocate%> directive", var); + continue; + } + if (!c_check_in_current_scope (var)) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%<allocate%> directive must be in the same scope as %qD", + var); + inform (DECL_SOURCE_LOCATION (var), "declared here"); + continue; + } + if (lookup_attribute ("omp allocate", DECL_ATTRIBUTES (var))) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%qD already appeared as list item in an " + "%<allocate%> directive", var); + continue; + } + if (TREE_STATIC (var)) + { + if (allocator == NULL_TREE && allocator_loc == UNKNOWN_LOCATION) + error_at (loc, "%<allocator%> clause required for " + "static variable %qD", var); + else if (allocator + && (tree_int_cst_sgn (allocator) != 1 + || tree_to_shwi (allocator) > 8)) + /* 8 = largest predefined memory allocator. */ + error_at (allocator_loc, + "%<allocator%> clause requires a predefined allocator as " + "%qD is static", var); + else + sorry_at (OMP_CLAUSE_LOCATION (nl), + "%<#pragma omp allocate%> for static variables like " + "%qD not yet supported", var); + continue; + } + 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 " + "%<allocate%> directive for %qD, must not be " + "modified between declaration of %qD and its " + "%<allocate%> directive", + allocator, var, var); + inform (DECL_SOURCE_LOCATION (var), "declared here"); + inform (OMP_CLAUSE_LOCATION (nl), "used here"); + break; + } + --l; + } + } + } + DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"), + build_tree_list (allocator, alignment), + DECL_ATTRIBUTES (var)); + } } /* OpenMP 2.5: @@ -24926,9 +25018,6 @@ c_parser_omp_construct (c_parser *parser, bool *if_p) strcpy (p_name, "#pragma wait"); stmt = c_parser_oacc_wait (loc, parser, p_name); break; - case PRAGMA_OMP_ALLOCATE: - c_parser_omp_allocate (loc, parser); - return; case PRAGMA_OMP_ATOMIC: c_parser_omp_atomic (loc, parser, false); return; diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index f928137c8d4..2664354337b 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -626,6 +626,7 @@ extern unsigned int start_underspecified_init (location_t, tree); extern void finish_underspecified_init (tree, unsigned int); extern void push_scope (void); extern tree pop_scope (void); +extern void c_mark_decl_jump_unsafe_in_current_scope (); extern void c_bindings_start_stmt_expr (struct c_spot_bindings *); extern void c_bindings_end_stmt_expr (struct c_spot_bindings *); diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index a49b50bc857..a0e8cc2199d 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -1363,6 +1363,46 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) if (VAR_P (t)) { struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp; + tree attr; + + if (flag_openmp + && !is_global_var (t) + && DECL_CONTEXT (t) == current_function_decl + && TREE_USED (t) + && (attr = lookup_attribute ("omp allocate", DECL_ATTRIBUTES (t))) + != NULL_TREE) + { + tree alloc = TREE_PURPOSE (TREE_VALUE (attr)); + tree align = TREE_VALUE (TREE_VALUE (attr)); + /* Allocate directives that appear in a target region must specify + an allocator clause unless a requires directive with the + dynamic_allocators clause is present in the same compilation + unit. */ + bool missing_dyn_alloc = false; + if (alloc == NULL_TREE + && ((omp_requires_mask & OMP_REQUIRES_DYNAMIC_ALLOCATORS) + == 0)) + { + /* This comes too early for omp_discover_declare_target..., + but should at least catch the most common cases. */ + missing_dyn_alloc + = cgraph_node::get (current_function_decl)->offloadable; + for (struct gimplify_omp_ctx *ctx2 = ctx; + ctx2 && !missing_dyn_alloc; ctx2 = ctx2->outer_context) + if (ctx2->code == OMP_TARGET) + missing_dyn_alloc = true; + } + if (missing_dyn_alloc) + error_at (DECL_SOURCE_LOCATION (t), + "%<allocate%> directive for %qD inside a target " + "region must specify an %<allocator%> clause", t); + else if (align != NULL_TREE + || alloc == NULL_TREE + || !integer_onep (alloc)) + sorry_at (DECL_SOURCE_LOCATION (t), + "OpenMP %<allocate%> directive, used for %qD, not " + "yet supported", t); + } /* Mark variable as local. */ if (ctx && ctx->region_type != ORT_NONE && !DECL_EXTERNAL (t)) diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-11.c b/gcc/testsuite/c-c++-common/gomp/allocate-11.c new file mode 100644 index 00000000000..f9ad50abb7f --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-11.c @@ -0,0 +1,40 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +void bar(); +void use (int*); + +void +f (int i) +{ + switch (i) /* { dg-note "switch starts here" } */ + { + int j; /* { dg-note "'j' declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + #pragma omp allocate(j) + case 42: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" } */ + bar (); + /* { dg-warning "statement will never be executed \\\[-Wswitch-unreachable\\\]" "" { target *-*-* } .-1 } */ + break; + case 51: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" } */ + use (&j); + break; + } +} + +int +h (int i2) +{ + if (i2 == 5) + goto label; /* { dg-error "jump skips OpenMP 'allocate' allocation" } */ + return 5; + + int k2; /* { dg-note "'k2' declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + int j2 = 4; /* { dg-note "'j2' declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + #pragma omp allocate(k2, j2) +label: /* { dg-note "label 'label' defined here" } */ + k2 = 4; + return j2 + k2; +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-12.c b/gcc/testsuite/c-c++-common/gomp/allocate-12.c new file mode 100644 index 00000000000..8474b2fe27b --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-12.c @@ -0,0 +1,49 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +typedef enum omp_allocator_handle_t +#if __cplusplus >= 201103L +: __UINTPTR_TYPE__ +#endif +{ + omp_default_mem_alloc = 1, + omp_low_lat_mem_alloc = 5, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + +int +f () +{ + omp_allocator_handle_t my_allocator; + int n = 5; /* { dg-note "declared here" } */ + my_allocator = omp_default_mem_alloc; /* { dg-error "allocator variable 'my_allocator', used in the 'allocate' directive for 'n', must not be modified between declaration of 'n' and its 'allocate' directive" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-2 } */ + #pragma omp allocate(n) allocator(my_allocator) /* { dg-note "used here" } */ + n = 7; + return n; +} + + +int +g () +{ + int n = 5; /* { dg-note "declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; /* { dg-note "allocator declared here" } */ + #pragma omp allocate(n) allocator(my_allocator) /* { dg-error "allocator variable 'my_allocator' must be declared before 'n'" } */ + n = 7; + return n; +} + +int +h () +{ + /* my_allocator uninitialized - but only diagnosed in the ME with -Wuninitialized; + see gomp/allocate-10.c. */ + omp_allocator_handle_t my_allocator; + int n = 5; + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + #pragma omp allocate(n) allocator(my_allocator) + n = 7; + return n; +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-14.c b/gcc/testsuite/c-c++-common/gomp/allocate-14.c new file mode 100644 index 00000000000..b25da5497c5 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-14.c @@ -0,0 +1,26 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +#pragma omp begin declare target +void +f () +{ + + int var; /* { dg-error "'allocate' directive for 'var' inside a target region must specify an 'allocator' clause" } */ + #pragma omp allocate(var) + var = 5; +} +#pragma omp end declare target + +void +h () +{ + #pragma omp target + #pragma omp parallel + #pragma omp serial + { + int var2[5]; /* { dg-error "'allocate' directive for 'var2' inside a target region must specify an 'allocator' clause" } */ + #pragma omp allocate(var2) + var2[0] = 7; + } +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-15.c b/gcc/testsuite/c-c++-common/gomp/allocate-15.c new file mode 100644 index 00000000000..d9600f96c46 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-15.c @@ -0,0 +1,28 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +#pragma omp requires dynamic_allocators + +#pragma omp begin declare target +void +f () +{ + + int var; /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive, used for 'var', not yet supported" } */ + #pragma omp allocate(var) + var = 5; +} +#pragma omp end declare target + +void +h () +{ + #pragma omp target + #pragma omp parallel + #pragma omp serial + { + int var2[5]; /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive, used for 'var2', not yet supported" } */ + #pragma omp allocate(var2) + var2[0] = 7; + } +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c b/gcc/testsuite/c-c++-common/gomp/allocate-5.c index 8a9181205f7..2ca4786264f 100644 --- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c +++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c @@ -18,60 +18,64 @@ typedef enum omp_allocator_handle_t void foo () { + omp_allocator_handle_t my_allocator = omp_default_mem_alloc; int a, b; - omp_allocator_handle_t my_allocator; -#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ -#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ + static int c; +#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate(c) align(32) + /* { dg-message "'allocator' clause required for static variable 'c'" "" { target c } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ } void bar () { - int a, b; + int a, a2, b; omp_allocator_handle_t my_allocator; #pragma omp allocate /* { dg-error "expected '\\(' before end of line" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate allocator(my_allocator) /* { dg-error "expected '\\(' before 'allocator'" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate(a) foo(my_allocator) /* { dg-error "expected 'allocator'" } */ /* { dg-error "expected end of line before '\\(' token" "" { target *-*-* } .-1 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-2 } */ -#pragma omp allocate(a) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ +#pragma omp allocate(a2) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } void align_test () { - int i; - #pragma omp allocate(i) allocator(omp_default_mem_alloc), align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ),allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i),allocator(omp_default_mem_alloc) align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ) allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + int i1,i2,i3,i4,i5,i6; + #pragma omp allocate(i1) allocator(omp_default_mem_alloc), align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 32 ),allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3),allocator(omp_default_mem_alloc) align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i4) align ( 32 ) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ - #pragma omp allocate(i) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) + #pragma omp allocate(i5) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) /* { dg-error "too many 'allocator' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ - #pragma omp allocate(i) align ( 32 ), align(32) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ + #pragma omp allocate(i6) align ( 32 ), align(32) allocator(omp_default_mem_alloc) /* { dg-error "too many 'align' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ } void align_test2 () { - int i; + int i, i2,i3; #pragma omp allocate(i) align (32.0) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-9.c b/gcc/testsuite/c-c++-common/gomp/allocate-9.c new file mode 100644 index 00000000000..f35404a3173 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-9.c @@ -0,0 +1,99 @@ +typedef enum omp_allocator_handle_t +#if __cplusplus >= 201103L +: __UINTPTR_TYPE__ +#endif +{ + omp_null_allocator = 0, + omp_default_mem_alloc = 1, + omp_large_cap_mem_alloc = 2, + omp_const_mem_alloc = 3, + omp_high_bw_mem_alloc = 4, + omp_low_lat_mem_alloc = 5, + omp_cgroup_mem_alloc = 6, + omp_pteam_mem_alloc = 7, + omp_thread_mem_alloc = 8, + __ompx_last_mem_alloc = omp_thread_mem_alloc, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + + +static int A[5] = {1,2,3,4,5}; +int B, C, D; + +/* If the following fails bacause of added predefined allocators, please update + - c/c-parser.c's c_parser_omp_allocate + - fortran/openmp.cc's is_predefined_allocator + - libgomp/env.c's parse_allocator + - libgomp/libgomp.texi (document the new values - multiple locations) + + ensure that the memory-spaces are also up to date. */ + +#pragma omp allocate(A) align(32) allocator((omp_allocator_handle_t) 9) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + + +// typo in allocator name: +#pragma omp allocate(A) allocator(omp_low_latency_mem_alloc) +/* { dg-error "'omp_low_latency_mem_alloc' undeclared here \\(not in a function\\); did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c } .-1 } */ +/* { dg-error "'omp_low_latency_mem_alloc' was not declared in this scope; did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c++ } .-2 } */ +/* { dg-error "'allocator' clause required for static variable 'A'" "" { target c } .-3 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-4 } */ + +/* align be const multiple of 2 */ +#pragma omp allocate(A) align(31) allocator(omp_default_mem_alloc) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'A' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +/* allocator missing (required as A is static) */ +#pragma omp allocate(A) align(32) /* { dg-error "'allocator' clause required for static variable 'A'" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +/* "expression in the clause must be a constant expression that evaluates to one of the + predefined memory allocator values -> omp_low_lat_mem_alloc" */ +#pragma omp allocate(B) allocator((omp_allocator_handle_t) (omp_high_bw_mem_alloc+1)) align(32) /* OK: omp_low_lat_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'B' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(C) allocator((omp_allocator_handle_t) 2) /* OK: omp_large_cap_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(A) align(32) allocator(omp_null_allocator) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +#pragma omp allocate(C) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'C' already appeared as list item in an 'allocate' directive" "" { xfail *-*-* } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +// allocate directive in same TU +int f() +{ + #pragma omp allocate(D) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'allocate' directive must be in the same scope as 'D'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } 21 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return A[0]; +} + +int g() +{ + int a2=1, b2=2; + #pragma omp allocate(a2) +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(a2) /* { dg-error "'a2' already appeared as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + { + int c2=3; + #pragma omp allocate(c2, b2) /* { dg-error "'allocate' directive must be in the same scope as 'b2'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } .-8 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return c2+a2+b2; + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target c } .-5 } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target c } .-12 } */ + } +} + +int h(int q) +{ + #pragma omp allocate(q) /* { dg-error "function parameter 'q' may not appear as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + return q; +} diff --git a/gcc/testsuite/c-c++-common/gomp/directive-1.c b/gcc/testsuite/c-c++-common/gomp/directive-1.c index fc441538778..21ca319b9f9 100644 --- a/gcc/testsuite/c-c++-common/gomp/directive-1.c +++ b/gcc/testsuite/c-c++-common/gomp/directive-1.c @@ -19,7 +19,7 @@ foo (void) int i, k = 0, l = 0; #pragma omp allocate, (i) /* { dg-error "expected '\\\(' before ',' token" } */ /* { dg-error "expected end of line before ',' token" "" { target c++ } .-1 } */ - /* { dg-message "not yet supported" "" { target *-*-* } .-2 } */ + /* { dg-message "not yet supported" "" { target c++ } .-2 } */ #pragma omp critical, (bar) /* { dg-error "expected an OpenMP clause before '\\\(' token" } */ ; #pragma omp flush, (k, l) /* { dg-error "expected '\\\(' or end of line before ',' token" "" { target c } } */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic) 2023-09-11 13:21 ` Tobias Burnus @ 2023-09-11 13:34 ` Jakub Jelinek 2023-09-12 7:04 ` [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic Tobias Burnus 0 siblings, 1 reply; 15+ messages in thread From: Jakub Jelinek @ 2023-09-11 13:34 UTC (permalink / raw) To: Tobias Burnus; +Cc: David Malcolm, gcc-patches 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, "%<allocator%> 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 " > + "%<allocate%> directive for %qD, must not be " > + "modified between declaration of %qD and its " > + "%<allocate%> 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, &b[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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic 2023-09-11 13:34 ` Jakub Jelinek @ 2023-09-12 7:04 ` Tobias Burnus 2023-09-12 7:15 ` Jakub Jelinek 2023-09-12 8:55 ` Tobias Burnus 0 siblings, 2 replies; 15+ messages in thread From: Tobias Burnus @ 2023-09-12 7:04 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 2827 bytes --] 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, "%<allocator%> 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 " >> + "%<allocate%> directive for %qD, must not be " >> + "modified between declaration of %qD and its " >> + "%<allocate%> 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 [-- Attachment #2: omp-allocate-c-v5.diff --] [-- Type: text/x-patch, Size: 34185 bytes --] 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 +++++++++ gcc/testsuite/c-c++-common/gomp/allocate-5.c | 60 +++++++------- gcc/testsuite/c-c++-common/gomp/allocate-9.c | 108 ++++++++++++++++++++++++ gcc/testsuite/c-c++-common/gomp/directive-1.c | 2 +- 12 files changed, 491 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 %<allocate%> 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 (case_loc, + "switch jumps over OpenMP %<allocate%> allocation"); + emitted = true; + } else emitted = warning_at (case_loc, OPT_Wjump_misses_init, diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index c04962d67a4..643ec02706b 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -1681,6 +1681,7 @@ static bool c_parser_omp_declare (c_parser *, enum pragma_context); static void c_parser_omp_requires (c_parser *); static bool c_parser_omp_error (c_parser *, enum pragma_context); static void c_parser_omp_assumption_clauses (c_parser *, bool); +static void c_parser_omp_allocate (c_parser *); static void c_parser_omp_assumes (c_parser *); static bool c_parser_omp_ordered (c_parser *, enum pragma_context, bool *); static void c_parser_oacc_routine (c_parser *, enum pragma_context); @@ -13649,6 +13650,10 @@ c_parser_pragma (c_parser *parser, enum pragma_context context, bool *if_p) c_parser_omp_requires (parser); return false; + case PRAGMA_OMP_ALLOCATE: + c_parser_omp_allocate (parser); + return false; + case PRAGMA_OMP_ASSUMES: if (context != pragma_external) { @@ -19348,10 +19353,13 @@ c_parser_oacc_wait (location_t loc, c_parser *parser, char *p_name) align (constant-expression)] */ static void -c_parser_omp_allocate (location_t loc, c_parser *parser) +c_parser_omp_allocate (c_parser *parser) { tree alignment = NULL_TREE; tree allocator = NULL_TREE; + c_parser_consume_pragma (parser); + location_t loc = c_parser_peek_token (parser)->location; + location_t allocator_loc = UNKNOWN_LOCATION; tree nl = c_parser_omp_var_list_parens (parser, OMP_CLAUSE_ALLOCATE, NULL_TREE); do { @@ -19376,7 +19384,9 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) c_expr expr = c_parser_expr_no_commas (parser, NULL); expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); expr_loc = c_parser_peek_token (parser)->location; - if (p[2] == 'i' && alignment) + if (expr.value == error_mark_node) + ; + else if (p[2] == 'i' && alignment) { error_at (expr_loc, "too many %qs clauses", "align"); break; @@ -19403,6 +19413,7 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) else { allocator = c_fully_fold (expr.value, false, NULL); + allocator_loc = expr_loc; tree orig_type = expr.original_type ? expr.original_type : TREE_TYPE (allocator); orig_type = TYPE_MAIN_VARIANT (orig_type); @@ -19422,14 +19433,95 @@ c_parser_omp_allocate (location_t loc, c_parser *parser) } while (true); c_parser_skip_to_pragma_eol (parser); - if (allocator || alignment) - for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) - { - OMP_CLAUSE_ALLOCATE_ALLOCATOR (c) = allocator; - OMP_CLAUSE_ALLOCATE_ALIGN (c) = alignment; - } - - sorry_at (loc, "%<#pragma omp allocate%> not yet supported"); + c_mark_decl_jump_unsafe_in_current_scope (); + for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c)) + { + tree var = OMP_CLAUSE_DECL (c); + if (TREE_CODE (var) == PARM_DECL) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "function parameter %qD may not appear as list item in an " + "%<allocate%> directive", var); + continue; + } + if (!c_check_in_current_scope (var)) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%<allocate%> directive must be in the same scope as %qD", + var); + inform (DECL_SOURCE_LOCATION (var), "declared here"); + continue; + } + if (lookup_attribute ("omp allocate", DECL_ATTRIBUTES (var))) + { + error_at (OMP_CLAUSE_LOCATION (nl), + "%qD already appeared as list item in an " + "%<allocate%> directive", var); + continue; + } + if (TREE_STATIC (var)) + { + if (allocator == NULL_TREE && allocator_loc == UNKNOWN_LOCATION) + error_at (loc, "%<allocator%> clause required for " + "static variable %qD", var); + else if (allocator + && (tree_int_cst_sgn (allocator) != 1 + || tree_to_shwi (allocator) > 8)) + /* 8 = largest predefined memory allocator. */ + error_at (allocator_loc, + "%<allocator%> clause requires a predefined allocator as " + "%qD is static", var); + else + sorry_at (OMP_CLAUSE_LOCATION (nl), + "%<#pragma omp allocate%> for static variables like " + "%qD not yet supported", var); + continue; + } + 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 " + "%<allocate%> directive for %qD, must not be " + "modified between declaration of %qD and its " + "%<allocate%> directive", + allocator, var, var); + inform (DECL_SOURCE_LOCATION (var), "declared here"); + inform (OMP_CLAUSE_LOCATION (nl), "used here"); + break; + } + --l; + } + } + } + DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"), + build_tree_list (allocator, alignment), + DECL_ATTRIBUTES (var)); + } } /* OpenMP 2.5: @@ -24926,9 +25018,6 @@ c_parser_omp_construct (c_parser *parser, bool *if_p) strcpy (p_name, "#pragma wait"); stmt = c_parser_oacc_wait (loc, parser, p_name); break; - case PRAGMA_OMP_ALLOCATE: - c_parser_omp_allocate (loc, parser); - return; case PRAGMA_OMP_ATOMIC: c_parser_omp_atomic (loc, parser, false); return; diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index f928137c8d4..2664354337b 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -626,6 +626,7 @@ extern unsigned int start_underspecified_init (location_t, tree); extern void finish_underspecified_init (tree, unsigned int); extern void push_scope (void); extern tree pop_scope (void); +extern void c_mark_decl_jump_unsafe_in_current_scope (); extern void c_bindings_start_stmt_expr (struct c_spot_bindings *); extern void c_bindings_end_stmt_expr (struct c_spot_bindings *); diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index a49b50bc857..a0e8cc2199d 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -1363,6 +1363,46 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) if (VAR_P (t)) { struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp; + tree attr; + + if (flag_openmp + && !is_global_var (t) + && DECL_CONTEXT (t) == current_function_decl + && TREE_USED (t) + && (attr = lookup_attribute ("omp allocate", DECL_ATTRIBUTES (t))) + != NULL_TREE) + { + tree alloc = TREE_PURPOSE (TREE_VALUE (attr)); + tree align = TREE_VALUE (TREE_VALUE (attr)); + /* Allocate directives that appear in a target region must specify + an allocator clause unless a requires directive with the + dynamic_allocators clause is present in the same compilation + unit. */ + bool missing_dyn_alloc = false; + if (alloc == NULL_TREE + && ((omp_requires_mask & OMP_REQUIRES_DYNAMIC_ALLOCATORS) + == 0)) + { + /* This comes too early for omp_discover_declare_target..., + but should at least catch the most common cases. */ + missing_dyn_alloc + = cgraph_node::get (current_function_decl)->offloadable; + for (struct gimplify_omp_ctx *ctx2 = ctx; + ctx2 && !missing_dyn_alloc; ctx2 = ctx2->outer_context) + if (ctx2->code == OMP_TARGET) + missing_dyn_alloc = true; + } + if (missing_dyn_alloc) + error_at (DECL_SOURCE_LOCATION (t), + "%<allocate%> directive for %qD inside a target " + "region must specify an %<allocator%> clause", t); + else if (align != NULL_TREE + || alloc == NULL_TREE + || !integer_onep (alloc)) + sorry_at (DECL_SOURCE_LOCATION (t), + "OpenMP %<allocate%> directive, used for %qD, not " + "yet supported", t); + } /* Mark variable as local. */ if (ctx && ctx->region_type != ORT_NONE && !DECL_EXTERNAL (t)) diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-11.c b/gcc/testsuite/c-c++-common/gomp/allocate-11.c new file mode 100644 index 00000000000..f9ad50abb7f --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-11.c @@ -0,0 +1,40 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +void bar(); +void use (int*); + +void +f (int i) +{ + switch (i) /* { dg-note "switch starts here" } */ + { + int j; /* { dg-note "'j' declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + #pragma omp allocate(j) + case 42: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" } */ + bar (); + /* { dg-warning "statement will never be executed \\\[-Wswitch-unreachable\\\]" "" { target *-*-* } .-1 } */ + break; + case 51: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" } */ + use (&j); + break; + } +} + +int +h (int i2) +{ + if (i2 == 5) + goto label; /* { dg-error "jump skips OpenMP 'allocate' allocation" } */ + return 5; + + int k2; /* { dg-note "'k2' declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + int j2 = 4; /* { dg-note "'j2' declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + #pragma omp allocate(k2, j2) +label: /* { dg-note "label 'label' defined here" } */ + k2 = 4; + return j2 + k2; +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-12.c b/gcc/testsuite/c-c++-common/gomp/allocate-12.c new file mode 100644 index 00000000000..3c7c3bb3a2b --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-12.c @@ -0,0 +1,49 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +typedef enum omp_allocator_handle_t +#if __cplusplus >= 201103L +: __UINTPTR_TYPE__ +#endif +{ + omp_default_mem_alloc = 1, + omp_low_lat_mem_alloc = 5, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + +int +f () +{ + omp_allocator_handle_t my_allocator; + int n = 5; /* { dg-note "to be allocated variable declared here" } */ + my_allocator = omp_default_mem_alloc; /* { dg-note "modified here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-2 } */ + #pragma omp allocate(n) allocator(my_allocator) /* { dg-error "variable 'my_allocator' used in the 'allocator' clause must not be modified between declaration of 'n' and its 'allocate' directive" } */ + n = 7; + return n; +} + + +int +g () +{ + int n = 5; /* { dg-note "to be allocated variable declared here" } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; /* { dg-note "declared here" } */ + #pragma omp allocate(n) allocator(my_allocator) /* { dg-error "variable 'my_allocator' used in the 'allocator' clause must be declared before 'n'" } */ + n = 7; + return n; +} + +int +h () +{ + /* my_allocator uninitialized - but only diagnosed in the ME with -Wuninitialized; + see gomp/allocate-10.c. */ + omp_allocator_handle_t my_allocator; + int n = 5; + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target *-*-* } .-1 } */ + #pragma omp allocate(n) allocator(my_allocator) + n = 7; + return n; +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-14.c b/gcc/testsuite/c-c++-common/gomp/allocate-14.c new file mode 100644 index 00000000000..b25da5497c5 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-14.c @@ -0,0 +1,26 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +#pragma omp begin declare target +void +f () +{ + + int var; /* { dg-error "'allocate' directive for 'var' inside a target region must specify an 'allocator' clause" } */ + #pragma omp allocate(var) + var = 5; +} +#pragma omp end declare target + +void +h () +{ + #pragma omp target + #pragma omp parallel + #pragma omp serial + { + int var2[5]; /* { dg-error "'allocate' directive for 'var2' inside a target region must specify an 'allocator' clause" } */ + #pragma omp allocate(var2) + var2[0] = 7; + } +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-15.c b/gcc/testsuite/c-c++-common/gomp/allocate-15.c new file mode 100644 index 00000000000..d9600f96c46 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-15.c @@ -0,0 +1,28 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +#pragma omp requires dynamic_allocators + +#pragma omp begin declare target +void +f () +{ + + int var; /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive, used for 'var', not yet supported" } */ + #pragma omp allocate(var) + var = 5; +} +#pragma omp end declare target + +void +h () +{ + #pragma omp target + #pragma omp parallel + #pragma omp serial + { + int var2[5]; /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive, used for 'var2', not yet supported" } */ + #pragma omp allocate(var2) + var2[0] = 7; + } +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-16.c b/gcc/testsuite/c-c++-common/gomp/allocate-16.c new file mode 100644 index 00000000000..08738030e6a --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-16.c @@ -0,0 +1,38 @@ +/* TODO: enable for C++ once implemented. */ +/* { dg-do compile { target c } } */ + +typedef enum omp_allocator_handle_t +#if __cplusplus >= 201103L +: __UINTPTR_TYPE__ +#endif +{ + omp_default_mem_alloc = 1, + omp_low_lat_mem_alloc = 5, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + +omp_allocator_handle_t foo(int, int *); + + +void +f () +{ + int v; /* { dg-note "to be allocated variable declared here" } */ + int n = 5; + int a = 1; /* { dg-note "declared here" } */ + int b[n]; + b[a] = 5; + #pragma omp allocate (v) allocator (foo (a, &b[a])) /* { dg-error "variable 'a' used in the 'allocator' clause must be declared before 'v'" } */ +} + +void +g () +{ + int n = 5; + int a = 1; + int b[n]; + b[a] = 5; + int v; /* { dg-note "to be allocated variable declared here" } */ + a = 2; /* { dg-note "modified here" } */ + #pragma omp allocate (v) allocator (foo (a, &b[a])) /* { dg-error "variable 'a' used in the 'allocator' clause must not be modified between declaration of 'v' and its 'allocate' directive" } */ +} diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c b/gcc/testsuite/c-c++-common/gomp/allocate-5.c index 8a9181205f7..2ca4786264f 100644 --- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c +++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c @@ -18,60 +18,64 @@ typedef enum omp_allocator_handle_t void foo () { + omp_allocator_handle_t my_allocator = omp_default_mem_alloc; int a, b; - omp_allocator_handle_t my_allocator; -#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ -#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" } */ + static int c; +#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */ +#pragma omp allocate(c) align(32) + /* { dg-message "'allocator' clause required for static variable 'c'" "" { target c } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ } void bar () { - int a, b; + int a, a2, b; omp_allocator_handle_t my_allocator; #pragma omp allocate /* { dg-error "expected '\\(' before end of line" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate allocator(my_allocator) /* { dg-error "expected '\\(' before 'allocator'" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ #pragma omp allocate(a) foo(my_allocator) /* { dg-error "expected 'allocator'" } */ /* { dg-error "expected end of line before '\\(' token" "" { target *-*-* } .-1 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-2 } */ -#pragma omp allocate(a) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ +#pragma omp allocate(a2) allocator(b) /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } void align_test () { - int i; - #pragma omp allocate(i) allocator(omp_default_mem_alloc), align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ),allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i),allocator(omp_default_mem_alloc) align(32) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 32 ) allocator(omp_default_mem_alloc) - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + int i1,i2,i3,i4,i5,i6; + #pragma omp allocate(i1) allocator(omp_default_mem_alloc), align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 32 ),allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3),allocator(omp_default_mem_alloc) align(32) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i4) align ( 32 ) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ - #pragma omp allocate(i) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) + #pragma omp allocate(i5) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc) /* { dg-error "too many 'allocator' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ - #pragma omp allocate(i) align ( 32 ), align(32) allocator(omp_default_mem_alloc) + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ + #pragma omp allocate(i6) align ( 32 ), align(32) allocator(omp_default_mem_alloc) /* { dg-error "too many 'align' clauses" "" { target *-*-* } .-1 } */ /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-3 } */ } void align_test2 () { - int i; + int i, i2,i3; #pragma omp allocate(i) align (32.0) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ - #pragma omp allocate(i) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ - /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i2) align ( 31 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(i3) align ( -32 ) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ + /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ } diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-9.c b/gcc/testsuite/c-c++-common/gomp/allocate-9.c new file mode 100644 index 00000000000..8e010419a5f --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/allocate-9.c @@ -0,0 +1,108 @@ +typedef enum omp_allocator_handle_t +#if __cplusplus >= 201103L +: __UINTPTR_TYPE__ +#endif +{ + omp_null_allocator = 0, + omp_default_mem_alloc = 1, + omp_large_cap_mem_alloc = 2, + omp_const_mem_alloc = 3, + omp_high_bw_mem_alloc = 4, + omp_low_lat_mem_alloc = 5, + omp_cgroup_mem_alloc = 6, + omp_pteam_mem_alloc = 7, + omp_thread_mem_alloc = 8, + __ompx_last_mem_alloc = omp_thread_mem_alloc, + __omp_allocator_handle_t_max__ = __UINTPTR_MAX__ +} omp_allocator_handle_t; + + +static int A[5] = {1,2,3,4,5}; +int B, C, D; + +/* If the following fails bacause of added predefined allocators, please update + - c/c-parser.c's c_parser_omp_allocate + - fortran/openmp.cc's is_predefined_allocator + - libgomp/env.c's parse_allocator + - libgomp/libgomp.texi (document the new values - multiple locations) + + ensure that the memory-spaces are also up to date. */ + +#pragma omp allocate(A) align(32) allocator((omp_allocator_handle_t) 9) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + + +// typo in allocator name: +#pragma omp allocate(A) allocator(omp_low_latency_mem_alloc) +/* { dg-error "'omp_low_latency_mem_alloc' undeclared here \\(not in a function\\); did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c } .-1 } */ +/* { dg-error "'omp_low_latency_mem_alloc' was not declared in this scope; did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c++ } .-2 } */ +/* { dg-error "'allocator' clause required for static variable 'A'" "" { target c } .-3 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-4 } */ + +/* align be const multiple of 2 */ +#pragma omp allocate(A) align(31) allocator(omp_default_mem_alloc) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'A' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +/* allocator missing (required as A is static) */ +#pragma omp allocate(A) align(32) /* { dg-error "'allocator' clause required for static variable 'A'" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +/* "expression in the clause must be a constant expression that evaluates to one of the + predefined memory allocator values -> omp_low_lat_mem_alloc" */ +#pragma omp allocate(B) allocator((omp_allocator_handle_t) (omp_high_bw_mem_alloc+1)) align(32) /* OK: omp_low_lat_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'B' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(C) allocator((omp_allocator_handle_t) 2) /* OK: omp_large_cap_mem_alloc */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +#pragma omp allocate(A) align(32) allocator(omp_null_allocator) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + +#pragma omp allocate(C) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'C' already appeared as list item in an 'allocate' directive" "" { xfail *-*-* } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + +// allocate directive in same TU +int f() +{ + #pragma omp allocate(D) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'allocate' directive must be in the same scope as 'D'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } 21 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return A[0]; +} + +int g() +{ + int a2=1, b2=2; + #pragma omp allocate(a2) +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + #pragma omp allocate(a2) /* { dg-error "'a2' already appeared as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + { + int c2=3; + #pragma omp allocate(c2, b2) /* { dg-error "'allocate' directive must be in the same scope as 'b2'" "" { xfail c++ } } */ +/* { dg-note "declared here" "" { target c } .-8 } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */ + return c2+a2+b2; + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target c } .-5 } */ + /* { dg-message "sorry, unimplemented: OpenMP 'allocate' directive" "" { target c } .-12 } */ + } +} + +int h(int q) +{ + #pragma omp allocate(q) /* { dg-error "function parameter 'q' may not appear as list item in an 'allocate' directive" "" { xfail c++ } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + return q; +} + +int +k () +{ + static int var3 = 8; + #pragma omp allocate(var3) allocator((omp_allocator_handle_t)-1L) /* { dg-error "'allocator' clause requires a predefined allocator as 'var3' is static" "" { target c } } */ +/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */ + return var3; +} diff --git a/gcc/testsuite/c-c++-common/gomp/directive-1.c b/gcc/testsuite/c-c++-common/gomp/directive-1.c index fc441538778..21ca319b9f9 100644 --- a/gcc/testsuite/c-c++-common/gomp/directive-1.c +++ b/gcc/testsuite/c-c++-common/gomp/directive-1.c @@ -19,7 +19,7 @@ foo (void) int i, k = 0, l = 0; #pragma omp allocate, (i) /* { dg-error "expected '\\\(' before ',' token" } */ /* { dg-error "expected end of line before ',' token" "" { target c++ } .-1 } */ - /* { dg-message "not yet supported" "" { target *-*-* } .-2 } */ + /* { dg-message "not yet supported" "" { target c++ } .-2 } */ #pragma omp critical, (bar) /* { dg-error "expected an OpenMP clause before '\\\(' token" } */ ; #pragma omp flush, (k, l) /* { dg-error "expected '\\\(' or end of line before ',' token" "" { target c } } */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic 2023-09-12 7:04 ` [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic Tobias Burnus @ 2023-09-12 7:15 ` Jakub Jelinek 2023-09-12 8:55 ` Tobias Burnus 1 sibling, 0 replies; 15+ messages in thread From: Jakub Jelinek @ 2023-09-12 7:15 UTC (permalink / raw) To: Tobias Burnus; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic 2023-09-12 7:04 ` [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic Tobias Burnus 2023-09-12 7:15 ` Jakub Jelinek @ 2023-09-12 8:55 ` Tobias Burnus 1 sibling, 0 replies; 15+ messages in thread From: Tobias Burnus @ 2023-09-12 8:55 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 1010 bytes --] 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 [-- Attachment #2: fix-omp-allocate-check.diff --] [-- Type: text/x-patch, Size: 5160 bytes --] 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 %<allocator%> 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 %<allocator%> clause " + "must not be modified between declaration of %qD " + "and its %<allocate%> 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, "%<allocator%> 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, "%<allocator%> 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) == 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 " - "%<allocate%> directive for %qD, must not be " - "modified between declaration of %qD and its " - "%<allocate%> directive", - allocator, var, var); - inform (DECL_SOURCE_LOCATION (var), "declared here"); - inform (OMP_CLAUSE_LOCATION (nl), "used here"); - break; - } - --l; - } - } + struct c_omp_loc_tree data + = {EXPR_LOC_OR_LOC (allocator, OMP_CLAUSE_LOCATION (nl)), var}; + walk_tree (&allocator, c_check_omp_allocate_allocator_r, &data, NULL); } DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"), build_tree_list (allocator, alignment), ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-09-12 8:55 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-29 16:12 [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic Tobias Burnus 2023-08-29 16:28 ` Jakub Jelinek 2023-08-29 16:56 ` Tobias Burnus 2023-08-29 17:14 ` Jakub Jelinek 2023-08-30 10:47 ` Tobias Burnus 2023-08-30 11:13 ` Jakub Jelinek 2023-08-30 12:43 ` Tobias Burnus 2023-09-11 11:44 ` [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic) Tobias Burnus 2023-09-11 11:54 ` Jakub Jelinek 2023-09-11 13:07 ` David Malcolm 2023-09-11 13:21 ` Tobias Burnus 2023-09-11 13:34 ` Jakub Jelinek 2023-09-12 7:04 ` [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic Tobias Burnus 2023-09-12 7:15 ` Jakub Jelinek 2023-09-12 8:55 ` Tobias Burnus
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).