Re: [PATCH] Teach alias oracle about threading barriers (PR middle-end/45838)

[Richard Guenther <richard dot guenther at gmail dot com>]

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
>