[PATCH] Fix PR70937

[PATCH] Fix PR70937

[Richard Biener]

The following patch fixes another case of missing DECL_EXPR in the FE.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Ok for trunk?

Thanks,
Richard.

2016-05-06  Richard Biener  <rguenther@suse.de>

	PR fortran/70937
	* trans-decl.c (gfc_trans_vla_type_sizes): Add a DECL_EXPR for
	the TYPE_DECL as well.

	* gfortran.dg/pr70937.f90: New testcase.

Index: gcc/fortran/trans-decl.c
===================================================================
*** gcc/fortran/trans-decl.c	(revision 235945)
--- gcc/fortran/trans-decl.c	(working copy)
*************** gfc_trans_vla_type_sizes (gfc_symbol *sy
*** 3818,3823 ****
--- 3818,3828 ----
      }
  
    gfc_trans_vla_type_sizes_1 (type, body);
+   /* gfc_build_qualified_array may have built this type but left TYPE_NAME
+      pointing to the original type whose type sizes we need to expose to
+      the gimplifier unsharing.  */
+   if (TYPE_NAME (type))
+     gfc_add_expr_to_block (body, build1 (DECL_EXPR, type, TYPE_NAME (type)));
  }
  
  
Index: gcc/testsuite/gfortran.dg/pr70937.f90
===================================================================
*** gcc/testsuite/gfortran.dg/pr70937.f90	(revision 0)
--- gcc/testsuite/gfortran.dg/pr70937.f90	(working copy)
***************
*** 0 ****
--- 1,10 ----
+ ! { dg-do compile }
+ ! { dg-options "-flto" }
+   SUBROUTINE dbcsr_test_read_args(narg, args)
+     CHARACTER(len=*), DIMENSION(:), &
+       INTENT(out)         :: args
+     CHARACTER(len=80)     :: line
+     DO
+        args(narg) = line
+     ENDDO
+   END SUBROUTINE dbcsr_test_read_args

Re: [PATCH] Fix PR70937

[Richard Biener]

On Fri, 6 May 2016, Richard Biener wrote:

> 
> The following patch fixes another case of missing DECL_EXPR in the FE.
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> Ok for trunk?

Dominique noticed a FAIL early which is fixed by adjusting the patch
to only handle TYPE_DECL TYPE_NAME like so:

Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c    (revision 235945)
+++ gcc/fortran/trans-decl.c    (working copy)
@@ -3818,6 +3818,12 @@ gfc_trans_vla_type_sizes (gfc_symbol *sy
     }
 
   gfc_trans_vla_type_sizes_1 (type, body);
+  /* gfc_build_qualified_array may have built this type but left 
TYPE_NAME
+     pointing to the original type whose type sizes we need to expose to
+     the gimplifier unsharing.  */
+  if (TYPE_NAME (type)
+      && TREE_CODE (TYPE_NAME (type)) == TYPE_DECL)
+    gfc_add_expr_to_block (body, build1 (DECL_EXPR, type, TYPE_NAME 
(type)));
 }
 
 
I've re-started testing.

Ok with that change?

Thanks,
Richard.

> Thanks,
> Richard.
> 
> 2016-05-06  Richard Biener  <rguenther@suse.de>
> 
> 	PR fortran/70937
> 	* trans-decl.c (gfc_trans_vla_type_sizes): Add a DECL_EXPR for
> 	the TYPE_DECL as well.
> 
> 	* gfortran.dg/pr70937.f90: New testcase.
> 
> Index: gcc/fortran/trans-decl.c
> ===================================================================
> *** gcc/fortran/trans-decl.c	(revision 235945)
> --- gcc/fortran/trans-decl.c	(working copy)
> *************** gfc_trans_vla_type_sizes (gfc_symbol *sy
> *** 3818,3823 ****
> --- 3818,3828 ----
>       }
>   
>     gfc_trans_vla_type_sizes_1 (type, body);
> +   /* gfc_build_qualified_array may have built this type but left TYPE_NAME
> +      pointing to the original type whose type sizes we need to expose to
> +      the gimplifier unsharing.  */
> +   if (TYPE_NAME (type))
> +     gfc_add_expr_to_block (body, build1 (DECL_EXPR, type, TYPE_NAME (type)));
>   }
>   
>   
> Index: gcc/testsuite/gfortran.dg/pr70937.f90
> ===================================================================
> *** gcc/testsuite/gfortran.dg/pr70937.f90	(revision 0)
> --- gcc/testsuite/gfortran.dg/pr70937.f90	(working copy)
> ***************
> *** 0 ****
> --- 1,10 ----
> + ! { dg-do compile }
> + ! { dg-options "-flto" }
> +   SUBROUTINE dbcsr_test_read_args(narg, args)
> +     CHARACTER(len=*), DIMENSION(:), &
> +       INTENT(out)         :: args
> +     CHARACTER(len=80)     :: line
> +     DO
> +        args(narg) = line
> +     ENDDO
> +   END SUBROUTINE dbcsr_test_read_args
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Fix PR70937

[Richard Biener]

On Fri, 6 May 2016, Richard Biener wrote:

> On Fri, 6 May 2016, Richard Biener wrote:
> 
> > 
> > The following patch fixes another case of missing DECL_EXPR in the FE.
> > 
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > 
> > Ok for trunk?
> 
> Dominique noticed a FAIL early which is fixed by adjusting the patch
> to only handle TYPE_DECL TYPE_NAME like so:
> 
> Index: gcc/fortran/trans-decl.c
> ===================================================================
> --- gcc/fortran/trans-decl.c    (revision 235945)
> +++ gcc/fortran/trans-decl.c    (working copy)
> @@ -3818,6 +3818,12 @@ gfc_trans_vla_type_sizes (gfc_symbol *sy
>      }
>  
>    gfc_trans_vla_type_sizes_1 (type, body);
> +  /* gfc_build_qualified_array may have built this type but left 
> TYPE_NAME
> +     pointing to the original type whose type sizes we need to expose to
> +     the gimplifier unsharing.  */
> +  if (TYPE_NAME (type)
> +      && TREE_CODE (TYPE_NAME (type)) == TYPE_DECL)
> +    gfc_add_expr_to_block (body, build1 (DECL_EXPR, type, TYPE_NAME 
> (type)));
>  }
>  
>  
> I've re-started testing.
> 
> Ok with that change?

It doesn't work fully either.  Given also the original problem where
unshared IL exposes the need to visit the "unrelated" VLA types in
the gimplifier unsharing I tested the following patch instead which
resolves the issue as well.  Furthermore it can't regress things
and at this point is the easiest way forward to unbreak fortran.

Thus, bootstrapped and tested on x86_64-unknown-linux-gnu, applied to 
trunk.

I am testing if reverting my first fix is possible after this now.

Richard.

2016-05-09  Richard Biener  <rguenther@suse.de>

	PR fortran/70937
	* trans-decl.c: Include gimplify.h for unshare_expr.
	(gfc_trans_vla_one_sizepos): Unshare exprs before inserting
	them into the IL.

	* gfortran.dg/pr70937.f90: New testcase.

Index: gcc/fortran/trans-decl.c
===================================================================
*** gcc/fortran/trans-decl.c	(revision 235945)
--- gcc/fortran/trans-decl.c	(working copy)
*************** along with GCC; see the file COPYING3.
*** 45,50 ****
--- 45,51 ----
  /* Only for gfc_trans_code.  Shouldn't need to include this.  */
  #include "trans-stmt.h"
  #include "gomp-constants.h"
+ #include "gimplify.h"
  
  #define MAX_LABEL_VALUE 99999
  
*************** gfc_trans_vla_one_sizepos (tree *tp, stm
*** 3738,3744 ****
  
    var = gfc_create_var_np (TREE_TYPE (t), NULL);
    gfc_add_decl_to_function (var);
!   gfc_add_modify (body, var, val);
    if (TREE_CODE (t) == SAVE_EXPR)
      TREE_OPERAND (t, 0) = var;
    *tp = var;
--- 3739,3745 ----
  
    var = gfc_create_var_np (TREE_TYPE (t), NULL);
    gfc_add_decl_to_function (var);
!   gfc_add_modify (body, var, unshare_expr (val));
    if (TREE_CODE (t) == SAVE_EXPR)
      TREE_OPERAND (t, 0) = var;
    *tp = var;
 
Index: gcc/testsuite/gfortran.dg/pr70937.f90
===================================================================
*** gcc/testsuite/gfortran.dg/pr70937.f90	(revision 0)
--- gcc/testsuite/gfortran.dg/pr70937.f90	(working copy)
***************
*** 0 ****
--- 1,10 ----
+ ! { dg-do compile }
+ ! { dg-options "-flto" }
+   SUBROUTINE dbcsr_test_read_args(narg, args)
+     CHARACTER(len=*), DIMENSION(:), &
+       INTENT(out)         :: args
+     CHARACTER(len=80)     :: line
+     DO
+        args(narg) = line
+     ENDDO
+   END SUBROUTINE dbcsr_test_read_args

Re: [PATCH] Fix PR70937

[Jerry DeLisle]

On 05/09/2016 12:41 AM, Richard Biener wrote:
> On Fri, 6 May 2016, Richard Biener wrote:
> 
>> On Fri, 6 May 2016, Richard Biener wrote:
>>
>>>
>>> The following patch fixes another case of missing DECL_EXPR in the FE.
>>>
>>> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>>>
>>> Ok for trunk?
>>
>> Dominique noticed a FAIL early which is fixed by adjusting the patch
>> to only handle TYPE_DECL TYPE_NAME like so:
>>
>> Index: gcc/fortran/trans-decl.c
>> ===================================================================
>> --- gcc/fortran/trans-decl.c    (revision 235945)
>> +++ gcc/fortran/trans-decl.c    (working copy)
>> @@ -3818,6 +3818,12 @@ gfc_trans_vla_type_sizes (gfc_symbol *sy
>>      }
>>  
>>    gfc_trans_vla_type_sizes_1 (type, body);
>> +  /* gfc_build_qualified_array may have built this type but left 
>> TYPE_NAME
>> +     pointing to the original type whose type sizes we need to expose to
>> +     the gimplifier unsharing.  */
>> +  if (TYPE_NAME (type)
>> +      && TREE_CODE (TYPE_NAME (type)) == TYPE_DECL)
>> +    gfc_add_expr_to_block (body, build1 (DECL_EXPR, type, TYPE_NAME 
>> (type)));
>>  }
>>  
>>  
>> I've re-started testing.
>>
>> Ok with that change?
>

Richard, yes Ok if all works.  We are very short on reviewers right now, so very
much appreciate your help here. (the below approach if you find it best)

Jerry

> It doesn't work fully either.  Given also the original problem where
> unshared IL exposes the need to visit the "unrelated" VLA types in
> the gimplifier unsharing I tested the following patch instead which
> resolves the issue as well.  Furthermore it can't regress things
> and at this point is the easiest way forward to unbreak fortran.
> 
> Thus, bootstrapped and tested on x86_64-unknown-linux-gnu, applied to 
> trunk.
> 
> I am testing if reverting my first fix is possible after this now.
> 
> Richard.