Re: Fix PR41212 (miscompilation)

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

On Tue, Sep 15, 2009 at 5:16 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> so, using a completely new type for variables not marked with the target
> attribute doesn't really work in the presence of address taking and
> restrict pointers. ?The latter happens trivially with inlining. ?The
> result is that accesses over the restrict pointer and direct accesses to
> the decl don't conflict anymore -> boom.
>
> Hence, this patch moves the disambiguation that we really really want
> (nontarget variables can't be aliased via most pointers) to the oracle
> itself. ?Instead of using type based aliasing for disambiguation (that
> idea looked so nice :-/ ) we instead mark the decl with a flag and use
> that in the oracle to check for conflicts of restrict pointers vs. such
> decls. ?A restrict pointer can only alias such marked decl if that decl is
> in the points-to set of that very pointer (in effect that same mechanism
> we use to determine the based-on predicate for two restrict pointers).
>
> Accordingly we don't need the nontarget_type thing in the fortran frontend
> anymore, just have to set that bit.
>
> Regstrapped on x86_64-linux (all langs+Ada), no regressions (in particular
> this means that the important loop in gems still is vectorized). ?Okay for
> trunk?

Ok with ...

===================================================================
> --- tree-ssa-alias.c ? ?(revision 151719)
> +++ tree-ssa-alias.c ? ?(working copy)
> @@ -208,6 +208,14 @@ ptr_deref_may_alias_decl_p (tree ptr, tr
> ? if (!pi)
> ? ? return true;
>
> + ?/* If the decl can be used as a restrict tag and we have a restrict
> + ? ? pointer and that pointers points-to set doesn't contain this decl
> + ? ? then they can't alias. ?*/
> + ?if (DECL_RESTRICTED_P (decl)
> + ? ? ?&& TYPE_RESTRICT (TREE_TYPE (ptr))
> + ? ? ?&& pi->pt.vars)
> + ? ?return bitmap_bit_p (pi->pt.vars, DECL_UID (decl));

also checking pi->pt.vars_contains_restrict (in fact you can
substitute the pi->pt.vars check with that).

Thanks,
Richard.

> +
> ? return pt_solution_includes (&pi->pt, decl);
> ?}
>
> Index: fortran/trans.c
> ===================================================================
> --- fortran/trans.c ? ? (revision 151719)
> +++ fortran/trans.c ? ? (working copy)
> @@ -162,13 +162,6 @@ gfc_add_modify (stmtblock_t * pblock, tr
> ? tree t1, t2;
> ? t1 = TREE_TYPE (rhs);
> ? t2 = TREE_TYPE (lhs);
> - ?/* ??? This is actually backwards, we should test the "base" type
> - ? ? from which the nontarget_type was copied, but we don't have this
> - ? ? backlink. ?This will do for now, it's for checking anyway. ?*/
> - ?if (TYPE_LANG_SPECIFIC (t1))
> - ? ?t1 = TYPE_LANG_SPECIFIC (t1)->nontarget_type;
> - ?if (TYPE_LANG_SPECIFIC (t2))
> - ? ?t2 = TYPE_LANG_SPECIFIC (t2)->nontarget_type;
> ? /* Make sure that the types of the rhs and the lhs are the same
> ? ? ?for scalar assignments. ?We should probably have something
> ? ? ?similar for aggregates, but right now removing that check just
> Index: fortran/trans.h
> ===================================================================
> --- fortran/trans.h ? ? (revision 151719)
> +++ fortran/trans.h ? ? (working copy)
> @@ -629,7 +629,6 @@ struct GTY(()) ? ? ?lang_type ? ? ? ?{
> ? tree dataptr_type;
> ? tree span;
> ? tree base_decl[2];
> - ?tree nontarget_type;
> ?};
>
> ?struct GTY(()) lang_decl {
> Index: fortran/trans-decl.c
> ===================================================================
> --- fortran/trans-decl.c ? ? ? ?(revision 151719)
> +++ fortran/trans-decl.c ? ? ? ?(working copy)
> @@ -581,26 +581,8 @@ gfc_finish_var_decl (tree decl, gfc_symb
>
> ? if (!sym->attr.target
> ? ? ? && !sym->attr.pointer
> - ? ? ?&& !sym->attr.proc_pointer
> - ? ? ?/* For now, don't bother with aggregate types. ?We would need
> - ? ? ? ? to adjust DECL_CONTEXT of all field decls. ?*/
> - ? ? ?&& !AGGREGATE_TYPE_P (TREE_TYPE (decl)))
> - ? ?{
> - ? ? ?tree type = TREE_TYPE (decl);
> - ? ? ?if (!TYPE_LANG_SPECIFIC (type))
> - ? ? ? TYPE_LANG_SPECIFIC (type) = (struct lang_type *)
> - ? ? ? ? ggc_alloc_cleared (sizeof (struct lang_type));
> - ? ? ?if (!TYPE_LANG_SPECIFIC (type)->nontarget_type)
> - ? ? ? {
> - ? ? ? ? alias_set_type set = new_alias_set ();
> - ? ? ? ? type = build_distinct_type_copy (type);
> - ? ? ? ? TYPE_ALIAS_SET (type) = set;
> - ? ? ? ? TYPE_LANG_SPECIFIC (type)->nontarget_type = type;
> - ? ? ? }
> - ? ? ?else
> - ? ? ? type = TYPE_LANG_SPECIFIC (type)->nontarget_type;
> - ? ? ?TREE_TYPE (decl) = type;
> - ? ?}
> + ? ? ?&& !sym->attr.proc_pointer)
> + ? ?DECL_RESTRICTED_P (decl) = 1;
> ?}
>
>
> Index: testsuite/gfortran.dg/pr41212.f90
> ===================================================================
> --- testsuite/gfortran.dg/pr41212.f90 ? (revision 0)
> +++ testsuite/gfortran.dg/pr41212.f90 ? (revision 0)
> @@ -0,0 +1,34 @@
> +! { dg-do run }
> +! { dg-options "-O2" }
> +program m
> + ? double precision :: y,z
> + ? call b(1.0d0,y,z)
> + ? if (ABS (z - 1.213) > 0.1) call abort
> +contains
> + ? ? subroutine b( x, y, z)
> + ? ? ? implicit none
> + ? ? ? double precision :: x,y,z
> + ? ? ? integer :: i, k
> + ? ? ? double precision :: h, r
> +
> + ? ? ? y = 1.0d0
> + ? ? ? z = 0.0d0
> +
> + ? ? ? h = 0
> + ? ? ? DO k = 1,10
> + ? ? ? ? ?h = h + 1.0d0/k
> +
> + ? ? ? ? ?r = 1
> + ? ? ? ? ?DO i = 1,k
> + ? ? ? ? ? ? r = (x/(2*i) ) * r
> + ? ? ? ? ?END DO
> +
> + ? ? ? ? ?y = y + (-1)**k * r
> + ? ? ? ? ?z = z + (-1)**(k+1) * h * r
> +
> + ? ? ? ? ?IF ( ABS(2*k/x*r) < 1d-6 ) EXIT
> + ? ? ? END DO
> +
> + ? ? ? z = 2*y
> + ? ? end subroutine b
> +end program m
>