On 3/18/22 15:56, Jakub Jelinek wrote:
On Fri, Mar 18, 2022 at 03:42:48PM +0100, Tom de Vries wrote:
And for NVPTX we somehow lower the taskloop into GIMPLE_ASM
or how we end up ICEing?
In the nvptx backend, gen_comment (triggering not very frequently atm) uses
gen_rtx_ASM_INPUT_loc with as location argument DECL_SOURCE_LOCATION
(cfun->decl).
Ok.
Alternatively, if there's a better way to get some random valid location
than DECL_SOURCE_LOCATION (cfun->decl), that would also work for me. ]
No objection against doing that, but if we do it, we should probably do it
for all or at least most gimple_build_omp_* calls, not just these 2.
So in gimplify_omp_parallel, gimplify_omp_task, another spot in
gimplify_omp_for beyond these 2, gimplify_omp_workshare (ideally just
in one spot for all the cases), gimplify_omp_target_update,
gimplify_omp_atomic, gimplify_omp_ordered, gimplify_expr's
case OMP_* that call gimple_build_omp_*.
Or is it normally handled using
if (!gimple_seq_empty_p (internal_post))
{
annotate_all_with_location (internal_post, input_location);
gimplify_seq_add_seq (pre_p, internal_post);
}
and we just need to catch the cases where we gimplify something into
multiple nested stmts because annotate_all_with_location doesn't
walk into gimple_omp_body?
I can try to update the patch to take care of these additional cases.
I reckon answering the questions that you're asking requires writing
test-cases for all of these.
Actually, in the light of annotate_all_with_location annotating
the newly generated sequence except for the stmts in nested contexts
I think only the two spots you have in your patch is what needs adjusting.
But I'd do it only when actually dealing with a OMP_TASKLOOP, so both
in the spot of your second hunk and for consistency with the
annotate_all_with_location do there (pseudo patch):
+ gimple_set_location (gfor, input_location);
g = gimple_build_bind (NULL_TREE, gfor, NULL_TREE);
g = gimple_build_omp_task (g, task_clauses, NULL_TREE, NULL_TREE,
NULL_TREE, NULL_TREE, NULL_TREE);
gimple_omp_task_set_taskloop_p (g, true);
+ gimple_set_location (g, input_location);
g = gimple_build_bind (NULL_TREE, g, NULL_TREE);
gomp_for *gforo
= gimple_build_omp_for (g, GF_OMP_FOR_KIND_TASKLOOP,
outer_for_clauses,
gimple_omp_for_collapse (gfor),
gimple_omp_for_pre_body (gfor));
gimple_omp_for_set_pre_body (gfor, NULL);
gimple_omp_for_set_combined_p (gforo, true);
gimple_omp_for_set_combined_into_p (gfor, true);
In theory we could do it for the gimple_build_bind results too, but we don't
do that in other spots where we gimple_build_bind in OpenMP/OpenACC related
gimplification.
Ok for trunk with those tweaks.
Ack, committed (in two steps though, I accidentally first committed the
old patch).
Thanks,
- Tom
