fortran: Fix up ISO_Fortran_binding_15.f90 failures [PR92123]

Hi!

This is something that has been discussed already a few months ago, but
seems to have stalled.  Here is Paul's patch from the PR except for the
TREE_STATIC hunk which is wrong, and does the most conservative fn spec
tweak for the problematic two builtins we are aware of (to repeat what is in
the PR, both .wR and .ww are wrong for these builtins that transform one
layout of an descriptor to another one; while the first pointer is properly
marked that we only store to what it points to, from the second pointer
we copy and reshuffle the content and store into the first one; if there
wouldn't be any pointers, ".wr" would be just fine, but as there is a
pointer and that pointer is copied to the area pointed by first argument,
the pointer effectively leaks that way, so we e.g. can't optimize stores
into what the data pointer in the descriptor points to).  I haven't
analyzed other fn spec attributes in the FE, but think it is better to
fix at least this one we have analyzed.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-01-30  Paul Thomas  <pault@gcc.gnu.org>
	    Jakub Jelinek  <jakub@redhat.com>

	PR fortran/92123
	* trans-decl.c (gfc_get_symbol_decl): Call gfc_defer_symbol_init for
	CFI descs.
	(gfc_build_builtin_function_decls): Use ".w." instead of ".ww" or ".wR"
	for gfor_fndecl_{cfi_to_gfc,gfc_to_cfi}.
	(convert_CFI_desc): Handle references to CFI descriptors.


	Jakub

On Thu, 30 Jan 2020, Jakub Jelinek wrote:

> Hi!
> 
> This is something that has been discussed already a few months ago, but
> seems to have stalled.  Here is Paul's patch from the PR except for the
> TREE_STATIC hunk which is wrong, and does the most conservative fn spec
> tweak for the problematic two builtins we are aware of (to repeat what is in
> the PR, both .wR and .ww are wrong for these builtins that transform one
> layout of an descriptor to another one; while the first pointer is properly
> marked that we only store to what it points to, from the second pointer
> we copy and reshuffle the content and store into the first one; if there
> wouldn't be any pointers, ".wr" would be just fine, but as there is a
> pointer and that pointer is copied to the area pointed by first argument,
> the pointer effectively leaks that way, so we e.g. can't optimize stores
> into what the data pointer in the descriptor points to).  I haven't
> analyzed other fn spec attributes in the FE, but think it is better to
> fix at least this one we have analyzed.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2020-01-30  Paul Thomas  <pault@gcc.gnu.org>
> 	    Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR fortran/92123
> 	* trans-decl.c (gfc_get_symbol_decl): Call gfc_defer_symbol_init for
> 	CFI descs.
> 	(gfc_build_builtin_function_decls): Use ".w." instead of ".ww" or ".wR"
> 	for gfor_fndecl_{cfi_to_gfc,gfc_to_cfi}.
> 	(convert_CFI_desc): Handle references to CFI descriptors.
> 
> --- gcc/fortran/trans-decl.c.jj	2020-01-12 11:54:36.600410587 +0100
> +++ gcc/fortran/trans-decl.c	2020-01-29 10:54:36.771077452 +0100
> @@ -1552,6 +1552,9 @@ gfc_get_symbol_decl (gfc_symbol * sym)
>        sym->ts.u.cl->backend_decl = build_fold_indirect_ref (sym->ts.u.cl->backend_decl);
>      }
>  
> +  if (is_CFI_desc (sym, NULL))
> +    gfc_defer_symbol_init (sym);
> +
>    fun_or_res = byref && (sym->attr.result
>  			 || (sym->attr.function && sym->ts.deferred));
>    if ((sym->attr.dummy && ! sym->attr.function) || fun_or_res)
> @@ -3763,12 +3766,17 @@ gfc_build_builtin_function_decls (void)
>  	get_identifier (PREFIX("internal_unpack")), ".wR",
>  	void_type_node, 2, pvoid_type_node, pvoid_type_node);
>  
> +  /* These two builtins write into what the first argument points to and
> +     read from what the second argument points to, but we can't use R
> +     for that, because the directly pointed structure contains a pointer
> +     which is copied into the descriptor pointed by the first argument,
> +     effectively escaping that way.  See PR92123.  */
>    gfor_fndecl_cfi_to_gfc = gfc_build_library_function_decl_with_spec (
> -	get_identifier (PREFIX("cfi_desc_to_gfc_desc")), ".ww",
> +	get_identifier (PREFIX("cfi_desc_to_gfc_desc")), ".w.",
>  	void_type_node, 2, pvoid_type_node, ppvoid_type_node);
>  
>    gfor_fndecl_gfc_to_cfi = gfc_build_library_function_decl_with_spec (
> -	get_identifier (PREFIX("gfc_desc_to_cfi_desc")), ".wR",
> +	get_identifier (PREFIX("gfc_desc_to_cfi_desc")), ".w.",
>  	void_type_node, 2, ppvoid_type_node, pvoid_type_node);
>  
>    gfor_fndecl_associated = gfc_build_library_function_decl_with_spec (
> @@ -4398,6 +4406,8 @@ convert_CFI_desc (gfc_wrapped_block * bl
>       while CFI_desc is the descriptor itself.  */
>    if (DECL_LANG_SPECIFIC (sym->backend_decl))
>      CFI_desc = GFC_DECL_SAVED_DESCRIPTOR (sym->backend_decl);
> +  else if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (TREE_TYPE (sym->backend_decl))))
> +    CFI_desc = sym->backend_decl;
>    else
>      CFI_desc = NULL;
>  
> 
> 	Jakub
> 
>

Hi All,

Thank you for taking care of that, Jakub. I have been overwhelmed by
daytime work since my last posting and, in addition, have yet to set
up a git workflow. This situation is likely to continue for at least
another 3-4 months. I will release all the PRs assigned to me, except
those associated with PDTs for which I do not think there will be any
other takers.

With regard to the other fn spec attributes: I think that we can
assume that they are OK, or at least acceptable, since I did not find
any similar PRs that might implicate a similar problem.

Cheers

Paul

On Thu, 30 Jan 2020 at 07:37, Richard Biener <rguenther@suse.de> wrote:
>
> On Thu, 30 Jan 2020, Jakub Jelinek wrote:
>
> > Hi!
> >
> > This is something that has been discussed already a few months ago, but
> > seems to have stalled.  Here is Paul's patch from the PR except for the
> > TREE_STATIC hunk which is wrong, and does the most conservative fn spec
> > tweak for the problematic two builtins we are aware of (to repeat what is in
> > the PR, both .wR and .ww are wrong for these builtins that transform one
> > layout of an descriptor to another one; while the first pointer is properly
> > marked that we only store to what it points to, from the second pointer
> > we copy and reshuffle the content and store into the first one; if there
> > wouldn't be any pointers, ".wr" would be just fine, but as there is a
> > pointer and that pointer is copied to the area pointed by first argument,
> > the pointer effectively leaks that way, so we e.g. can't optimize stores
> > into what the data pointer in the descriptor points to).  I haven't
> > analyzed other fn spec attributes in the FE, but think it is better to
> > fix at least this one we have analyzed.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> OK.
>
> Thanks,
> Richard.
>
> > 2020-01-30  Paul Thomas  <pault@gcc.gnu.org>
> >           Jakub Jelinek  <jakub@redhat.com>
> >
> >       PR fortran/92123
> >       * trans-decl.c (gfc_get_symbol_decl): Call gfc_defer_symbol_init for
> >       CFI descs.
> >       (gfc_build_builtin_function_decls): Use ".w." instead of ".ww" or ".wR"
> >       for gfor_fndecl_{cfi_to_gfc,gfc_to_cfi}.
> >       (convert_CFI_desc): Handle references to CFI descriptors.
> >
> > --- gcc/fortran/trans-decl.c.jj       2020-01-12 11:54:36.600410587 +0100
> > +++ gcc/fortran/trans-decl.c  2020-01-29 10:54:36.771077452 +0100
> > @@ -1552,6 +1552,9 @@ gfc_get_symbol_decl (gfc_symbol * sym)
> >        sym->ts.u.cl->backend_decl = build_fold_indirect_ref (sym->ts.u.cl->backend_decl);
> >      }
> >
> > +  if (is_CFI_desc (sym, NULL))
> > +    gfc_defer_symbol_init (sym);
> > +
> >    fun_or_res = byref && (sym->attr.result
> >                        || (sym->attr.function && sym->ts.deferred));
> >    if ((sym->attr.dummy && ! sym->attr.function) || fun_or_res)
> > @@ -3763,12 +3766,17 @@ gfc_build_builtin_function_decls (void)
> >       get_identifier (PREFIX("internal_unpack")), ".wR",
> >       void_type_node, 2, pvoid_type_node, pvoid_type_node);
> >
> > +  /* These two builtins write into what the first argument points to and
> > +     read from what the second argument points to, but we can't use R
> > +     for that, because the directly pointed structure contains a pointer
> > +     which is copied into the descriptor pointed by the first argument,
> > +     effectively escaping that way.  See PR92123.  */
> >    gfor_fndecl_cfi_to_gfc = gfc_build_library_function_decl_with_spec (
> > -     get_identifier (PREFIX("cfi_desc_to_gfc_desc")), ".ww",
> > +     get_identifier (PREFIX("cfi_desc_to_gfc_desc")), ".w.",
> >       void_type_node, 2, pvoid_type_node, ppvoid_type_node);
> >
> >    gfor_fndecl_gfc_to_cfi = gfc_build_library_function_decl_with_spec (
> > -     get_identifier (PREFIX("gfc_desc_to_cfi_desc")), ".wR",
> > +     get_identifier (PREFIX("gfc_desc_to_cfi_desc")), ".w.",
> >       void_type_node, 2, ppvoid_type_node, pvoid_type_node);
> >
> >    gfor_fndecl_associated = gfc_build_library_function_decl_with_spec (
> > @@ -4398,6 +4406,8 @@ convert_CFI_desc (gfc_wrapped_block * bl
> >       while CFI_desc is the descriptor itself.  */
> >    if (DECL_LANG_SPECIFIC (sym->backend_decl))
> >      CFI_desc = GFC_DECL_SAVED_DESCRIPTOR (sym->backend_decl);
> > +  else if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (TREE_TYPE (sym->backend_decl))))
> > +    CFI_desc = sym->backend_decl;
> >    else
> >      CFI_desc = NULL;
> >
> >
> >       Jakub
> >
> >
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)