From: Tobias Burnus <tobias@codesourcery.com>
To: Julian Brown <julian@codesourcery.com>
Cc: <gcc-patches@gcc.gnu.org>, <fortran@gcc.gnu.org>,
<jakub@redhat.com>, <thomas@codesourcery.com>
Subject: Re: [PATCH] OpenACC: Further attach/detach clause fixes for Fortran [PR109622]
Date: Wed, 3 May 2023 15:50:49 +0200 [thread overview]
Message-ID: <b8682c4e-d5a5-d085-71c4-11ee3c1a20f9@codesourcery.com> (raw)
In-Reply-To: <20230503135956.1ec395a2@squid.athome>
On 03.05.23 14:59, Julian Brown wrote:
> How does this version look?
> Retested with offloading to nvptx.
LGTM (for mainline + GCC 13 backporting) but I have two tiny comments:
> diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
> index 86e4515..322856a 100644
> --- a/gcc/fortran/openmp.cc
> +++ b/gcc/fortran/openmp.cc
> @@ -7711,6 +7711,23 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
> &n->where);
> }
> }
> + if (openacc
> + && list == OMP_LIST_MAP
> + && (n->u.map_op == OMP_MAP_ATTACH
> + || n->u.map_op == OMP_MAP_DETACH))
> + {
> + symbol_attribute attr;
> + gfc_clear_attr (&attr);
> + if (n->expr)
> + attr = gfc_expr_attr (n->expr);
> + else if (n->sym)
> + attr = n->sym->attr;
Note that n->sym == NULL is only the case if the argument was
omp_all_memory (→ gfc_match_omp_variable_list); that can only be the
case for OMP_CLAUSE_DEPEND.
As OpenMP's 'depend' clause is handled before and you additionally deal
with OpenACC, only, you could just use 'else' instead of 'else if
(n->sym)' – and also get rid of the 'gfc_clear_attr' as the values get
overwritten by the assignment and by the function call.
Later code (e.g. line 7785 in the current code) also assumes (for
OpenACC + MAP) that n->sym != NULL by bluntly dereferencing it.
@@ -3523,15 +3525,20 @@ gfc_trans_omp_clauses (stmtblock_t *block,
gfc_omp_clauses *clauses,
> if (n->u.map_op == OMP_MAP_ATTACH
> || n->u.map_op == OMP_MAP_DETACH)
> {
> - if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (inner)))
> + if (POINTER_TYPE_P (TREE_TYPE (inner))
> + || GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (inner)))
> {
...
> }
> - else
> - OMP_CLAUSE_DECL (node) = inner;
> - OMP_CLAUSE_SIZE (node) = size_zero_node;
> - goto finalize_map_clause;
> }
You can now combine the two if conditions, which avoids some indenting
and should permit to put 'tree ptr' / ' = ...' again on the same line.
Thanks for the 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
next prev parent reply other threads:[~2023-05-03 13:51 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <138a36f7-8b1e-5955-1d3a-5713a0fcf5b6@univ-grenoble-alpes.fr>
2023-04-27 18:36 ` [PATCH] OpenACC: Stand-alone " Julian Brown
2023-04-28 8:16 ` Tobias Burnus
2023-04-28 12:56 ` Thomas Schwinge
2023-04-29 10:57 ` [PATCH] OpenACC: Further " Julian Brown
2023-05-02 10:29 ` Tobias Burnus
2023-05-03 12:59 ` Julian Brown
2023-05-03 13:50 ` Tobias Burnus [this message]
2023-05-03 11:29 ` Thomas Schwinge
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b8682c4e-d5a5-d085-71c4-11ee3c1a20f9@codesourcery.com \
--to=tobias@codesourcery.com \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=julian@codesourcery.com \
--cc=thomas@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).