public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jim Wilson <jim.wilson@linaro.org>
To: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>, fortran@gcc.gnu.org
Subject: [PATCH, libgfortran] proposed fix for SPEC CPU2017 621.wrf_s failures
Date: Mon, 26 Jun 2017 00:50:00 -0000	[thread overview]
Message-ID: <CABXYE2W97i1hxp=o17wjjezekDUzonLM3DjP2aHOfKMQd5=zSg@mail.gmail.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

As mentioned in bug 81195, I see openmp related failures due to a lack
of locking of the newunit_stack and newunit_tos variables.  The code
locks when pushing onto the stack, but does not lock when popping from
the stack.  This can cause multiple threads to pop the same structure,
which then eventually leads to an error.

This patch was tested with an aarch64 bootstrap and make check.  There
were no regressions.  It was also tested with a SPEC CPU2017 run, and
solves the 621.wrf_s failure I get without the patch.

gcc 7 has the same problem.  gcc 6 is OK.

Jim

[-- Attachment #2: newunit-locking.patch --]
[-- Type: text/x-patch, Size: 1513 bytes --]

	libgfortran/
	PR libfortran/81195
	* io/unit.c (get_unit): Call __gthread_mutex_lock before newunit_stack
	and newunit_tos references.  Call __gthread_mutex_unlock afterward.

Index: libgfortran/io/unit.c
===================================================================
--- libgfortran/io/unit.c	(revision 249613)
+++ libgfortran/io/unit.c	(working copy)
@@ -583,14 +583,17 @@
 	}
       else
 	{
+	  __gthread_mutex_lock (&unit_lock);
 	  if (newunit_tos)
 	    {
 	      dtp->common.unit = newunit_stack[newunit_tos].unit_number;
 	      unit = newunit_stack[newunit_tos--].unit;
+	      __gthread_mutex_unlock (&unit_lock);
 	      unit->fbuf->act = unit->fbuf->pos = 0;
 	    }
 	  else
 	    {
+	      __gthread_mutex_unlock (&unit_lock);
 	      dtp->common.unit = newunit_alloc ();
 	      unit = xcalloc (1, sizeof (gfc_unit));
 	      fbuf_init (unit, 128);
@@ -603,12 +606,15 @@
   /* If an internal unit number is passed from the parent to the child
      it should have been stashed on the newunit_stack ready to be used.
      Check for it now and return the internal unit if found.  */
+  __gthread_mutex_lock (&unit_lock);
   if (newunit_tos && (dtp->common.unit <= NEWUNIT_START)
       && (dtp->common.unit == newunit_stack[newunit_tos].unit_number))
     {
       unit = newunit_stack[newunit_tos--].unit;
+      __gthread_mutex_unlock (&unit_lock);
       return unit;
     }
+  __gthread_mutex_unlock (&unit_lock);
 
   /* Has to be an external unit.  */
   dtp->u.p.unit_is_internal = 0;

             reply	other threads:[~2017-06-26  0:50 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-26  0:50 Jim Wilson [this message]
2017-06-26  4:14 ` Jerry DeLisle

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='CABXYE2W97i1hxp=o17wjjezekDUzonLM3DjP2aHOfKMQd5=zSg@mail.gmail.com' \
    --to=jim.wilson@linaro.org \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    /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).