| Message ID | 20101116225930.GP29412@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
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 >
--- 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. */; }
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