Hi Thomas,
On 2023/6/23 6:47 PM, Thomas Schwinge wrote:
>> +
>> ctx->clauses = *orig_list_p;
>> gimplify_omp_ctxp = ctx;
>> }
> Instead of this, in 'gimplify_omp_workshare', before the
> 'gimplify_scan_omp_clauses' call, do something like:
>
> if ((ort & ORT_ACC)
> && !omp_find_clause (OMP_CLAUSES (expr), OMP_CLAUSE_DEFAULT))
> {
> /* Determine effective 'default' clause for OpenACC compute
> construct. */
> for (struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp; ctx; ctx =
> ctx->outer_context)
> {
> if (ctx->region_type == ORT_ACC_DATA
> && ctx->default_kind != OMP_CLAUSE_DEFAULT_SHARED)
> {
> [Append actual default clause on compute construct.]
> break;
> }
> }
> }
>
> That seems conceptually simpler to me?
I'm not sure if this is conceptually simpler, but using 'oacc_default_kind'
is definitely faster computationally :)
However, as you mention below...
> For the 'build_omp_clause', does using 'ctx->location' instead of
> 'UNKNOWN_LOCATION' help diagnostics in any way? Like if we add in
> 'gcc/gimplify.cc:oacc_default_clause',
> 'if (ctx->default_kind == OMP_CLAUSE_DEFAULT_NONE)' another 'inform' to
> point to the 'data' construct's 'default' clause? (But not sure if
> that's easily done; otherwise don't.)
Noticed that we will need to track the actually lexically enclosing OpenACC
construct
with the user set default-clause somewhere in 'ctx', in order to satisfy the
current
diagnostics in oacc_default_clause().
(the UNKNOWN_LOCATION for the internally created default-clause probably doesn't
matter, that one is just for reminder in internal dumps, probably never plays
role
in user diagnostics)
> Similar to the ones you've already got, please also add a few test cases
> for nested 'default' clauses, like:
>
> #pragma acc data // no vs. 'default(none)' vs. 'default(present)'
> {
> #pragma acc data // no vs. same vs. different 'default' clause
> {
> #pragma acc data // no vs. same vs. different 'default' clause
> {
> #pragma acc parallel
>
> Similarly, test cases where 'default' on the compute construct overrides
> 'default' of an outer 'data' construct.
Okay, will add more testcases.
Thanks,
Chung-Lin