[PATCH] Emit DECL_EXPR so that type of what pstr.N points to is gimplified (PR fortran/78757)

[PATCH] Emit DECL_EXPR so that type of what pstr.N points to is gimplified (PR fortran/78757)

[Jakub Jelinek]

Hi!

The gimplifier (because of Ada :( ) doesn't recurse to POINTER_TYPE's
type when gimplifying types, so we need a DECL_EXPR to gimplify such
types if they are VLAs.  The following patch is an attempt to do that.

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

2016-12-14  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/78757
	* trans-expr.c (gfc_conv_procedure_call): Emit DECL_EXPR for the
	type pstr var points to.

	* gfortran.dg/pr78757.f90: New test.

--- gcc/fortran/trans-expr.c.jj	2016-12-09 20:32:39.000000000 +0100
+++ gcc/fortran/trans-expr.c	2016-12-14 15:22:50.142968565 +0100
@@ -6009,6 +6009,19 @@ gfc_conv_procedure_call (gfc_se * se, gf
 	    {
 	      var = gfc_create_var (type, "pstr");
 
+	      /* Emit a DECL_EXPR for the VLA type.  */
+	      tmp = TREE_TYPE (type);
+	      if (TYPE_SIZE (tmp)
+		  && TREE_CODE (TYPE_SIZE (tmp)) != INTEGER_CST)
+		{
+		  tmp = build_decl (input_location, TYPE_DECL, NULL_TREE, tmp);
+		  DECL_ARTIFICIAL (tmp) = 1;
+		  DECL_IGNORED_P (tmp) = 1;
+		  tmp = fold_build1_loc (input_location, DECL_EXPR,
+					 TREE_TYPE (tmp), tmp);
+		  gfc_add_expr_to_block (&se->pre, tmp);
+		}
+
 	      if ((!comp && sym->attr.allocatable)
 		  || (comp && comp->attr.allocatable))
 		{
--- gcc/testsuite/gfortran.dg/pr78757.f90.jj	2016-12-14 15:28:09.707932278 +0100
+++ gcc/testsuite/gfortran.dg/pr78757.f90	2016-12-14 15:27:54.000000000 +0100
@@ -0,0 +1,16 @@
+! PR fortran/78757
+! { dg-do compile }
+! { dg-options "-O1" }
+
+program pr78757
+  implicit none
+  character (len = 30), target :: x
+  character (len = 30), pointer :: s
+  s => foo (70_8)
+contains
+  function foo (i)
+    integer (8) :: i
+    character (len = i), pointer :: foo
+    foo => x
+  end function foo
+end program pr78757

	Jakub

Re: [PATCH] Emit DECL_EXPR so that type of what pstr.N points to is gimplified (PR fortran/78757)

[Eric Botcazou]

> The gimplifier (because of Ada :( ) doesn't recurse to POINTER_TYPE's
> type when gimplifying types, so we need a DECL_EXPR to gimplify such
> types if they are VLAs.  The following patch is an attempt to do that.

Well, you should rather say thanks...  The gimplification point is also the 
point where side effects are evaluated and you cannot let the gimplifier 
randomly decide to evaluate them when it happens to see a pointer, because 
really bad things may happen in that case.

-- 
Eric Botcazou

Re: [PATCH] Emit DECL_EXPR so that type of what pstr.N points to is gimplified (PR fortran/78757)

[Andre Vehreschild]

Hi Jakub,

the patch looks ok to me. I am wondering whether we should setup a routine
or a macro for doing such a DECL_EXPR like Fortran needs it. In your case the
TYPE_DECL is marked artificial and ignored in other cases not. Would that be
need also at the six other locations gfortran uses a type decl?

My idea of using a common way to do these DECL_EXPR is to prevent us 
fortraneers from doing something wrong, because we don't usually need this
construct. And a "templated" way prevents us from reinventing the wheel anew
with the all pits one can fall in.

Regards,
	Andre

On Wed, 14 Dec 2016 22:55:29 +0100
Jakub Jelinek <jakub@redhat.com> wrote:

> Hi!
> 
> The gimplifier (because of Ada :( ) doesn't recurse to POINTER_TYPE's
> type when gimplifying types, so we need a DECL_EXPR to gimplify such
> types if they are VLAs.  The following patch is an attempt to do that.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2016-12-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR fortran/78757
> 	* trans-expr.c (gfc_conv_procedure_call): Emit DECL_EXPR for the
> 	type pstr var points to.
> 
> 	* gfortran.dg/pr78757.f90: New test.
> 
> --- gcc/fortran/trans-expr.c.jj	2016-12-09 20:32:39.000000000 +0100
> +++ gcc/fortran/trans-expr.c	2016-12-14 15:22:50.142968565 +0100
> @@ -6009,6 +6009,19 @@ gfc_conv_procedure_call (gfc_se * se, gf
>  	    {
>  	      var = gfc_create_var (type, "pstr");
>  
> +	      /* Emit a DECL_EXPR for the VLA type.  */
> +	      tmp = TREE_TYPE (type);
> +	      if (TYPE_SIZE (tmp)
> +		  && TREE_CODE (TYPE_SIZE (tmp)) != INTEGER_CST)
> +		{
> +		  tmp = build_decl (input_location, TYPE_DECL, NULL_TREE,
> tmp);
> +		  DECL_ARTIFICIAL (tmp) = 1;
> +		  DECL_IGNORED_P (tmp) = 1;
> +		  tmp = fold_build1_loc (input_location, DECL_EXPR,
> +					 TREE_TYPE (tmp), tmp);
> +		  gfc_add_expr_to_block (&se->pre, tmp);
> +		}
> +
>  	      if ((!comp && sym->attr.allocatable)
>  		  || (comp && comp->attr.allocatable))
>  		{
> --- gcc/testsuite/gfortran.dg/pr78757.f90.jj	2016-12-14
> 15:28:09.707932278 +0100 +++ gcc/testsuite/gfortran.dg/pr78757.f90
> 2016-12-14 15:27:54.000000000 +0100 @@ -0,0 +1,16 @@
> +! PR fortran/78757
> +! { dg-do compile }
> +! { dg-options "-O1" }
> +
> +program pr78757
> +  implicit none
> +  character (len = 30), target :: x
> +  character (len = 30), pointer :: s
> +  s => foo (70_8)
> +contains
> +  function foo (i)
> +    integer (8) :: i
> +    character (len = i), pointer :: foo
> +    foo => x
> +  end function foo
> +end program pr78757
> 
> 	Jakub


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

Re: [PATCH] Emit DECL_EXPR so that type of what pstr.N points to is gimplified (PR fortran/78757)

[Jakub Jelinek]

On Fri, Dec 16, 2016 at 04:49:12PM +0100, Andre Vehreschild wrote:
> the patch looks ok to me. I am wondering whether we should setup a routine
> or a macro for doing such a DECL_EXPR like Fortran needs it. In your case the
> TYPE_DECL is marked artificial and ignored in other cases not. Would that be
> need also at the six other locations gfortran uses a type decl?

I think it depends.  If the TYPE_DECL is added as TYPE_NAME of the type,
then the DECL_ARTIFICIAL and DECL_IGNORED_P on it should depend on what kind
of type it is, if it is a user type, it shouldn't be artificial.
While in this case it isn't added as TYPE_NAME of the type, it is just a
mean to tell the gimplifier to gimplify the type, so it shouldn't make it
in that case into debug info etc.

Maybe it would be better not to use a pointer to VLA type here at all,
just pointer to void, and then after the call convert it to the right lhs
type.  I haven't analyzed all the possible cases though to find out if it
could work.

	Jakub

Re: [PATCH] Emit DECL_EXPR so that type of what pstr.N points to is gimplified (PR fortran/78757)

[Andre Vehreschild]

Hi Jakub,

I don't have enough knowledge on this part of gcc. So please don't take my
comment as an "ok for trunk". I don't have that privilege.

Furthermore did Dominique just now raise the question in IRC whether the
testcase is valid: foo is declared to return a pointer to a char(len=70) but
points to a char(len=30). My Fortran is not good enough know what the result
should be. I was more concerned about unifying the way typedefs are done by
gfortran.

Sorry, for the noise.

- Andre

On Fri, 16 Dec 2016 16:54:45 +0100
Jakub Jelinek <jakub@redhat.com> wrote:

> On Fri, Dec 16, 2016 at 04:49:12PM +0100, Andre Vehreschild wrote:
> > the patch looks ok to me. I am wondering whether we should setup a routine
> > or a macro for doing such a DECL_EXPR like Fortran needs it. In your case
> > the TYPE_DECL is marked artificial and ignored in other cases not. Would
> > that be need also at the six other locations gfortran uses a type decl?  
> 
> I think it depends.  If the TYPE_DECL is added as TYPE_NAME of the type,
> then the DECL_ARTIFICIAL and DECL_IGNORED_P on it should depend on what kind
> of type it is, if it is a user type, it shouldn't be artificial.
> While in this case it isn't added as TYPE_NAME of the type, it is just a
> mean to tell the gimplifier to gimplify the type, so it shouldn't make it
> in that case into debug info etc.
> 
> Maybe it would be better not to use a pointer to VLA type here at all,
> just pointer to void, and then after the call convert it to the right lhs
> type.  I haven't analyzed all the possible cases though to find out if it
> could work.
> 
> 	Jakub


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

Re: [PATCH] Emit DECL_EXPR so that type of what pstr.N points to is gimplified (PR fortran/78757)

[Janne Blomqvist]

On Wed, Dec 14, 2016 at 11:55 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The gimplifier (because of Ada :( ) doesn't recurse to POINTER_TYPE's
> type when gimplifying types, so we need a DECL_EXPR to gimplify such
> types if they are VLAs.  The following patch is an attempt to do that.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok, but could you name the testcase something more descriptive, say,
char_result_16.f90?

>
> 2016-12-14  Jakub Jelinek  <jakub@redhat.com>
>
>         PR fortran/78757
>         * trans-expr.c (gfc_conv_procedure_call): Emit DECL_EXPR for the
>         type pstr var points to.
>
>         * gfortran.dg/pr78757.f90: New test.
>
> --- gcc/fortran/trans-expr.c.jj 2016-12-09 20:32:39.000000000 +0100
> +++ gcc/fortran/trans-expr.c    2016-12-14 15:22:50.142968565 +0100
> @@ -6009,6 +6009,19 @@ gfc_conv_procedure_call (gfc_se * se, gf
>             {
>               var = gfc_create_var (type, "pstr");
>
> +             /* Emit a DECL_EXPR for the VLA type.  */
> +             tmp = TREE_TYPE (type);
> +             if (TYPE_SIZE (tmp)
> +                 && TREE_CODE (TYPE_SIZE (tmp)) != INTEGER_CST)
> +               {
> +                 tmp = build_decl (input_location, TYPE_DECL, NULL_TREE, tmp);
> +                 DECL_ARTIFICIAL (tmp) = 1;
> +                 DECL_IGNORED_P (tmp) = 1;
> +                 tmp = fold_build1_loc (input_location, DECL_EXPR,
> +                                        TREE_TYPE (tmp), tmp);
> +                 gfc_add_expr_to_block (&se->pre, tmp);
> +               }
> +
>               if ((!comp && sym->attr.allocatable)
>                   || (comp && comp->attr.allocatable))
>                 {
> --- gcc/testsuite/gfortran.dg/pr78757.f90.jj    2016-12-14 15:28:09.707932278 +0100
> +++ gcc/testsuite/gfortran.dg/pr78757.f90       2016-12-14 15:27:54.000000000 +0100
> @@ -0,0 +1,16 @@
> +! PR fortran/78757
> +! { dg-do compile }
> +! { dg-options "-O1" }
> +
> +program pr78757
> +  implicit none
> +  character (len = 30), target :: x
> +  character (len = 30), pointer :: s
> +  s => foo (70_8)
> +contains
> +  function foo (i)
> +    integer (8) :: i
> +    character (len = i), pointer :: foo
> +    foo => x
> +  end function foo
> +end program pr78757
>
>         Jakub



-- 
Janne Blomqvist

Re: [PATCH] Emit DECL_EXPR so that type of what pstr.N points to is gimplified (PR fortran/78757)

[Jakub Jelinek]

On Fri, Dec 16, 2016 at 09:31:37PM +0200, Janne Blomqvist wrote:
> On Wed, Dec 14, 2016 at 11:55 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > Hi!
> >
> > The gimplifier (because of Ada :( ) doesn't recurse to POINTER_TYPE's
> > type when gimplifying types, so we need a DECL_EXPR to gimplify such
> > types if they are VLAs.  The following patch is an attempt to do that.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Ok, but could you name the testcase something more descriptive, say,
> char_result_16.f90?

Thanks, I've also changed the 70_8 to 30_8 in the testcase, so that it is
clearly valid.  It still ICEs without the patch and works with the patch.

So here is what I've committed:

2016-12-16  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/78757
	* trans-expr.c (gfc_conv_procedure_call): Emit DECL_EXPR for the
	type pstr var points to.

	* gfortran.dg/char_result_16.f90: New test.

--- gcc/fortran/trans-expr.c.jj	2016-12-09 20:32:39.000000000 +0100
+++ gcc/fortran/trans-expr.c	2016-12-14 15:22:50.142968565 +0100
@@ -6009,6 +6009,19 @@ gfc_conv_procedure_call (gfc_se * se, gf
 	    {
 	      var = gfc_create_var (type, "pstr");
 
+	      /* Emit a DECL_EXPR for the VLA type.  */
+	      tmp = TREE_TYPE (type);
+	      if (TYPE_SIZE (tmp)
+		  && TREE_CODE (TYPE_SIZE (tmp)) != INTEGER_CST)
+		{
+		  tmp = build_decl (input_location, TYPE_DECL, NULL_TREE, tmp);
+		  DECL_ARTIFICIAL (tmp) = 1;
+		  DECL_IGNORED_P (tmp) = 1;
+		  tmp = fold_build1_loc (input_location, DECL_EXPR,
+					 TREE_TYPE (tmp), tmp);
+		  gfc_add_expr_to_block (&se->pre, tmp);
+		}
+
 	      if ((!comp && sym->attr.allocatable)
 		  || (comp && comp->attr.allocatable))
 		{
--- gcc/testsuite/gfortran.dg/char_result_16.f90.jj	2016-12-14 15:28:09.707932278 +0100
+++ gcc/testsuite/gfortran.dg/char_result_16.f90	2016-12-14 15:27:54.000000000 +0100
@@ -0,0 +1,16 @@
+! PR fortran/78757
+! { dg-do compile }
+! { dg-options "-O1" }
+
+program pr78757
+  implicit none
+  character (len = 30), target :: x
+  character (len = 30), pointer :: s
+  s => foo (30_8)
+contains
+  function foo (i)
+    integer (8) :: i
+    character (len = i), pointer :: foo
+    foo => x
+  end function foo
+end program pr78757


	Jakub