Fortran: simplify array constructors with typespec [PR93483, PR107216, PR107219]

Message ID trinity-c0a8c36e-266b-4a31-89b5-242246403fc5-1665603941818@3c-app-gmx-bs25
State Accepted, archived
Headers
Series Fortran: simplify array constructors with typespec [PR93483, PR107216, PR107219] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Harald Anlauf Oct. 12, 2022, 7:45 p.m. UTC
  Dear Fortranners,

this one was really bugging me for quite some time.  We failed to
properly handle (= simplify) expressions using array constructors
with typespec, and with parentheses and unary '+' and '-'
sprinkled here and there.  When there was no typespec, there was
no related problem.

The underlying issue apparently was that we should simplify
elements of the array constructor before attempting the type
conversion.

Thanks to Gerhard, who insisted by submitted many related PRs.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald
  

Comments

Mikael Morin Oct. 16, 2022, 9:17 p.m. UTC | #1
Le 15/10/2022 à 22:15, Harald Anlauf via Fortran a écrit :
> Dear all,
> 
> here is an updated version of the patch that includes suggestions
> and comments by Mikael in PR93483.
> 
> Basic new features are:
> - a new enum value ARITH_NOT_REDUCED to keep track if we encountered
>    an expression that was not reduced via reduce_unary/reduce_binary
> - a cleanup of the related checking, resulting in more readable
>    code.
> - a new testcase by Mikael that exhibited a flaw in the first patch
>    due to a false resolution of a symbol by premature simplification.
> 
> Regtested again.  OK for mainline?
> 
(...)
> diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
> index 10bb098d136..7b8f0b148bd 100644
> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
> @@ -222,11 +222,12 @@ enum gfc_intrinsic_op
>     Assumptions are made about the numbering of the interface_op enums.  */
>  #define GFC_INTRINSIC_OPS GFC_INTRINSIC_END
>  
> -/* Arithmetic results.  */
> +/* Arithmetic results.  ARITH_NOT_REDUCED is used to keep track of failed
> +   reductions because an erroneous expression was encountered.  */

The expressions are not always erroneous.  They can be, but in the 
testcase for example, all the expressions are valid.  They are just 
unsupported by the arithmetic evaluation code which works only with 
literal constants and arrays of literal constants (and arrays of arrays 
etc).

OK with that comment fixed.

Thanks.
  
Harald Anlauf Oct. 17, 2022, 5:31 p.m. UTC | #2
Hi Mikael,

Am 16.10.22 um 23:17 schrieb Mikael Morin:
> Le 15/10/2022 à 22:15, Harald Anlauf via Fortran a écrit :
>> Dear all,
>>
>> here is an updated version of the patch that includes suggestions
>> and comments by Mikael in PR93483.
>>
>> Basic new features are:
>> - a new enum value ARITH_NOT_REDUCED to keep track if we encountered
>>    an expression that was not reduced via reduce_unary/reduce_binary
>> - a cleanup of the related checking, resulting in more readable
>>    code.
>> - a new testcase by Mikael that exhibited a flaw in the first patch
>>    due to a false resolution of a symbol by premature simplification.
>>
>> Regtested again.  OK for mainline?
>>
> (...)
>> diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
>> index 10bb098d136..7b8f0b148bd 100644
>> --- a/gcc/fortran/gfortran.h
>> +++ b/gcc/fortran/gfortran.h
>> @@ -222,11 +222,12 @@ enum gfc_intrinsic_op
>>     Assumptions are made about the numbering of the interface_op
>> enums.  */
>>  #define GFC_INTRINSIC_OPS GFC_INTRINSIC_END
>>
>> -/* Arithmetic results.  */
>> +/* Arithmetic results.  ARITH_NOT_REDUCED is used to keep track of
>> failed
>> +   reductions because an erroneous expression was encountered.  */
>
> The expressions are not always erroneous.  They can be, but in the
> testcase for example, all the expressions are valid.  They are just
> unsupported by the arithmetic evaluation code which works only with
> literal constants and arrays of literal constants (and arrays of arrays
> etc).
>
> OK with that comment fixed.

you're absolutely right.  I adjusted the comment and the commit
message according to your suggestion.

Pushed as https://gcc.gnu.org/g:d45af5c2eb1ba1e48449d8f3c5b4e3994a956f92

Thanks,
Harald

> Thanks.
>
  

Patch

From ee65197f4d0b0050dc61687b5a77f1afe3bd4a27 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Wed, 12 Oct 2022 21:33:36 +0200
Subject: [PATCH] Fortran: simplify array constructors with typespec [PR93483,
 PR107216, PR107219]

gcc/fortran/ChangeLog:

	PR fortran/93483
	PR fortran/107216
	PR fortran/107219
	* array.cc (walk_array_constructor): If an element of an array
	constructor is an EXPR_OP, try simplification before type conversion.

gcc/testsuite/ChangeLog:

	PR fortran/93483
	PR fortran/107216
	PR fortran/107219
	* gfortran.dg/array_constructor_56.f90: New test.
---
 gcc/fortran/array.cc                          |  4 ++++
 .../gfortran.dg/array_constructor_56.f90      | 22 +++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/array_constructor_56.f90

diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index bbdb5b392fc..9bec299f160 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -1205,6 +1205,10 @@  walk_array_constructor (gfc_typespec *ts, gfc_constructor_base head)
   for (c = gfc_constructor_first (head); c; c = gfc_constructor_next (c))
     {
       e = c->expr;
+
+      if (e->expr_type == EXPR_OP)
+	gfc_simplify_expr (e, 0);
+
       if (e->expr_type == EXPR_ARRAY && e->ts.type == BT_UNKNOWN
 	  && !e->ref && e->value.constructor)
 	{
diff --git a/gcc/testsuite/gfortran.dg/array_constructor_56.f90 b/gcc/testsuite/gfortran.dg/array_constructor_56.f90
new file mode 100644
index 00000000000..4701fb36225
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/array_constructor_56.f90
@@ -0,0 +1,22 @@ 
+! { dg-do compile }
+!
+! Test the fix for the following:
+! PR fortran/93483
+! PR fortran/107216
+! PR fortran/107219
+!
+! Contributed by G.Steinmetz
+
+program p
+  real, parameter :: r0(*) = +[real :: +(1) ]
+  real, parameter :: r1(*) = +[real :: +[1] ]
+  real, parameter :: r2(*) = -[real :: [(1)]]
+  real, parameter :: r3(*) = +[real :: [-(1)]]
+  real, parameter :: r4(*) = -[real :: [[(1)]]]
+  real, parameter :: r5(*) = -[real :: -[1, 2]]
+  real, parameter :: r6(*) = +[real :: +[1, 2]]
+  real, parameter :: r7(*) =  [real :: 1, 2] * [real :: 1, (2)]
+  real, parameter :: r8(*) =  [real :: 1, (2)] * [real :: 1, 2]
+  real, parameter :: r9(*) = +[real :: 1, 2] * [real :: 1, (2)]
+  real, parameter :: rr(*) = -[real :: 1, (2)] * [real :: 1, 2]
+end
--
2.35.3