public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andre Vehreschild <vehre@gmx.de>
To: Tobias Burnus <tobias@codesourcery.com>
Cc: Jakub Jelinek <jakub@redhat.com>,
	gcc-patches <gcc-patches@gcc.gnu.org>,
	fortran <fortran@gcc.gnu.org>
Subject: Re: [Patch] OpenMP/Fortran: Handle polymorphic scalars in data-sharing FIRSTPRIVATE (PR86470)
Date: Mon, 31 Aug 2020 12:55:07 +0200	[thread overview]
Message-ID: <20200831125507.3affd4af@vepi2> (raw)
In-Reply-To: <8bcd00c3-9de5-c072-d893-9218a08a1c2f@codesourcery.com>

Hi Tobias,

in (look for ^^^):

diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 063d4c145e2..705cdc7749f 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -355,6 +355,51 @@ gfc_has_alloc_comps (tree type, tree decl)
   return false;
 }

+/* Return true if TYPE is polymorphic but not with pointer attribute.  */
+
+static bool
+gfc_is_polymorphic_nonptr (tree type)
+{
+  if (POINTER_TYPE_P (type))
+    type = TREE_TYPE (type);
+  if (TREE_CODE (type) != RECORD_TYPE)
+    return false;
+
+  tree field = TYPE_FIELDS (type);
+  if (!field || 0 != strcmp ("_data", IDENTIFIER_POINTER (DECL_NAME (field))))

^^^ here you are comparing the field - name

+    return false;
+  field = DECL_CHAIN (field);
+  if (!field || 0 != strcmp ("_vptr", IDENTIFIER_POINTER (DECL_NAME (field))))
+    return false;
+
+  return true;
+}
+
+/* Return true if TYPE is unlimited polymorphic but not with pointer attribute;
+   unlimited means also intrinsic types are handled and _len is used.  */
+
+static bool
+gfc_is_unlimited_polymorphic_nonptr (tree type)
+{
+  if (POINTER_TYPE_P (type))
+    type = TREE_TYPE (type);
+  if (TREE_CODE (type) != RECORD_TYPE)
+    return false;
+
+  tree field = TYPE_FIELDS (type); /* _data */
+  if (!field)

^^^ here you don't . So theoretically this routine could match a type which
has a _len as its third field, but that is not a unlim-poly class. Maybe factor
out the test from the above routine and unify with this one to reuse the test
for a BT_CLASS?!

+    return false;
+  field = DECL_CHAIN (field); /* _vptr */
+  if (!field)
+    return false;
+  field = DECL_CHAIN (field);
+  if (!field || 0 != strcmp ("_len", IDENTIFIER_POINTER (DECL_NAME (field))))
+    return false;
+
+  return true;
+}
+
+

---------------------------------------------------------------------------------------------------------------

Btw, I believe the first routine can be better replaced by:

static bool
gfc_is_polymorphic_nonptr (tree type)
{
  if (POINTER_TYPE_P (type))
    type = TREE_TYPE (type);
  return GFC_CLASS_TYPE_P (type);
}

I have no better solution for learning whether a tree's type is unlimited poly
yet.


@@ -740,6 +785,87 @@ gfc_omp_clause_copy_ctor (tree clause, tree dest, tree src)
   gcc_assert (OMP_CLAUSE_CODE (clause) == OMP_CLAUSE_FIRSTPRIVATE
 	      || OMP_CLAUSE_CODE (clause) == OMP_CLAUSE_LINEAR);

+  /* TODO: implement support for polymorphic arrays; reject for now.  */
+  /* Void arrays appear as var.0 = var._data.data. A bit hackish to
+     distinguish from 'type(c_ptr) :: var(5)' by scanning for '.';
+     this assumes that ASM_FORMAT_PRIVATE_NAME uses a '.', which most
+     systems do. */
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && TREE_TYPE (type) == pvoid_type_node
+      && TREE_CODE (dest) == MEM_REF
+      && strchr (IDENTIFIER_POINTER (DECL_NAME (TREE_OPERAND (dest, 0))), '.'))
+    fatal_error (input_location,
+		 "Sorry, polymorphic arrays not yet supported for "
+		 "firstprivate");

I totally agree that this is hackish and I don't like for that. But I can't
come up with a better solution at the moment.

The remainder looks ok to me.

Regards,
	Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de

  parent reply	other threads:[~2020-08-31 10:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-25 10:50 Tobias Burnus
2020-08-31  8:28 ` Tobias Burnus
2020-08-31 10:55 ` Andre Vehreschild [this message]
2020-08-31 13:58   ` Tobias Burnus
2020-08-31 16:34 ` Jakub Jelinek
2021-03-10 10:55 [Patch] OpenMP/Fortran: Handle polymorphic scalars in data-sharing FIRSTPRIVATE [PR86470] Tobias Burnus
2021-05-23  9:47 ` Tobias Burnus
2021-05-24 14:06 ` Jakub Jelinek

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=20200831125507.3affd4af@vepi2 \
    --to=vehre@gmx.de \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=tobias@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).