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
