Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
> Am 26.03.2022 um 12:28 schrieb Thomas Koenig :
>
> 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.
>
>
Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
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;
Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
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
Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
On Fri, Mar 25, 2022 at 12:34 PM Jakub Jelinek 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
> > 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
> > > >
> > > > 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
>
Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
On Fri, Mar 25, 2022 at 12:16:40PM +0100, Richard Biener wrote:
> On Fri, Mar 25, 2022 at 11:13 AM Tobias Burnus
> 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
> > >
> > > 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
Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
On Fri, Mar 25, 2022 at 11:13 AM Tobias Burnus 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
> >
> > 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.cc2022-03-24 16:14:58.334498775 +0100
> > @@ -6267,10 +6267,17 @@ gfc_conv_array_initializer (tree type, g
> > else
> > gfc_conv_structure (, 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
Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]
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
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.cc2022-03-24 16:14:58.334498775 +0100
@@ -6267,10 +6267,17 @@ gfc_conv_array_initializer (tree type, g
else
gfc_conv_structure (, 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
