[PATCH] Fix fortran libgomp failures

[Richard Guenther]

On Nov 27, 2007 10:32 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sun, Nov 25, 2007 at 11:04:24AM -0400, Aldy Hernandez wrote:
> > I've found what the cause of the problem reported below, but I am unsure
> > as to how to fix it.  Perhaps Jakub can shed some light.
> >
> > With the patch below, the type of the P_P_E is now:
> >
> >     type <reference_type 0x2aaaaab90240
> >         type <array_type 0x2aaaaab90180 type ...
> >
> > gfc_omp_privatize_by_reference is now being called with a temporary of
> > the above type.  This function returns TRUE for any REFERENCE_TYPE.
> > Without the patch, it obviously returned pchar_type_node which returned
> > false.
> >
> > My naive solution is to use PTR_TYPE_NODE instead of TREE_TYPE(dest) in
> > my original patch, but Richard G had commented that we should ideally be
> > type-exact in the expression tree.  Do folks care if I make this
> > PTR_TYPE_NODE, or is a fix in gfc_omp_privatize_by_reference in order?
>
> Here is a patch that does both.  It is just plain wrong to use
> say REFERENCE_TYPE in the memset call, we have there a pointer/reference
> to some array and we do
> memmove (dest, src, slen);
> memset (dest + slen, ' ', dlen - slen);
> but dest + slen arithmetics is not adding some multiple of the *dest
> sizes, but just some number of bytes.  So, either we should use
> pchar_type_node, or pvoid_type_node.  I chose the latter because memmove and
> memset arguments have void * types.

I don't think there is anything wrong with using a REFERENCE_TYPE to
some array in this case (arithmetics always operate in bytes, not elements),
but obviously using void * types also works here (and is completely equivalent,
you'll see the casts removed as useless and the original pointer types
propagated)

> REFERENCE_TYPEs emitted by gfortran FE so far were always what
> gfc_omp_privatize_by_reference expected - parameters or results (passed
> indirectly, so parameters again), so just returning true always was ok.
> With the first hunk no change is needed either, but just to make Richi
> happy I've changed it as well to make sure REFERENCE_TYPE is only special
> on parameters (which can be PARM_DECL, or e.g. for return types VAR_DECL),
> some PARM_DECL have DECL_ARTIFICIAL set, fortunately all VAR_DECLs which
> need this treatment are not DECL_ARTIFICIAL.
>
> Regtested on x86_64-linux, ok for trunk?

Fine with me, even if I think the trans-expr.c part is unneeded.

Richard.

> 2007-11-27  Jakub Jelinek  <jakub@redhat.com>
>
>         * trans-expr.c (gfc_trans_string_copy): Convert both dest and
>         src to void *.
>
>         * trans-openmp.c (gfc_omp_privatize_by_reference): For REFERENCE_TYPE
>         pass by reference only PARM_DECLs or non-artificial decls.
>
> --- gcc/fortran/trans-expr.c.jj 2007-11-26 11:02:23.000000000 +0100
> +++ gcc/fortran/trans-expr.c    2007-11-27 09:04:31.000000000 +0100
> @@ -2708,7 +2708,10 @@ gfc_trans_string_copy (stmtblock_t * blo
>
>       We're now doing it here for better optimization, but the logic
>       is the same.  */
> -
> +
> +  dest = fold_convert (pvoid_type_node, dest);
> +  src = fold_convert (pvoid_type_node, src);
> +
>    /* Truncate string if source is too long.  */
>    cond2 = fold_build2 (GE_EXPR, boolean_type_node, slen, dlen);
>    tmp2 = build_call_expr (built_in_decls[BUILT_IN_MEMMOVE],
> --- gcc/fortran/trans-openmp.c.jj       2007-08-27 10:15:33.000000000 +0200
> +++ gcc/fortran/trans-openmp.c  2007-11-27 09:39:40.000000000 +0100
> @@ -44,7 +44,8 @@ gfc_omp_privatize_by_reference (const_tr
>  {
>    tree type = TREE_TYPE (decl);
>
> -  if (TREE_CODE (type) == REFERENCE_TYPE)
> +  if (TREE_CODE (type) == REFERENCE_TYPE
> +      && (!DECL_ARTIFICIAL (decl) || TREE_CODE (decl) == PARM_DECL))
>      return true;
>
>    if (TREE_CODE (type) == POINTER_TYPE)
>
>         Jakub
>