* [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
@ 2022-03-25 8:57 Jakub Jelinek
2022-03-25 10:13 ` Tobias Burnus
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2022-03-25 8:57 UTC (permalink / raw)
To: fortran; +Cc: gcc-patches
Hi!
On the gfortran.dg/pr103691.f90 testcase the Fortran ICE emits
static real(kind=4) a[0] = {[0 ... -1]=2.0e+0};
That is an invalid RANGE_EXPR where the maximum is smaller than the minimum.
The following patch fixes that. If TYPE_MAX_VALUE is smaller than
TYPE_MIN_VALUE, the array is empty and so doesn't need any initializer,
if the two are equal, we don't need to bother with a RANGE_EXPR and
can just use that INTEGER_CST as the index and finally for the 2+ values
in the range it uses a RANGE_EXPR as before.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2022-03-25 Jakub Jelinek <jakub@redhat.com>
PR fortran/103691
* trans-array.cc (gfc_conv_array_initializer): If TYPE_MAX_VALUE is
smaller than TYPE_MIN_VALUE (i.e. empty array), throw the initializer
on the floor, if TYPE_MIN_VALUE is equal to TYPE_MAX_VALUE, use just
the TYPE_MIN_VALUE as index instead of RANGE_EXPR.
--- gcc/fortran/trans-array.cc.jj 2022-02-04 14:36:55.113603791 +0100
+++ gcc/fortran/trans-array.cc 2022-03-24 16:14:58.334498775 +0100
@@ -6267,10 +6267,17 @@ gfc_conv_array_initializer (tree type, g
else
gfc_conv_structure (&se, expr, 1);
- CONSTRUCTOR_APPEND_ELT (v, build2 (RANGE_EXPR, gfc_array_index_type,
- TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
- TYPE_MAX_VALUE (TYPE_DOMAIN (type))),
- se.expr);
+ if (tree_int_cst_lt (TYPE_MAX_VALUE (TYPE_DOMAIN (type)),
+ TYPE_MIN_VALUE (TYPE_DOMAIN (type))))
+ break;
+ else if (tree_int_cst_equal (TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
+ TYPE_MAX_VALUE (TYPE_DOMAIN (type))))
+ range = TYPE_MIN_VALUE (TYPE_DOMAIN (type));
+ else
+ range = build2 (RANGE_EXPR, gfc_array_index_type,
+ TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
+ TYPE_MAX_VALUE (TYPE_DOMAIN (type)));
+ CONSTRUCTOR_APPEND_ELT (v, range, se.expr);
break;
case EXPR_ARRAY:
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
2022-03-25 8:57 [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691] Jakub Jelinek
@ 2022-03-25 10:13 ` Tobias Burnus
2022-03-25 11:16 ` Richard Biener
0 siblings, 1 reply; 8+ messages in thread
From: Tobias Burnus @ 2022-03-25 10:13 UTC (permalink / raw)
To: Jakub Jelinek, fortran; +Cc: gcc-patches
On 25.03.22 09:57, Jakub Jelinek via Fortran wrote:
> On the gfortran.dg/pr103691.f90 testcase the Fortran ICE emits
> static real(kind=4) a[0] = {[0 ... -1]=2.0e+0};
> That is an invalid RANGE_EXPR where the maximum is smaller than the minimum.
>
> The following patch fixes that. If TYPE_MAX_VALUE is smaller than
> TYPE_MIN_VALUE, the array is empty and so doesn't need any initializer,
> if the two are equal, we don't need to bother with a RANGE_EXPR and
> can just use that INTEGER_CST as the index and finally for the 2+ values
> in the range it uses a RANGE_EXPR as before.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
LGTM – thanks for taking care of Fortran patches and regressions.
> 2022-03-25 Jakub Jelinek <jakub@redhat.com>
>
> PR fortran/103691
> * trans-array.cc (gfc_conv_array_initializer): If TYPE_MAX_VALUE is
> smaller than TYPE_MIN_VALUE (i.e. empty array), throw the initializer
> on the floor, if TYPE_MIN_VALUE is equal to TYPE_MAX_VALUE, use just
> the TYPE_MIN_VALUE as index instead of RANGE_EXPR.
I am not sure whether "throw the initializer on the floor" is the best wording
for a changelog. I think I prefer a wording like "ignore the initializer" or
another less idiomatic expression. And I think a ';' before the second 'if'
also increases readability.
Tobias
> --- gcc/fortran/trans-array.cc.jj 2022-02-04 14:36:55.113603791 +0100
> +++ gcc/fortran/trans-array.cc 2022-03-24 16:14:58.334498775 +0100
> @@ -6267,10 +6267,17 @@ gfc_conv_array_initializer (tree type, g
> else
> gfc_conv_structure (&se, expr, 1);
>
> - CONSTRUCTOR_APPEND_ELT (v, build2 (RANGE_EXPR, gfc_array_index_type,
> - TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
> - TYPE_MAX_VALUE (TYPE_DOMAIN (type))),
> - se.expr);
> + if (tree_int_cst_lt (TYPE_MAX_VALUE (TYPE_DOMAIN (type)),
> + TYPE_MIN_VALUE (TYPE_DOMAIN (type))))
> + break;
> + else if (tree_int_cst_equal (TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
> + TYPE_MAX_VALUE (TYPE_DOMAIN (type))))
> + range = TYPE_MIN_VALUE (TYPE_DOMAIN (type));
> + else
> + range = build2 (RANGE_EXPR, gfc_array_index_type,
> + TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
> + TYPE_MAX_VALUE (TYPE_DOMAIN (type)));
> + CONSTRUCTOR_APPEND_ELT (v, range, se.expr);
> break;
>
> case EXPR_ARRAY:
>
> Jakub
>
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
2022-03-25 10:13 ` Tobias Burnus
@ 2022-03-25 11:16 ` Richard Biener
2022-03-25 11:34 ` Jakub Jelinek
0 siblings, 1 reply; 8+ messages in thread
From: Richard Biener @ 2022-03-25 11:16 UTC (permalink / raw)
To: Tobias Burnus; +Cc: Jakub Jelinek, fortran, GCC Patches
On Fri, Mar 25, 2022 at 11:13 AM Tobias Burnus <tobias@codesourcery.com> wrote:
>
> On 25.03.22 09:57, Jakub Jelinek via Fortran wrote:
> > On the gfortran.dg/pr103691.f90 testcase the Fortran ICE emits
> > static real(kind=4) a[0] = {[0 ... -1]=2.0e+0};
> > That is an invalid RANGE_EXPR where the maximum is smaller than the minimum.
> >
> > The following patch fixes that. If TYPE_MAX_VALUE is smaller than
> > TYPE_MIN_VALUE, the array is empty and so doesn't need any initializer,
> > if the two are equal, we don't need to bother with a RANGE_EXPR and
> > can just use that INTEGER_CST as the index and finally for the 2+ values
> > in the range it uses a RANGE_EXPR as before.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> LGTM – thanks for taking care of Fortran patches and regressions.
>
> > 2022-03-25 Jakub Jelinek <jakub@redhat.com>
> >
> > PR fortran/103691
> > * trans-array.cc (gfc_conv_array_initializer): If TYPE_MAX_VALUE is
> > smaller than TYPE_MIN_VALUE (i.e. empty array), throw the initializer
> > on the floor, if TYPE_MIN_VALUE is equal to TYPE_MAX_VALUE, use just
> > the TYPE_MIN_VALUE as index instead of RANGE_EXPR.
>
> I am not sure whether "throw the initializer on the floor" is the best wording
> for a changelog. I think I prefer a wording like "ignore the initializer" or
> another less idiomatic expression. And I think a ';' before the second 'if'
> also increases readability.
Can there be side-effects in those initializer elements in Fortran?
Richard.
> Tobias
>
> > --- gcc/fortran/trans-array.cc.jj 2022-02-04 14:36:55.113603791 +0100
> > +++ gcc/fortran/trans-array.cc 2022-03-24 16:14:58.334498775 +0100
> > @@ -6267,10 +6267,17 @@ gfc_conv_array_initializer (tree type, g
> > else
> > gfc_conv_structure (&se, expr, 1);
> >
> > - CONSTRUCTOR_APPEND_ELT (v, build2 (RANGE_EXPR, gfc_array_index_type,
> > - TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
> > - TYPE_MAX_VALUE (TYPE_DOMAIN (type))),
> > - se.expr);
> > + if (tree_int_cst_lt (TYPE_MAX_VALUE (TYPE_DOMAIN (type)),
> > + TYPE_MIN_VALUE (TYPE_DOMAIN (type))))
> > + break;
> > + else if (tree_int_cst_equal (TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
> > + TYPE_MAX_VALUE (TYPE_DOMAIN (type))))
> > + range = TYPE_MIN_VALUE (TYPE_DOMAIN (type));
> > + else
> > + range = build2 (RANGE_EXPR, gfc_array_index_type,
> > + TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
> > + TYPE_MAX_VALUE (TYPE_DOMAIN (type)));
> > + CONSTRUCTOR_APPEND_ELT (v, range, se.expr);
> > break;
> >
> > case EXPR_ARRAY:
> >
> > Jakub
> >
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
2022-03-25 11:16 ` Richard Biener
@ 2022-03-25 11:34 ` Jakub Jelinek
2022-03-25 12:13 ` Richard Biener
2022-03-26 11:27 ` Thomas Koenig
0 siblings, 2 replies; 8+ messages in thread
From: Jakub Jelinek @ 2022-03-25 11:34 UTC (permalink / raw)
To: Richard Biener; +Cc: Tobias Burnus, fortran, GCC Patches
On Fri, Mar 25, 2022 at 12:16:40PM +0100, Richard Biener wrote:
> On Fri, Mar 25, 2022 at 11:13 AM Tobias Burnus <tobias@codesourcery.com> wrote:
> >
> > On 25.03.22 09:57, Jakub Jelinek via Fortran wrote:
> > > On the gfortran.dg/pr103691.f90 testcase the Fortran ICE emits
> > > static real(kind=4) a[0] = {[0 ... -1]=2.0e+0};
> > > That is an invalid RANGE_EXPR where the maximum is smaller than the minimum.
> > >
> > > The following patch fixes that. If TYPE_MAX_VALUE is smaller than
> > > TYPE_MIN_VALUE, the array is empty and so doesn't need any initializer,
> > > if the two are equal, we don't need to bother with a RANGE_EXPR and
> > > can just use that INTEGER_CST as the index and finally for the 2+ values
> > > in the range it uses a RANGE_EXPR as before.
> > >
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > LGTM – thanks for taking care of Fortran patches and regressions.
> >
> > > 2022-03-25 Jakub Jelinek <jakub@redhat.com>
> > >
> > > PR fortran/103691
> > > * trans-array.cc (gfc_conv_array_initializer): If TYPE_MAX_VALUE is
> > > smaller than TYPE_MIN_VALUE (i.e. empty array), throw the initializer
> > > on the floor, if TYPE_MIN_VALUE is equal to TYPE_MAX_VALUE, use just
> > > the TYPE_MIN_VALUE as index instead of RANGE_EXPR.
> >
> > I am not sure whether "throw the initializer on the floor" is the best wording
> > for a changelog. I think I prefer a wording like "ignore the initializer" or
> > another less idiomatic expression. And I think a ';' before the second 'if'
> > also increases readability.
>
> Can there be side-effects in those initializer elements in Fortran?
For PARAMETERs certainly not, those need to be constant.
Even otherwise, this is in a routine that does
/* Create a constructor from the list of elements. */
tmp = build_constructor (type, v);
TREE_CONSTANT (tmp) = 1;
return tmp;
at the end so I wouldn't expect side-effects anywhere.
Also, I think typically in the Fortran FE side-effects would go into
se.pre and se.post sequences, not into se.expr, and this routine
doesn't emit those se.pre/se.post sequences anywhere, so presumably it
assumes they don't exist.
What is the behavior with a RANGE_EXPR when one has { [0..10] = ++i; },
is that applying the side-effects 11 times or once ?
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
2022-03-25 11:34 ` Jakub Jelinek
@ 2022-03-25 12:13 ` Richard Biener
2022-03-25 12:15 ` Jakub Jelinek
2022-03-26 11:27 ` Thomas Koenig
1 sibling, 1 reply; 8+ messages in thread
From: Richard Biener @ 2022-03-25 12:13 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Tobias Burnus, fortran, GCC Patches
On Fri, Mar 25, 2022 at 12:34 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, Mar 25, 2022 at 12:16:40PM +0100, Richard Biener wrote:
> > On Fri, Mar 25, 2022 at 11:13 AM Tobias Burnus <tobias@codesourcery.com> wrote:
> > >
> > > On 25.03.22 09:57, Jakub Jelinek via Fortran wrote:
> > > > On the gfortran.dg/pr103691.f90 testcase the Fortran ICE emits
> > > > static real(kind=4) a[0] = {[0 ... -1]=2.0e+0};
> > > > That is an invalid RANGE_EXPR where the maximum is smaller than the minimum.
> > > >
> > > > The following patch fixes that. If TYPE_MAX_VALUE is smaller than
> > > > TYPE_MIN_VALUE, the array is empty and so doesn't need any initializer,
> > > > if the two are equal, we don't need to bother with a RANGE_EXPR and
> > > > can just use that INTEGER_CST as the index and finally for the 2+ values
> > > > in the range it uses a RANGE_EXPR as before.
> > > >
> > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > >
> > > LGTM – thanks for taking care of Fortran patches and regressions.
> > >
> > > > 2022-03-25 Jakub Jelinek <jakub@redhat.com>
> > > >
> > > > PR fortran/103691
> > > > * trans-array.cc (gfc_conv_array_initializer): If TYPE_MAX_VALUE is
> > > > smaller than TYPE_MIN_VALUE (i.e. empty array), throw the initializer
> > > > on the floor, if TYPE_MIN_VALUE is equal to TYPE_MAX_VALUE, use just
> > > > the TYPE_MIN_VALUE as index instead of RANGE_EXPR.
> > >
> > > I am not sure whether "throw the initializer on the floor" is the best wording
> > > for a changelog. I think I prefer a wording like "ignore the initializer" or
> > > another less idiomatic expression. And I think a ';' before the second 'if'
> > > also increases readability.
> >
> > Can there be side-effects in those initializer elements in Fortran?
>
> For PARAMETERs certainly not, those need to be constant.
> Even otherwise, this is in a routine that does
> /* Create a constructor from the list of elements. */
> tmp = build_constructor (type, v);
> TREE_CONSTANT (tmp) = 1;
> return tmp;
> at the end so I wouldn't expect side-effects anywhere.
Ah, didn't see that.
> Also, I think typically in the Fortran FE side-effects would go into
> se.pre and se.post sequences, not into se.expr, and this routine
> doesn't emit those se.pre/se.post sequences anywhere, so presumably it
> assumes they don't exist.
>
> What is the behavior with a RANGE_EXPR when one has { [0..10] = ++i; },
> is that applying the side-effects 11 times or once ?
11 times is what is documented.
Richard.
>
>
> Jakub
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
2022-03-25 12:13 ` Richard Biener
@ 2022-03-25 12:15 ` Jakub Jelinek
0 siblings, 0 replies; 8+ messages in thread
From: Jakub Jelinek @ 2022-03-25 12:15 UTC (permalink / raw)
To: Richard Biener; +Cc: Tobias Burnus, fortran, GCC Patches
On Fri, Mar 25, 2022 at 01:13:06PM +0100, Richard Biener wrote:
> > Also, I think typically in the Fortran FE side-effects would go into
> > se.pre and se.post sequences, not into se.expr, and this routine
> > doesn't emit those se.pre/se.post sequences anywhere, so presumably it
> > assumes they don't exist.
> >
> > What is the behavior with a RANGE_EXPR when one has { [0..10] = ++i; },
> > is that applying the side-effects 11 times or once ?
>
> 11 times is what is documented.
Then [0..-1] = ++i should be 0 times the side-effect.
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
2022-03-25 11:34 ` Jakub Jelinek
2022-03-25 12:13 ` Richard Biener
@ 2022-03-26 11:27 ` Thomas Koenig
2022-03-26 17:13 ` Richard Biener
1 sibling, 1 reply; 8+ messages in thread
From: Thomas Koenig @ 2022-03-26 11:27 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: Tobias Burnus, GCC Patches, fortran
On 25.03.22 12:34, Jakub Jelinek via Fortran wrote:
> What is the behavior with a RANGE_EXPR when one has { [0..10] = ++i;
> }, is that applying the side-effects 11 times or once ?
For side effects during the evaluation of expression, Fortran has a
clear "if you depend on it, it's your fault" rule. In F 2018, it says
10.1.7 Evaluation of operands
1 It is not necessary for a processor to evaluate all of the operands of
an expression, or to evaluate entirely each operand, if the value of the
expression can be determined otherwise.
Also, the semantics of
a(a:b) = expr
say that the expression on the LHS is evaluated only once before
assignment. So, anything that looks like that should be translated
to
tmp = ++i;
[0..10] = tmp;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
2022-03-26 11:27 ` Thomas Koenig
@ 2022-03-26 17:13 ` Richard Biener
0 siblings, 0 replies; 8+ messages in thread
From: Richard Biener @ 2022-03-26 17:13 UTC (permalink / raw)
To: Thomas Koenig; +Cc: Jakub Jelinek, Tobias Burnus, GCC Patches, fortran
> Am 26.03.2022 um 12:28 schrieb Thomas Koenig <tkoenig@netcologne.de>:
>
> On 25.03.22 12:34, Jakub Jelinek via Fortran wrote:
>> What is the behavior with a RANGE_EXPR when one has { [0..10] = ++i;
>> }, is that applying the side-effects 11 times or once ?
>
> For side effects during the evaluation of expression, Fortran has a
> clear "if you depend on it, it's your fault" rule. In F 2018, it says
>
> 10.1.7 Evaluation of operands
>
> 1 It is not necessary for a processor to evaluate all of the operands of
> an expression, or to evaluate entirely each operand, if the value of the
> expression can be determined otherwise.
>
> Also, the semantics of
>
> a(a:b) = expr
>
> say that the expression on the LHS is evaluated only once before
> assignment. So, anything that looks like that should be translated
> to
>
> tmp = ++i;
> [0..10] = tmp;
Note I was not trying to question middle-end semantic here but gfortran se_expr (?) one. Is there a Fortan input where Jakob’s patch would cause a side-effect to be dropped and is that valid?
Richard.
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-03-26 17:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-25 8:57 [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691] Jakub Jelinek
2022-03-25 10:13 ` Tobias Burnus
2022-03-25 11:16 ` Richard Biener
2022-03-25 11:34 ` Jakub Jelinek
2022-03-25 12:13 ` Richard Biener
2022-03-25 12:15 ` Jakub Jelinek
2022-03-26 11:27 ` Thomas Koenig
2022-03-26 17:13 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).