Patchwork Teach alias oracle about threading barriers (PR middle-end/45838)

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 16, 2010, 10:59 p.m.
Message ID <20101116225930.GP29412@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/71470/
State New
Headers show

Comments

Jakub Jelinek - Nov. 16, 2010, 10:59 p.m.
Hi!

pr34513.{c,C} testcases now fail on pa since the additions of leaf
attributes.
We have:
  #pragma omp parallel num_threads (thrs)
  {
    static int shrd = 0;

    #pragma omp atomic
      shrd += 1;

    #pragma omp barrier

    if (shrd != thrs)
      #pragma omp atomic
        errors += 1;
  }
and as pa doesn't have corresponding __sync_ builtin,
GOMP_atomic_{start,end} is used around the shrd increment.
As most of the GOMP_* builtins (those that can't call functions in current
TU) are now marked as leaf, GCC assumes shrd can't be modified by
any of the calls, because it is a local static without address taken.
But multiple threads can (and in this case do) modify the variable, so
we can't cache its value across the barriers.

__sync_* builtins are documented as barriers (most of them full memory
barriers) and several GOMP_* functions are also barrier points.

Fixed by teaching the alias oracle that threading barriers conflict
with any refs that may_be_aliased, bootstrapped/regtested on x86_64-linux
and i686-linux and tested on the testcase with a cross to hppa.  Ok for
trunk?

2010-11-16  Jakub Jelinek  <jakub@redhat.com>
	    Richard Guenther  <rguenther@suse.de>

	PR middle-end/45838
	* tree-ssa-alias.c (ref_maybe_used_by_call_p_1,
	call_may_clobber_ref_p_1): Return true for __sync_* and some
	OpenMP builtins that act as threading barriers.



	Jakub
Richard Guenther - Nov. 17, 2010, 10:53 a.m.
On Tue, Nov 16, 2010 at 11:59 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> pr34513.{c,C} testcases now fail on pa since the additions of leaf
> attributes.
> We have:
>  #pragma omp parallel num_threads (thrs)
>  {
>    static int shrd = 0;
>
>    #pragma omp atomic
>      shrd += 1;
>
>    #pragma omp barrier
>
>    if (shrd != thrs)
>      #pragma omp atomic
>        errors += 1;
>  }
> and as pa doesn't have corresponding __sync_ builtin,
> GOMP_atomic_{start,end} is used around the shrd increment.
> As most of the GOMP_* builtins (those that can't call functions in current
> TU) are now marked as leaf, GCC assumes shrd can't be modified by
> any of the calls, because it is a local static without address taken.
> But multiple threads can (and in this case do) modify the variable, so
> we can't cache its value across the barriers.
>
> __sync_* builtins are documented as barriers (most of them full memory
> barriers) and several GOMP_* functions are also barrier points.
>
> Fixed by teaching the alias oracle that threading barriers conflict
> with any refs that may_be_aliased, bootstrapped/regtested on x86_64-linux
> and i686-linux and tested on the testcase with a cross to hppa.  Ok for
> trunk?

Ok.

Thanks,
Richard.

> 2010-11-16  Jakub Jelinek  <jakub@redhat.com>
>            Richard Guenther  <rguenther@suse.de>
>
>        PR middle-end/45838
>        * tree-ssa-alias.c (ref_maybe_used_by_call_p_1,
>        call_may_clobber_ref_p_1): Return true for __sync_* and some
>        OpenMP builtins that act as threading barriers.
>
> --- gcc/tree-ssa-alias.c.jj     2010-11-01 09:07:22.000000000 +0100
> +++ gcc/tree-ssa-alias.c        2010-11-16 14:33:21.000000000 +0100
> @@ -1209,6 +1209,28 @@ ref_maybe_used_by_call_p_1 (gimple call,
>        case BUILT_IN_SINCOSF:
>        case BUILT_IN_SINCOSL:
>          return false;
> +       /* __sync_* builtins and some OpenMP builtins act as threading
> +          barriers.  */
> +#undef DEF_SYNC_BUILTIN
> +#define DEF_SYNC_BUILTIN(ENUM, NAME, TYPE, ATTRS) case ENUM:
> +#include "sync-builtins.def"
> +#undef DEF_SYNC_BUILTIN
> +       case BUILT_IN_GOMP_ATOMIC_START:
> +       case BUILT_IN_GOMP_ATOMIC_END:
> +       case BUILT_IN_GOMP_BARRIER:
> +       case BUILT_IN_GOMP_TASKWAIT:
> +       case BUILT_IN_GOMP_CRITICAL_START:
> +       case BUILT_IN_GOMP_CRITICAL_END:
> +       case BUILT_IN_GOMP_CRITICAL_NAME_START:
> +       case BUILT_IN_GOMP_CRITICAL_NAME_END:
> +       case BUILT_IN_GOMP_LOOP_END:
> +       case BUILT_IN_GOMP_ORDERED_START:
> +       case BUILT_IN_GOMP_ORDERED_END:
> +       case BUILT_IN_GOMP_PARALLEL_END:
> +       case BUILT_IN_GOMP_SECTIONS_END:
> +       case BUILT_IN_GOMP_SINGLE_COPY_START:
> +       case BUILT_IN_GOMP_SINGLE_COPY_END:
> +         return true;
>
>        default:
>          /* Fallthru to general call handling.  */;
> @@ -1465,6 +1487,28 @@ call_may_clobber_ref_p_1 (gimple call, a
>            return (ptr_deref_may_alias_ref_p_1 (sin, ref)
>                    || ptr_deref_may_alias_ref_p_1 (cos, ref));
>          }
> +       /* __sync_* builtins and some OpenMP builtins act as threading
> +          barriers.  */
> +#undef DEF_SYNC_BUILTIN
> +#define DEF_SYNC_BUILTIN(ENUM, NAME, TYPE, ATTRS) case ENUM:
> +#include "sync-builtins.def"
> +#undef DEF_SYNC_BUILTIN
> +       case BUILT_IN_GOMP_ATOMIC_START:
> +       case BUILT_IN_GOMP_ATOMIC_END:
> +       case BUILT_IN_GOMP_BARRIER:
> +       case BUILT_IN_GOMP_TASKWAIT:
> +       case BUILT_IN_GOMP_CRITICAL_START:
> +       case BUILT_IN_GOMP_CRITICAL_END:
> +       case BUILT_IN_GOMP_CRITICAL_NAME_START:
> +       case BUILT_IN_GOMP_CRITICAL_NAME_END:
> +       case BUILT_IN_GOMP_LOOP_END:
> +       case BUILT_IN_GOMP_ORDERED_START:
> +       case BUILT_IN_GOMP_ORDERED_END:
> +       case BUILT_IN_GOMP_PARALLEL_END:
> +       case BUILT_IN_GOMP_SECTIONS_END:
> +       case BUILT_IN_GOMP_SINGLE_COPY_START:
> +       case BUILT_IN_GOMP_SINGLE_COPY_END:
> +         return true;
>        default:
>          /* Fallthru to general call handling.  */;
>       }
>
>
>        Jakub
>

Patch

--- gcc/tree-ssa-alias.c.jj	2010-11-01 09:07:22.000000000 +0100
+++ gcc/tree-ssa-alias.c	2010-11-16 14:33:21.000000000 +0100
@@ -1209,6 +1209,28 @@  ref_maybe_used_by_call_p_1 (gimple call,
 	case BUILT_IN_SINCOSF:
 	case BUILT_IN_SINCOSL:
 	  return false;
+	/* __sync_* builtins and some OpenMP builtins act as threading
+	   barriers.  */
+#undef DEF_SYNC_BUILTIN
+#define DEF_SYNC_BUILTIN(ENUM, NAME, TYPE, ATTRS) case ENUM:
+#include "sync-builtins.def"
+#undef DEF_SYNC_BUILTIN
+	case BUILT_IN_GOMP_ATOMIC_START:
+	case BUILT_IN_GOMP_ATOMIC_END:
+	case BUILT_IN_GOMP_BARRIER:
+	case BUILT_IN_GOMP_TASKWAIT:
+	case BUILT_IN_GOMP_CRITICAL_START:
+	case BUILT_IN_GOMP_CRITICAL_END:
+	case BUILT_IN_GOMP_CRITICAL_NAME_START:
+	case BUILT_IN_GOMP_CRITICAL_NAME_END:
+	case BUILT_IN_GOMP_LOOP_END:
+	case BUILT_IN_GOMP_ORDERED_START:
+	case BUILT_IN_GOMP_ORDERED_END:
+	case BUILT_IN_GOMP_PARALLEL_END:
+	case BUILT_IN_GOMP_SECTIONS_END:
+	case BUILT_IN_GOMP_SINGLE_COPY_START:
+	case BUILT_IN_GOMP_SINGLE_COPY_END:
+	  return true;
 
 	default:
 	  /* Fallthru to general call handling.  */;
@@ -1465,6 +1487,28 @@  call_may_clobber_ref_p_1 (gimple call, a
 	    return (ptr_deref_may_alias_ref_p_1 (sin, ref)
 		    || ptr_deref_may_alias_ref_p_1 (cos, ref));
 	  }
+	/* __sync_* builtins and some OpenMP builtins act as threading
+	   barriers.  */
+#undef DEF_SYNC_BUILTIN
+#define DEF_SYNC_BUILTIN(ENUM, NAME, TYPE, ATTRS) case ENUM:
+#include "sync-builtins.def"
+#undef DEF_SYNC_BUILTIN
+	case BUILT_IN_GOMP_ATOMIC_START:
+	case BUILT_IN_GOMP_ATOMIC_END:
+	case BUILT_IN_GOMP_BARRIER:
+	case BUILT_IN_GOMP_TASKWAIT:
+	case BUILT_IN_GOMP_CRITICAL_START:
+	case BUILT_IN_GOMP_CRITICAL_END:
+	case BUILT_IN_GOMP_CRITICAL_NAME_START:
+	case BUILT_IN_GOMP_CRITICAL_NAME_END:
+	case BUILT_IN_GOMP_LOOP_END:
+	case BUILT_IN_GOMP_ORDERED_START:
+	case BUILT_IN_GOMP_ORDERED_END:
+	case BUILT_IN_GOMP_PARALLEL_END:
+	case BUILT_IN_GOMP_SECTIONS_END:
+	case BUILT_IN_GOMP_SINGLE_COPY_START:
+	case BUILT_IN_GOMP_SINGLE_COPY_END:
+	  return true;
 	default:
 	  /* Fallthru to general call handling.  */;
       }