public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Aldy Hernandez <aldyh@redhat.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [gomp4.1] fix more scheduling inconsistencies and add verification routines
Date: Fri, 09 Oct 2015 16:47:00 -0000	[thread overview]
Message-ID: <20151009164716.GM8714@tucnak.redhat.com> (raw)
In-Reply-To: <5617ED90.9050808@redhat.com>

On Fri, Oct 09, 2015 at 09:38:40AM -0700, Aldy Hernandez wrote:
> As per our IRC discussion.
> 
> I am conditionally compiling the verification code because you mentioned
> that the GPGPUs may not having a working printf.

Both that and code size being important there.

> Also, I removed the code caching the workgroup since it may contain the
> incorrect workgroup as I had suggested.  Now instead we look in
> child_task->taskgroup which will have the correct workgroup always.
> 
> Tested on x86-64 Linux with make check-target-libgomp for a variety of
> different OMP_NUM_THREADS and with _ENABLE_LIBGOMP_CHECKING_ set to 1.
> 
> OK for branch?

Yes, with a small change (just compile test with
-D_ENABLE_LIBGOMP_CHECKING_=1, no need to retest otherwise):

> p.s. As a thought, maybe we can set _ENABLE_LIBGOMP_CHECKING_ to 1 until
> release?

I'd prefer not to, it will affect the timing and slow down already slow
testing of libgomp for everybody.

> --- a/libgomp/task.c
> +++ b/libgomp/task.c
> @@ -27,6 +27,7 @@
>     creation and termination.  */
>  
>  #include "libgomp.h"
> +#include <stdio.h>

Please nuke the above.

> +  if (task->parent != parent)
> +    {
> +      fprintf (stderr, "verify_children_queue: incompatible parents\n");
> +      abort ();
> +    }

and just use
  if (task->parent != parent)
    gomp_fatal ("verify_children_queue: incompatible parents");
instead.  Note no \n at the end.
Ditto for all other fprintf + abort pairs.
gomp_fatal accepts printf style formatting string, so you can even
handle it in:

> +      fprintf (stderr, "verify_taskgroup_queue: incompatible taskgroups\n");
> +      fprintf (stderr, "%p %p\n", task->taskgroup, taskgroup);
> +      abort ();

this case, just use a single fmt string, and just use space instead of \n in
the middle.

	Jakub

      reply	other threads:[~2015-10-09 16:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-04 17:27 Aldy Hernandez
2015-10-04 22:00 ` Aldy Hernandez
2015-10-09 16:38 ` Aldy Hernandez
2015-10-09 16:47   ` Jakub Jelinek [this message]

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=20151009164716.GM8714@tucnak.redhat.com \
    --to=jakub@redhat.com \
    --cc=aldyh@redhat.com \
    --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).