| Message ID | 1445366076-16082-6-git-send-email-amonakov@ispras.ru |
|---|---|
| State | New |
| Headers | show |
On 10/20/2015 08:34 PM, Alexander Monakov wrote: > (note to reviewers: I'm not sure what we're after here, on the high level; > will be happy to rework the patch in a saner manner based on feedback, or even > drop it for now) > > At the moment the attribute setting logic in omp-low.c is such that if a > function that should be present in target code does not already have 'omp > declare target' attribute, it receives 'omp target entrypoint'. That is > wasteful: clearly not all user-declared target functions will be target region > entry points in OpenMP. > > The motivating example for this change is OpenMP parallel target regions. The > 'parallel' part is outlined into its own function. We don't want that > function be an 'entrypoint' on PTX (but only as a matter of optimality rather > than correctness). > > * omp-low.c (create_omp_child_function): Set "omp target entrypoint" > or "omp declare target" attribute based on is_gimple_omp_offloaded. I think this looks reasonable, but you might want to adjust it in whatever way is necessary so that you can drop patch 1/14. Bernd
On Tue, Oct 20, 2015 at 09:34:27PM +0300, Alexander Monakov wrote: > (note to reviewers: I'm not sure what we're after here, on the high level; > will be happy to rework the patch in a saner manner based on feedback, or even > drop it for now) > > At the moment the attribute setting logic in omp-low.c is such that if a > function that should be present in target code does not already have 'omp > declare target' attribute, it receives 'omp target entrypoint'. That is > wasteful: clearly not all user-declared target functions will be target region > entry points in OpenMP. > > The motivating example for this change is OpenMP parallel target regions. The > 'parallel' part is outlined into its own function. We don't want that > function be an 'entrypoint' on PTX (but only as a matter of optimality rather > than correctness). > > * omp-low.c (create_omp_child_function): Set "omp target entrypoint" > or "omp declare target" attribute based on is_gimple_omp_offloaded. This is principally ok, but you want to change it for 01/14. After that I think it is ready for trunk. Jakub
diff --git a/gcc/omp-low.c b/gcc/omp-low.c index 06b4a5e..6481163 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -2242,11 +2242,14 @@ create_omp_child_function (omp_context *ctx, bool task_copy) } } + const char *target_attr = (is_gimple_omp_offloaded (ctx->stmt) + ? "omp target entrypoint" + : "omp declare target"); if (cgraph_node::get_create (decl)->offloadable && !lookup_attribute ("omp declare target", DECL_ATTRIBUTES (current_function_decl))) DECL_ATTRIBUTES (decl) - = tree_cons (get_identifier ("omp target entrypoint"), + = tree_cons (get_identifier (target_attr), NULL_TREE, DECL_ATTRIBUTES (decl)); t = build_decl (DECL_SOURCE_LOCATION (decl),
(note to reviewers: I'm not sure what we're after here, on the high level; will be happy to rework the patch in a saner manner based on feedback, or even drop it for now) At the moment the attribute setting logic in omp-low.c is such that if a function that should be present in target code does not already have 'omp declare target' attribute, it receives 'omp target entrypoint'. That is wasteful: clearly not all user-declared target functions will be target region entry points in OpenMP. The motivating example for this change is OpenMP parallel target regions. The 'parallel' part is outlined into its own function. We don't want that function be an 'entrypoint' on PTX (but only as a matter of optimality rather than correctness). * omp-low.c (create_omp_child_function): Set "omp target entrypoint" or "omp declare target" attribute based on is_gimple_omp_offloaded. --- gcc/omp-low.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)