[PATCH] fortran: Factor the evaluation of MINLOCK/MAXLOC's BACK argument
From: Mikael Morin
Hello,
I discovered this while testing the inline MINLOC/MAXLOC (aka PR90608) patches.
Regression tested on x86_64-linux.
OK for master?
-- 8< --
Move the evaluation of the BACK argument out of the loop in the inline code
generated for MINLOC or MAXLOC. For that, add a new (scalar) element
associated with BACK to the scalarization loop chain, evaluate the argument
with the context of that element, and let the scalarizer do its job.
The problem was not only a missed optimisation, but also a wrong code
one in the cases where the expression associated with BACK is not free of
side-effects, making multiple evaluations observable.
The new tests check the evaluation count of the BACK argument, and try to
cover all the variations (with/out NANs, constant or unknown shape, absent
or scalar or array MASK) supported by the inline implementation of the
functions. Care has been taken to not check the case of a constant .FALSE.
MASK, for which the evaluation of BACK can be elided.
gcc/fortran/ChangeLog:
* trans-intrinsic.cc (gfc_conv_intrinsic_minmaxloc): Create a new
scalar scalarization chain element if BACK is present. Add it to
the loop. Set the scalarization chain before evaluating the
argument.
gcc/testsuite/ChangeLog:
* gfortran.dg/maxloc_5.f90: New test.
* gfortran.dg/minloc_5.f90: New test.
---
gcc/fortran/trans-intrinsic.cc | 10 +
gcc/testsuite/gfortran.dg/maxloc_5.f90 | 257 +
gcc/testsuite/gfortran.dg/minloc_5.f90 | 257 +
3 files changed, 524 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/maxloc_5.f90
create mode 100644 gcc/testsuite/gfortran.dg/minloc_5.f90
diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
index 5ea10e84060..cadbd177452 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -5325,6 +5325,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
gfc_actual_arglist *actual;
gfc_ss *arrayss;
gfc_ss *maskss;
+ gfc_ss *backss;
gfc_se arrayse;
gfc_se maskse;
gfc_expr *arrayexpr;
@@ -5390,6 +5391,11 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
&& maskexpr->symtree->n.sym->attr.dummy
&& maskexpr->symtree->n.sym->attr.optional;
backexpr = actual->next->next->expr;
+ if (backexpr)
+backss = gfc_get_scalar_ss (gfc_ss_terminator, backexpr);
+ else
+backss = nullptr;
+
nonempty = NULL;
if (maskexpr && maskexpr->rank != 0)
{
@@ -5449,6 +5455,9 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
if (maskss)
gfc_add_ss_to_loop (, maskss);
+ if (backss)
+gfc_add_ss_to_loop (, backss);
+
gfc_add_ss_to_loop (, arrayss);
/* Initialize the loop. */
@@ -5535,6 +5544,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
gfc_add_block_to_block (, );
gfc_init_se (, NULL);
+ backse.ss = backss;
gfc_conv_expr_val (, backexpr);
gfc_add_block_to_block (, );
diff --git a/gcc/testsuite/gfortran.dg/maxloc_5.f90
b/gcc/testsuite/gfortran.dg/maxloc_5.f90
new file mode 100644
index 000..5d722450c8f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/maxloc_5.f90
@@ -0,0 +1,257 @@
+! { dg-do run }
+!
+! Check that the evaluation of MAXLOC's BACK argument is made only once
+! before the scalarisation loops.
+
+program p
+ implicit none
+ integer, parameter :: data10(*) = (/ 7, 4, 7, 6, 6, 4, 6, 3, 9, 8 /)
+ logical, parameter :: mask10(*) = (/ .false., .true., .false., &
+ .false., .true., .true., &
+ .true. , .true., .false., &
+ .false. /)
+ integer :: calls_count = 0
+ call check_int_const_shape
+ call check_int_const_shape_scalar_mask
+ call check_int_const_shape_array_mask
+ call check_int_const_shape_optional_mask_present
+ call check_int_const_shape_optional_mask_absent
+ call check_int_const_shape_empty
+ call check_int_alloc
+ call check_int_alloc_scalar_mask
+ call check_int_alloc_array_mask
+ call check_int_alloc_empty
+ call check_real_const_shape
+ call check_real_const_shape_scalar_mask
+ call check_real_const_shape_array_mask
+ call check_real_const_shape_optional_mask_present
+ call check_real_const_shape_optional_mask_absent
+ call check_real_const_shape_empty
+ call check_real_alloc
+ call check_real_alloc_scalar_mask
+ call check_real_alloc_array_mask
+ call check_real_alloc_empty
+contains
+ function get_scalar_false()
+logical :: get_scalar_false
+calls_count = calls_count + 1
+get_scalar_false = .false.
+ end function
+ subroutine check_int_const_shape()
+integer :: a(10)
+logical :: m(10)
+integer :: r
+a = data10
+
[gcc] Created branch 'mikael/heads/factor_back_minmaxloc_v01' in namespace 'refs/users'
The branch 'mikael/heads/factor_back_minmaxloc_v01' was created in namespace 'refs/users' pointing to: a04c0d344553... Sauvegarde tests
[gcc(refs/users/mikael/heads/factor_back_minmaxloc_v01)] Sauvegarde tests
https://gcc.gnu.org/g:a04c0d344553cc0b405977b3b9eac4ca504a299d
commit a04c0d344553cc0b405977b3b9eac4ca504a299d
Author: Mikael Morin
Date: Mon Jul 8 22:19:43 2024 +0200
Sauvegarde tests
Correction 11 18
Correction tests masque scalaire .false.
Diff:
---
gcc/fortran/trans-intrinsic.cc | 10 ++
gcc/testsuite/gfortran.dg/maxloc_5.f90 | 257 +
gcc/testsuite/gfortran.dg/minloc_5.f90 | 257 +
3 files changed, 524 insertions(+)
diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
index 5ea10e840609..cadbd1774520 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -5325,6 +5325,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
gfc_actual_arglist *actual;
gfc_ss *arrayss;
gfc_ss *maskss;
+ gfc_ss *backss;
gfc_se arrayse;
gfc_se maskse;
gfc_expr *arrayexpr;
@@ -5390,6 +5391,11 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
&& maskexpr->symtree->n.sym->attr.dummy
&& maskexpr->symtree->n.sym->attr.optional;
backexpr = actual->next->next->expr;
+ if (backexpr)
+backss = gfc_get_scalar_ss (gfc_ss_terminator, backexpr);
+ else
+backss = nullptr;
+
nonempty = NULL;
if (maskexpr && maskexpr->rank != 0)
{
@@ -5449,6 +5455,9 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
if (maskss)
gfc_add_ss_to_loop (, maskss);
+ if (backss)
+gfc_add_ss_to_loop (, backss);
+
gfc_add_ss_to_loop (, arrayss);
/* Initialize the loop. */
@@ -5535,6 +5544,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
gfc_add_block_to_block (, );
gfc_init_se (, NULL);
+ backse.ss = backss;
gfc_conv_expr_val (, backexpr);
gfc_add_block_to_block (, );
diff --git a/gcc/testsuite/gfortran.dg/maxloc_5.f90
b/gcc/testsuite/gfortran.dg/maxloc_5.f90
new file mode 100644
index ..5d722450c8fb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/maxloc_5.f90
@@ -0,0 +1,257 @@
+! { dg-do run }
+!
+! Check that the evaluation of MAXLOC's BACK argument is made only once
+! before the scalarisation loops.
+
+program p
+ implicit none
+ integer, parameter :: data10(*) = (/ 7, 4, 7, 6, 6, 4, 6, 3, 9, 8 /)
+ logical, parameter :: mask10(*) = (/ .false., .true., .false., &
+ .false., .true., .true., &
+ .true. , .true., .false., &
+ .false. /)
+ integer :: calls_count = 0
+ call check_int_const_shape
+ call check_int_const_shape_scalar_mask
+ call check_int_const_shape_array_mask
+ call check_int_const_shape_optional_mask_present
+ call check_int_const_shape_optional_mask_absent
+ call check_int_const_shape_empty
+ call check_int_alloc
+ call check_int_alloc_scalar_mask
+ call check_int_alloc_array_mask
+ call check_int_alloc_empty
+ call check_real_const_shape
+ call check_real_const_shape_scalar_mask
+ call check_real_const_shape_array_mask
+ call check_real_const_shape_optional_mask_present
+ call check_real_const_shape_optional_mask_absent
+ call check_real_const_shape_empty
+ call check_real_alloc
+ call check_real_alloc_scalar_mask
+ call check_real_alloc_array_mask
+ call check_real_alloc_empty
+contains
+ function get_scalar_false()
+logical :: get_scalar_false
+calls_count = calls_count + 1
+get_scalar_false = .false.
+ end function
+ subroutine check_int_const_shape()
+integer :: a(10)
+logical :: m(10)
+integer :: r
+a = data10
+calls_count = 0
+r = maxloc(a, dim = 1, back = get_scalar_false())
+if (calls_count /= 1) stop 11
+ end subroutine
+ subroutine check_int_const_shape_scalar_mask()
+integer :: a(10)
+integer :: r
+a = data10
+calls_count = 0
+! We only check the case of a .true. mask.
+! If the mask is .false., the back argument is not necessary to deduce
+! the value returned by maxloc, so the compiler is free to elide it,
+! and the value of calls_count is undefined in that case.
+r = maxloc(a, dim = 1, mask = .true., back = get_scalar_false())
+if (calls_count /= 1) stop 18
+ end subroutine
+ subroutine check_int_const_shape_array_mask()
+integer :: a(10)
+logical :: m(10)
+integer :: r
+a = data10
+m = mask10
+calls_count = 0
+r = maxloc(a, dim = 1, mask = m, back = get_scalar_false())
+if (calls_count /= 1) stop 32
+ end subroutine
+ subroutine call_maxloc_int(r, a, m, b)
+integer :: a(:)
+logical, optional :: m(:)
+logical, optional :: b
+integer :: r
+r = maxloc(a, dim = 1, mask = m, back = b)
+ end subroutine
+ subroutine check_int_const_shape_optional_mask_present()
+integer :: a(10)
+logical :: m(10)
+
[gcc] Deleted branch 'mikael/heads/cleanup_trans_preloop_setup_v01' in namespace 'refs/users'
The branch 'mikael/heads/cleanup_trans_preloop_setup_v01' in namespace 'refs/users' was deleted. It previously pointed to: cfcb4489798c... fortran: Move definition of variable closer to its usages Diff: !!! WARNING: THE FOLLOWING COMMITS ARE NO LONGER ACCESSIBLE (LOST): --- cfcb448... fortran: Move definition of variable closer to its usages
[gcc r15-1893] fortran: Move definition of variable closer to its uses
https://gcc.gnu.org/g:7183a8ca18d5889a1f66ec1edbda00200d700c6c
commit r15-1893-g7183a8ca18d5889a1f66ec1edbda00200d700c6c
Author: Mikael Morin
Date: Mon Jul 8 09:38:42 2024 +0200
fortran: Move definition of variable closer to its uses
No change of behaviour, this makes a variable easier to track.
gcc/fortran/ChangeLog:
* trans-array.cc (gfc_trans_preloop_setup): Use a separate variable
for iteration. Use directly the value of variable I if it is known.
Move the definition of the variable to the branch where the
remaining uses are.
Diff:
---
gcc/fortran/trans-array.cc | 33 +++--
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 510f429ef8ed..c7d244689393 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -4294,7 +4294,6 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
gfc_ss *ss, *pss;
gfc_loopinfo *ploop;
gfc_array_ref *ar;
- int i;
/* This code will be executed before entering the scalarization loop
for this dimension. */
@@ -4340,19 +4339,12 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
pss = ss;
}
- if (dim == loop->dimen - 1)
- i = 0;
- else
- i = dim + 1;
-
- /* For the time being, there is no loop reordering. */
- gcc_assert (i == ploop->order[i]);
- i = ploop->order[i];
-
if (dim == loop->dimen - 1 && loop->parent == NULL)
{
+ gcc_assert (0 == ploop->order[0]);
+
stride = gfc_conv_array_stride (info->descriptor,
- innermost_ss (ss)->dim[i]);
+ innermost_ss (ss)->dim[0]);
/* Calculate the stride of the innermost loop. Hopefully this will
allow the backend optimizers to do their stuff more effectively.
@@ -4364,7 +4356,7 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
base offset of the array. */
if (info->ref)
{
- for (i = 0; i < ar->dimen; i++)
+ for (int i = 0; i < ar->dimen; i++)
{
if (ar->dimen_type[i] != DIMEN_ELEMENT)
continue;
@@ -4374,8 +4366,21 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
}
}
else
- /* Add the offset for the previous loop dimension. */
- add_array_offset (pblock, ploop, ss, ar, pss->dim[i], i);
+ {
+ int i;
+
+ if (dim == loop->dimen - 1)
+ i = 0;
+ else
+ i = dim + 1;
+
+ /* For the time being, there is no loop reordering. */
+ gcc_assert (i == ploop->order[i]);
+ i = ploop->order[i];
+
+ /* Add the offset for the previous loop dimension. */
+ add_array_offset (pblock, ploop, ss, ar, pss->dim[i], i);
+ }
/* Remember this offset for the second loop. */
if (dim == loop->temp_dim - 1 && loop->parent == NULL)
[PATCH] fortran: Remove useless nested end of scalarization chain handling
Hello,
this is another small cleanup I had lying around.
Regression-tested on x86_64-linux. Ok for master?
-- 8< --
Remove the special handling of end of nested scalarization chains, which
advanced the chain to an element of a parent chain when the current one
was reaching its end.
That handling was superfluous as nested chains correspond to nested
scalarizations of subexpressions and the scalarizations don't extend beyond
their associated subexpression and don't use any scalarisation element from
the parent expression.
No change of behaviour, as the GFC_SE struct is supposed to be in its final
state anyway when the last element from the chain has been consumed.
gcc/fortran/ChangeLog:
* trans-expr.cc (gfc_advance_se_ss_chain): Don't use an element
from the parent scalarization chain when the current chain reaches
its end.
---
gcc/fortran/trans-expr.cc | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 477c2720187..f0862db5f17 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -2052,7 +2052,6 @@ void
gfc_advance_se_ss_chain (gfc_se * se)
{
gfc_se *p;
- gfc_ss *ss;
gcc_assert (se != NULL && se->ss != NULL && se->ss != gfc_ss_terminator);
@@ -2064,15 +2063,7 @@ gfc_advance_se_ss_chain (gfc_se * se)
gcc_assert (p->parent == NULL || p->parent->ss == p->ss
|| p->parent->ss->nested_ss == p->ss);
- /* If we were in a nested loop, the next scalarized expression can be
-on the parent ss' next pointer. Thus we should not take the next
-pointer blindly, but rather go up one nest level as long as next
-is the end of chain. */
- ss = p->ss;
- while (ss->next == gfc_ss_terminator && ss->parent != NULL)
- ss = ss->parent;
-
- p->ss = ss->next;
+ p->ss = p->ss->next;
p = p->parent;
}
--
2.43.0
[PATCH] fortran: Move definition of variable closer to its uses
Hello,
I have found this small cleanup lying in a local branch.
Regression-tested on x86_64-linux, OK for master?
-- 8< --
No change of behaviour, this makes a variable easier to track.
gcc/fortran/ChangeLog:
* trans-array.cc (gfc_trans_preloop_setup): Use a separate variable
for iteration. Use directly the value of variable I if it is known.
Move the definition of the variable to the branch where the
remaining uses are.
---
gcc/fortran/trans-array.cc | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 510f429ef8e..c34c97257a9 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -4294,7 +4294,6 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
gfc_ss *ss, *pss;
gfc_loopinfo *ploop;
gfc_array_ref *ar;
- int i;
/* This code will be executed before entering the scalarization loop
for this dimension. */
@@ -4340,19 +4339,10 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
pss = ss;
}
- if (dim == loop->dimen - 1)
- i = 0;
- else
- i = dim + 1;
-
- /* For the time being, there is no loop reordering. */
- gcc_assert (i == ploop->order[i]);
- i = ploop->order[i];
-
if (dim == loop->dimen - 1 && loop->parent == NULL)
{
stride = gfc_conv_array_stride (info->descriptor,
- innermost_ss (ss)->dim[i]);
+ innermost_ss (ss)->dim[0]);
/* Calculate the stride of the innermost loop. Hopefully this will
allow the backend optimizers to do their stuff more effectively.
@@ -4364,7 +4354,7 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
base offset of the array. */
if (info->ref)
{
- for (i = 0; i < ar->dimen; i++)
+ for (int i = 0; i < ar->dimen; i++)
{
if (ar->dimen_type[i] != DIMEN_ELEMENT)
continue;
@@ -4374,8 +4364,21 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
}
}
else
- /* Add the offset for the previous loop dimension. */
- add_array_offset (pblock, ploop, ss, ar, pss->dim[i], i);
+ {
+ int i;
+
+ if (dim == loop->dimen - 1)
+ i = 0;
+ else
+ i = dim + 1;
+
+ /* For the time being, there is no loop reordering. */
+ gcc_assert (i == ploop->order[i]);
+ i = ploop->order[i];
+
+ /* Add the offset for the previous loop dimension. */
+ add_array_offset (pblock, ploop, ss, ar, pss->dim[i], i);
+ }
/* Remember this offset for the second loop. */
if (dim == loop->temp_dim - 1 && loop->parent == NULL)
--
2.43.0
[gcc(refs/users/mikael/heads/cleanup_trans_preloop_setup_v01)] fortran: Move definition of variable closer to its usages
https://gcc.gnu.org/g:cfcb4489798cfb9715e8cf92bb8eadcfe35dfd21
commit cfcb4489798cfb9715e8cf92bb8eadcfe35dfd21
Author: Mikael Morin
Date: Mon Nov 20 10:16:31 2023 +0100
fortran: Move definition of variable closer to its usages
No change of behaviour, this makes a variable easier to track.
gcc/fortran/ChangeLog:
* trans-array.cc (gfc_trans_preloop_setup): Use a separate variable
for iteration. Use directly the value of variable I if it is known.
Move the definition of the variable to the branch where the
remaining uses are.
Diff:
---
gcc/fortran/trans-array.cc | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 510f429ef8e..c34c97257a9 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -4294,7 +4294,6 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
gfc_ss *ss, *pss;
gfc_loopinfo *ploop;
gfc_array_ref *ar;
- int i;
/* This code will be executed before entering the scalarization loop
for this dimension. */
@@ -4340,19 +4339,10 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
pss = ss;
}
- if (dim == loop->dimen - 1)
- i = 0;
- else
- i = dim + 1;
-
- /* For the time being, there is no loop reordering. */
- gcc_assert (i == ploop->order[i]);
- i = ploop->order[i];
-
if (dim == loop->dimen - 1 && loop->parent == NULL)
{
stride = gfc_conv_array_stride (info->descriptor,
- innermost_ss (ss)->dim[i]);
+ innermost_ss (ss)->dim[0]);
/* Calculate the stride of the innermost loop. Hopefully this will
allow the backend optimizers to do their stuff more effectively.
@@ -4364,7 +4354,7 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
base offset of the array. */
if (info->ref)
{
- for (i = 0; i < ar->dimen; i++)
+ for (int i = 0; i < ar->dimen; i++)
{
if (ar->dimen_type[i] != DIMEN_ELEMENT)
continue;
@@ -4374,8 +4364,21 @@ gfc_trans_preloop_setup (gfc_loopinfo * loop, int dim,
int flag,
}
}
else
- /* Add the offset for the previous loop dimension. */
- add_array_offset (pblock, ploop, ss, ar, pss->dim[i], i);
+ {
+ int i;
+
+ if (dim == loop->dimen - 1)
+ i = 0;
+ else
+ i = dim + 1;
+
+ /* For the time being, there is no loop reordering. */
+ gcc_assert (i == ploop->order[i]);
+ i = ploop->order[i];
+
+ /* Add the offset for the previous loop dimension. */
+ add_array_offset (pblock, ploop, ss, ar, pss->dim[i], i);
+ }
/* Remember this offset for the second loop. */
if (dim == loop->temp_dim - 1 && loop->parent == NULL)
[gcc] Created branch 'mikael/heads/cleanup_trans_preloop_setup_v01' in namespace 'refs/users'
The branch 'mikael/heads/cleanup_trans_preloop_setup_v01' was created in namespace 'refs/users' pointing to: cfcb4489798... fortran: Move definition of variable closer to its usages
[gcc(refs/users/mikael/heads/cleanup_advance_se_ss_chain_v01)] fortran: Remove useless nested end of scalarization chain handling
https://gcc.gnu.org/g:c633926356155181081154c094010d672e715a4e
commit c633926356155181081154c094010d672e715a4e
Author: Mikael Morin
Date: Mon Nov 20 16:15:45 2023 +0100
fortran: Remove useless nested end of scalarization chain handling
Remove the special handling of end of nested scalarization chains, which
advanced the chain to an element of a parent chain when the current one
was reaching its end.
That handling was superfluous as nested chains correspond to nested
scalarizations of subexpressions and the scalarizations don't extend beyond
their associated subexpression and don't use any scalarisation element from
the parent expression.
No change in behaviour, as the GFC_SE struct is supposed to be at its final
state anyway when the last element from the chain has been consumed.
gcc/fortran/ChangeLog:
* trans-expr.cc (gfc_advance_se_ss_chain): Don't use an element
from the parent scalarization chain when the current chain reaches
its end.
Diff:
---
gcc/fortran/trans-expr.cc | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 477c2720187..f0862db5f17 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -2052,7 +2052,6 @@ void
gfc_advance_se_ss_chain (gfc_se * se)
{
gfc_se *p;
- gfc_ss *ss;
gcc_assert (se != NULL && se->ss != NULL && se->ss != gfc_ss_terminator);
@@ -2064,15 +2063,7 @@ gfc_advance_se_ss_chain (gfc_se * se)
gcc_assert (p->parent == NULL || p->parent->ss == p->ss
|| p->parent->ss->nested_ss == p->ss);
- /* If we were in a nested loop, the next scalarized expression can be
-on the parent ss' next pointer. Thus we should not take the next
-pointer blindly, but rather go up one nest level as long as next
-is the end of chain. */
- ss = p->ss;
- while (ss->next == gfc_ss_terminator && ss->parent != NULL)
- ss = ss->parent;
-
- p->ss = ss->next;
+ p->ss = p->ss->next;
p = p->parent;
}
[gcc] Created branch 'mikael/heads/cleanup_advance_se_ss_chain_v01' in namespace 'refs/users'
The branch 'mikael/heads/cleanup_advance_se_ss_chain_v01' was created in namespace 'refs/users' pointing to: c6339263561... fortran: Remove useless nested end of scalarization chain h
[gcc(refs/users/mikael/heads/non_lvalue_match.pd_v01)] match: Unwrap non-lvalue as unary or binary operand
https://gcc.gnu.org/g:b3cc0b4da1b94e3b3c2895011ab8d19d1268c34b
commit b3cc0b4da1b94e3b3c2895011ab8d19d1268c34b
Author: Mikael Morin
Date: Thu Jul 4 15:24:36 2024 +0200
match: Unwrap non-lvalue as unary or binary operand
gcc/ChangeLog:
* match.pd: Unwrap NON_LVALUE_EXPR trees when they are used as
operand of a unary or binary operator.
gcc/testsuite/ChangeLog:
* gfortran.dg/non_lvalue_2.f90: New test.
Diff:
---
gcc/match.pd | 12
gcc/testsuite/gfortran.dg/non_lvalue_2.f90 | 44 ++
2 files changed, 56 insertions(+)
diff --git a/gcc/match.pd b/gcc/match.pd
index d0859545ada..513df562fd7 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -280,6 +280,18 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(outer_op @0 @2)
@3))
+/* Remove superfluous NON_LVALUE_EXPR in unary operators. */
+(for op (UNCOND_UNARY)
+ (simplify (op (non_lvalue @0))
+ (op @0)))
+
+/* Remove superfluous NON_LVALUE_EXPR in binary operators. */
+(for op (UNCOND_BINARY tcc_comparison)
+ (simplify (op (non_lvalue @0) @1)
+ (op @0 @1))
+ (simplify (op @0 (non_lvalue @1))
+ (op @0 @1)))
+
/* Simplify x - x.
This is unsafe for certain floats even in non-IEEE formats.
In IEEE, it is unsafe because it does wrong for NaNs.
diff --git a/gcc/testsuite/gfortran.dg/non_lvalue_2.f90
b/gcc/testsuite/gfortran.dg/non_lvalue_2.f90
new file mode 100644
index 000..8c3197eab1f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/non_lvalue_2.f90
@@ -0,0 +1,44 @@
+! { dg-do compile }
+! { dg-additional-options "-fdump-tree-original" }
+!
+! Check the removal of NON_LVALUE_EXPR if they are used in a non-lvalue context
+
+! The NON_LVALUE_EXPR is dropped if it's part (left operand) of a bigger
expression
+function f1 (f1_arg1, f1_arg2)
+ integer, value :: f1_arg1, f1_arg2
+ integer :: f1
+ f1 = (f1_arg1 + 0) + f1_arg2
+end function
+! { dg-final { scan-tree-dump "__result_f1 = f1_arg1 \\+ f1_arg2;" "original"
} }
+
+! The NON_LVALUE_EXPR is dropped if it's part (right operand) of a bigger
expression
+function f2 (f2_arg1, f2_arg2)
+ integer, value :: f2_arg1, f2_arg2
+ integer :: f2
+ f2 = f2_arg1 + (f2_arg2 + 0)
+end function
+! { dg-final { scan-tree-dump "__result_f2 = f2_arg1 \\+ f2_arg2;" "original"
} }
+
+! The NON_LVALUE_EXPR is dropped if it's part (left operand) of a binary
logical operator
+function f3 (f3_arg1)
+ integer, value :: f3_arg1
+ logical :: f3
+ f3 = (f3_arg1 + 0) > 0
+end function
+! { dg-final { scan-tree-dump "__result_f3 = f3_arg1 > 0;" "original" } }
+
+! The NON_LVALUE_EXPR is dropped if it's part (right operand) of a binary
logical operator
+function f4 (f4_arg1, f4_arg2)
+ integer, value :: f4_arg1, f4_arg2
+ logical :: f4
+ f4 = f4_arg1 > (f4_arg2 + 0)
+end function
+! { dg-final { scan-tree-dump "__result_f4 = f4_arg1 > f4_arg2;" "original" } }
+
+! The NON_LVALUE_EXPR is dropped if it's part of a unary operator
+function f5 (f5_arg1)
+ integer, value :: f5_arg1
+ integer :: f5
+ f5 = -(not(not(f5_arg1)))
+end function
+! { dg-final { scan-tree-dump "__result_f5 = -f5_arg1;" "original" } }
[gcc(refs/users/mikael/heads/non_lvalue_match.pd_v01)] match: Simplify double not and double negate to a non_lvalue
https://gcc.gnu.org/g:540cb6b0dd2a9ece734e927d520a9ca15e8afff8
commit 540cb6b0dd2a9ece734e927d520a9ca15e8afff8
Author: Mikael Morin
Date: Thu Jul 4 12:59:34 2024 +0200
match: Simplify double not and double negate to a non_lvalue
gcc/ChangeLog:
* match.pd: Add a NON_LVALUE_EXPR wrapper around the simplification
of doubled unary operators NEGATE_EXPR and BIT_NOT_EXPR.
gcc/testsuite/ChangeLog:
* gfortran.dg/non_lvalue_1.f90: New test.
Diff:
---
gcc/match.pd | 4 ++--
gcc/testsuite/gfortran.dg/non_lvalue_1.f90 | 21 +
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/gcc/match.pd b/gcc/match.pd
index 4edfa2ae2c9..d0859545ada 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -2294,7 +2294,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* ~~x -> x */
(simplify
(bit_not (bit_not @0))
- @0)
+ (non_lvalue @0))
/* zero_one_valued_p will match when a value is known to be either
0 or 1 including constants 0 or 1.
@@ -3674,7 +3674,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(negate (nop_convert? (negate @1)))
(if (!TYPE_OVERFLOW_SANITIZED (type)
&& !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@1)))
- (view_convert @1)))
+ (non_lvalue (view_convert @1
/* We can't reassociate floating-point unless -fassociative-math
or fixed-point plus or minus because of saturation to +-Inf. */
diff --git a/gcc/testsuite/gfortran.dg/non_lvalue_1.f90
b/gcc/testsuite/gfortran.dg/non_lvalue_1.f90
new file mode 100644
index 000..ac52b272094
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/non_lvalue_1.f90
@@ -0,0 +1,21 @@
+! { dg-do compile }
+! { dg-additional-options "-fdump-tree-original" }
+!
+! Check the generation of NON_LVALUE_EXPR trees in cases where a unary
operator expression
+! simplifies to a data reference.
+
+! A NON_LVALUE_EXPR is generated for a double negation that simplifies to a
data reference. */
+function f1 (f1_arg1)
+ integer, value :: f1_arg1
+ integer :: f1
+ f1 = -(-f1_arg1)
+end function
+! { dg-final { scan-tree-dump "__result_f1 = NON_LVALUE_EXPR ;"
"original" } }
+
+! A NON_LVALUE_EXPR is generated for a double complement that simplifies to a
data reference. */
+function f2 (f2_arg1)
+ integer, value :: f2_arg1
+ integer :: f2
+ f2 = not(not(f2_arg1))
+end function
+! { dg-final { scan-tree-dump "__result_f2 = NON_LVALUE_EXPR ;"
"original" } }
[gcc] Created branch 'mikael/heads/non_lvalue_match.pd_v01' in namespace 'refs/users'
The branch 'mikael/heads/non_lvalue_match.pd_v01' was created in namespace 'refs/users' pointing to: b3cc0b4da1b... match: Unwrap non-lvalue as unary or binary operand
Re: [PATCH] Fortran: fix passing of optional dummy as actual to optional argument [PR55978]
Le 23/06/2024 à 22:58, Harald Anlauf a écrit : Dear all, the attached patch fixes issues exhibited by the testcase in comment#19 of PR55978. First, when passing an allocatable optional dummy array to an optional dummy, we need to prevent accessing the data component of the array when the argument is not present, and pass a null pointer instead. This is straightforward. Second, the case of a missing pointer optional dummy array should have worked, but the presence check surprisingly did not work as expected at -O0 or -Og, but at higher optimization levels. Interestingly, the dump-tree looked right, but running under gdb or investigating the assembler revealed that the order of tests in a logical AND expression was opposed to what the tree-dump looked like. Replacing TRUTH_AND_EXPR by TRUTH_ANDIF_EXPR and checking the optimized dump confirmed that this does fix the issue. Note that the tree-dump is not changed by this replacement. Does this mean thar AND and ANDIF currently are not differentiated at this level? tree-pretty-print.cc's op_symbol_code handles them as: case TRUTH_AND_EXPR: case TRUTH_ANDIF_EXPR: return "&&"; so no, I don't think they are differentiated. Regtested on x86_64-pc-linux-gnu. OK for mainline? Would it be ok to backport this to 14-branch, too? Sure, OK for both. Thanks.
Re: gcc git locked out for hours second day in a row
Hello, Le 12/06/2024 à 16:57, Jakub Jelinek a écrit : On Wed, Jun 12, 2024 at 04:53:38PM +0200, Mikael Morin wrote: Perhaps you could create a mirror version of the repo and do some experiments locally on that to identify where the bottle-neck is coming from? Not sure where to start for that.Are the hooks published somewhere? Yes: https://github.com/AdaCore/git-hooks/tree/master Note, we use some tweaks on top of that, but that is mostly for the release branches and trunk, so it would be interesting to just try to reproduce that with the stock AdaCore git hooks. I have finally taken some time to investigate this hook slowness, and here are my findings. My tests were run with configs commit-extra-checker and commit-email-formatter disabled, and hooks.update-hook set to a minimal script (either "true" or "sleep 1"). With that config, I could not reproduce the slowness pushing to refs/users/mikael/*. The push finishes in less than a minute. However, trying to push to a normal tag, there is some email count check coming into play, and I can reproduce some slowness (details below). This email count check shouldn't happen on the gcc repository in my use case (as email checks don't apply to user references), but the slowness could well happen in other cases than email count check depending on the configuration, as the problem relates to the size of the list of new commits and is not restricted to email count. Anyway, even with email count check triggering, each tag takes less than 2 minutes to be rejected in my test. With 330 tags to process, that would make an upper bound of 11 hours before rejecting the push in my test (I killed it after a few minutes). On the other hand, with the information you gave upthread, the hook on the gcc repository seemed to be still processing the first tag after a few hours (assuming they are processed in alphabetical order, which seems to be the case). So this still doesn't explain what was happening on the gcc repository. Regarding the email count check slowness I mentioned above, I traced it back to the updates.AbstractUpdate class, whose (procedural) new_commits_for_ref attribute is a list of "new" commits, containing both really new commits and commits newly on the branch to be updated, but already known to the repository. For a tag or branch creation, a list of "new on the branch" commits would be huge as everything is new, so parent commits of the oldest "repository-new" commit are not picked up. But in my test the list still amounts to a little less than 80,000 commits, basically what happened on trunk in the last 8 years. Anything that walks such a big list is bound to be slow. To sum up: - The hooks support checking "new on the branch" commits additionally to "new on the repository" commits, and that is a feature, not a bug. - In my use case, that means that the hooks process 80,000 commits, even if only 330 of them are new on the repository. - As the hook is called on a per-reference basis, the same commits would be processed over and over again for every reference in my use case, so the best would be to push them one by one, in order. - I still don't know why it took hours (without even finishing) to process just one tag the other day on the gcc repository. Nikael
Re: gcc git locked out for hours second day in a row
Le 12/06/2024 à 16:34, Richard Earnshaw (lists) a écrit : On 12/06/2024 14:23, Mikael Morin via Gcc wrote: Le 12/06/2024 à 14:58, Jonathan Wakely a écrit : On Wed, 12 Jun 2024 at 13:57, Mikael Morin via Gcc wrote: Le 12/06/2024 à 13:48, Jakub Jelinek a écrit : Hi! Yesterday the gcc git repository was locked for 3 hours locked by user mikael at 2024-06-11 13:27:44.301067 (pid = 974167) 78:06 python hooks/update.py refs/users/mikael/tags/fortran-dev_merges/r10-1545 c2f9fe1d8111b9671bf0aa8362446516fd942f1d process until overseers killed it but today we have the same situation for 3 ours and counting again: locked by user mikael at 2024-06-12 08:35:48.137564 (pid = 2219652) 78:06 python hooks/update.py refs/users/mikael/tags/toto cca005166dba2cefeb51afac3ea629b3972acea3 It is possible we have some bug in the git hook scripts, but it would be helpful trying to understand what exactly you're trying to commit and why nobody else (at least to my knowledge) has similarly stuck commits. The effect is that nobody can push anything else to gcc git repo for hours. Jakub Yes, sorry for the inconvenience. I tried pushing a series of tags labeling merge points between the fortran-dev branch and recent years master. Just pushing tags should not cause a problem, assuming all the commits being tagged already exist. What exactly are you pushing? Well, the individual commits to be merged do exist, but the merge points don't and they are what I'm trying to push. To be clear, the branch hasn't seen any update for years, and I'm trying to reapply what happened on trunk since, in a step-wise manner. With 300 merges I'm summing up 6 commits of history. The number of merge points is a bit high (329) but I expected it to be a manageable number. I tried again today with just the most recent merge point, but it got stuck again. I should try with the oldest one, but I'm afraid locking the repository again. I waited for the push to finish for say one hour before killing it yesterday, and no more than 15 minutes today. Unfortunately, killing the process doesn't seem to unlock things on the server side. It may be a misconfiguration on my side, but I have never had this problem before. Sorry again. Perhaps you could create a mirror version of the repo and do some experiments locally on that to identify where the bottle-neck is coming from? Not sure where to start for that.Are the hooks published somewhere?
Re: gcc git locked out for hours second day in a row
Le 12/06/2024 à 14:58, Jonathan Wakely a écrit : On Wed, 12 Jun 2024 at 13:57, Mikael Morin via Gcc wrote: Le 12/06/2024 à 13:48, Jakub Jelinek a écrit : Hi! Yesterday the gcc git repository was locked for 3 hours locked by user mikael at 2024-06-11 13:27:44.301067 (pid = 974167) 78:06 python hooks/update.py refs/users/mikael/tags/fortran-dev_merges/r10-1545 c2f9fe1d8111b9671bf0aa8362446516fd942f1d process until overseers killed it but today we have the same situation for 3 ours and counting again: locked by user mikael at 2024-06-12 08:35:48.137564 (pid = 2219652) 78:06 python hooks/update.py refs/users/mikael/tags/toto cca005166dba2cefeb51afac3ea629b3972acea3 It is possible we have some bug in the git hook scripts, but it would be helpful trying to understand what exactly you're trying to commit and why nobody else (at least to my knowledge) has similarly stuck commits. The effect is that nobody can push anything else to gcc git repo for hours. Jakub Yes, sorry for the inconvenience. I tried pushing a series of tags labeling merge points between the fortran-dev branch and recent years master. Just pushing tags should not cause a problem, assuming all the commits being tagged already exist. What exactly are you pushing? Well, the individual commits to be merged do exist, but the merge points don't and they are what I'm trying to push. To be clear, the branch hasn't seen any update for years, and I'm trying to reapply what happened on trunk since, in a step-wise manner. With 300 merges I'm summing up 6 commits of history. The number of merge points is a bit high (329) but I expected it to be a manageable number. I tried again today with just the most recent merge point, but it got stuck again. I should try with the oldest one, but I'm afraid locking the repository again. I waited for the push to finish for say one hour before killing it yesterday, and no more than 15 minutes today. Unfortunately, killing the process doesn't seem to unlock things on the server side. It may be a misconfiguration on my side, but I have never had this problem before. Sorry again.
Re: gcc git locked out for hours second day in a row
Le 12/06/2024 à 14:14, Mark Wielaard a écrit : Hi, On Wed, 2024-06-12 at 13:48 +0200, Jakub Jelinek via Gcc wrote: Yesterday the gcc git repository was locked for 3 hours locked by user mikael at 2024-06-11 13:27:44.301067 (pid = 974167) 78:06 python hooks/update.py refs/users/mikael/tags/fortran-dev_merges/r10-1545 c2f9fe1d8111b9671bf0aa8362446516fd942f1d process until overseers killed it but today we have the same situation for 3 ours and counting again: locked by user mikael at 2024-06-12 08:35:48.137564 (pid = 2219652) 78:06 python hooks/update.py refs/users/mikael/tags/toto cca005166dba2cefeb51afac3ea629b3972acea3 Just for the record, I kill the process and removed the git- hooks::update.token.lock file. Sorry that killed mikael's push, but hopefully that makes it possible for others to push again. No problem, I had interrupted the push long before on my side.
Re: gcc git locked out for hours second day in a row
Le 12/06/2024 à 13:48, Jakub Jelinek a écrit : Hi! Yesterday the gcc git repository was locked for 3 hours locked by user mikael at 2024-06-11 13:27:44.301067 (pid = 974167) 78:06 python hooks/update.py refs/users/mikael/tags/fortran-dev_merges/r10-1545 c2f9fe1d8111b9671bf0aa8362446516fd942f1d process until overseers killed it but today we have the same situation for 3 ours and counting again: locked by user mikael at 2024-06-12 08:35:48.137564 (pid = 2219652) 78:06 python hooks/update.py refs/users/mikael/tags/toto cca005166dba2cefeb51afac3ea629b3972acea3 It is possible we have some bug in the git hook scripts, but it would be helpful trying to understand what exactly you're trying to commit and why nobody else (at least to my knowledge) has similarly stuck commits. The effect is that nobody can push anything else to gcc git repo for hours. Jakub Yes, sorry for the inconvenience. I tried pushing a series of tags labeling merge points between the fortran-dev branch and recent years master. The number of merge points is a bit high (329) but I expected it to be a manageable number. I tried again today with just the most recent merge point, but it got stuck again. I should try with the oldest one, but I'm afraid locking the repository again. I waited for the push to finish for say one hour before killing it yesterday, and no more than 15 minutes today. Unfortunately, killing the process doesn't seem to unlock things on the server side. It may be a misconfiguration on my side, but I have never had this problem before. Sorry again.
Re: [COMMITTED 01/12] - Move all relation queries into relation_oracle.
Hello...
Le 23/05/2024 à 22:52, Andrew MacLeod a écrit :
A range-query currently provides a couple of relation query routines,
plus it also provides direct access to an oracle. This patch moves
those queries into the oracle where they should be, and ands the ability
to create and destroy the basic dominance oracle ranger uses. This is
the usual oracle most passes would want, and this provides full access
to it if ranger has been enabled. It also allows passes which do not
use ranger to turn on an oracle and work with it.
Full documentation for relations and the oracle can be found at:
https://gcc.gnu.org/wiki/AndrewMacLeod/Relations
Moving the queries into the oracle removes the need to check for a NULL
pointer on every query, and results in speeding up VRP by about 0.7%
Bootstrapped on x86_64-pc-linux-gnu with no regressions. Pushed.
(...)
diff --git a/gcc/value-query.cc b/gcc/value-query.cc
index c2ab745a466..b275a43b679 100644
--- a/gcc/value-query.cc
+++ b/gcc/value-query.cc
@@ -437,53 +439,3 @@ global_range_query::range_of_expr (vrange , tree expr,
gimple *stmt)
return true;
}
-
-// Return any known relation between SSA1 and SSA2 before stmt S is executed.
-// If GET_RANGE is true, query the range of both operands first to ensure
-// the definitions have been processed and any relations have be created.
-
-relation_kind
-range_query::query_relation (gimple *s, tree ssa1, tree ssa2, bool get_range)
> -{
- if (!m_oracle || TREE_CODE (ssa1) != SSA_NAME || TREE_CODE (ssa2) !=
SSA_NAME)
-return VREL_VARYING;
-
- // Ensure ssa1 and ssa2 have both been evaluated.
- if (get_range)
-{
- Value_Range tmp1 (TREE_TYPE (ssa1));
- Value_Range tmp2 (TREE_TYPE (ssa2));
- range_of_expr (tmp1, ssa1, s);
- range_of_expr (tmp2, ssa2, s);
-}
- return m_oracle->query_relation (gimple_bb (s), ssa1, ssa2);
-}
-
-// Return any known relation between SSA1 and SSA2 on edge E.
-// If GET_RANGE is true, query the range of both operands first to ensure
-// the definitions have been processed and any relations have be created.
-
-relation_kind
-range_query::query_relation (edge e, tree ssa1, tree ssa2, bool get_range)
-{
- basic_block bb;
- if (!m_oracle || TREE_CODE (ssa1) != SSA_NAME || TREE_CODE (ssa2) !=
SSA_NAME)
-return VREL_VARYING;
-
- // Use destination block if it has a single predecessor, and this picks
- // up any relation on the edge.
- // Otherwise choose the src edge and the result is the same as on-exit.
- if (!single_pred_p (e->dest))
-bb = e->src;
- else
-bb = e->dest;
-
- // Ensure ssa1 and ssa2 have both been evaluated.
- if (get_range)
-{
- Value_Range tmp (TREE_TYPE (ssa1));
- range_on_edge (tmp, e, ssa1);
- range_on_edge (tmp, e, ssa2);
-}
- return m_oracle->query_relation (bb, ssa1, ssa2);
-}
(...)
diff --git a/gcc/value-relation.cc b/gcc/value-relation.cc
index 619ee5f0867..d1081b3b3f5 100644
--- a/gcc/value-relation.cc
+++ b/gcc/value-relation.cc
@@ -288,6 +288,39 @@ relation_oracle::valid_equivs (bitmap b, const_bitmap
equivs, basic_block bb)
}
}
+// Return any known relation between SSA1 and SSA2 before stmt S is executed.
+// If GET_RANGE is true, query the range of both operands first to ensure
+// the definitions have been processed and any relations have be created.
The method lost its argument GET_RANGE in the move from value-query.cc,
so the documentation for GET_RANGE can be removed as well.
+
+relation_kind
+relation_oracle::query_relation (gimple *s, tree ssa1, tree ssa2)
+{
+ if (TREE_CODE (ssa1) != SSA_NAME || TREE_CODE (ssa2) != SSA_NAME)
+return VREL_VARYING;
+ return query_relation (gimple_bb (s), ssa1, ssa2);
+}
+
+// Return any known relation between SSA1 and SSA2 on edge E.
+// If GET_RANGE is true, query the range of both operands first to ensure
+// the definitions have been processed and any relations have be created.
Same here.
+
+relation_kind
+relation_oracle::query_relation (edge e, tree ssa1, tree ssa2)
+{
+ basic_block bb;
+ if (TREE_CODE (ssa1) != SSA_NAME || TREE_CODE (ssa2) != SSA_NAME)
+return VREL_VARYING;
+
+ // Use destination block if it has a single predecessor, and this picks
+ // up any relation on the edge.
+ // Otherwise choose the src edge and the result is the same as on-exit.
+ if (!single_pred_p (e->dest))
+bb = e->src;
+ else
+bb = e->dest;
+
+ return query_relation (bb, ssa1, ssa2);
+}
// -
// The very first element in the m_equiv chain is actually just a summary
Re: [COMMITTED] [prange] Drop range to VARYING if the bitmask intersection made it so [PR115131]
Hello,
Le 17/05/2024 à 16:03, Aldy Hernandez a écrit :
If the intersection of the bitmasks made the range span the entire
domain, normalize the range to VARYING.
gcc/ChangeLog:
PR middle-end/115131
* value-range.cc (prange::intersect): Set VARYING if intersection
of bitmasks made the range span the entire domain.
(range_tests_misc): New test.
---
gcc/value-range.cc | 21 +
1 file changed, 21 insertions(+)
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 334ffb70fbc..b38d6159a85 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -589,6 +589,11 @@ prange::intersect (const vrange )
irange_bitmask new_bitmask = get_bitmask_from_range (m_type, m_min, m_max);
m_bitmask.intersect (new_bitmask);
m_bitmask.intersect (r.m_bitmask);
+ if (varying_compatible_p ())
+{
+ set_varying (type ());
+ return true;
+}
if (flag_checking)
verify_range ();
@@ -2889,6 +2894,22 @@ range_tests_misc ()
p0.invert ();
ASSERT_TRUE (p0 == p1);
+ // The intersection of:
+ //[0, +INF] MASK 0xff..00 VALUE 0xf8
+ //[0, +INF] MASK 0xff..00 VALUE 0x00
+ // is [0, +INF] MASK 0xff..ff VALUE 0x00, which is VARYING.
+ // Test that we normalized to VARYING.
+ unsigned prec = TYPE_PRECISION (voidp);
+ p0.set_varying (voidp);
+ wide_int mask = wi::mask (8, true, prec);
+ wide_int value = wi::uhwi (0xf8, prec);
+ irange_bitmask bm (wi::uhwi (0xf8, prec), mask);
+ p0.update_bitmask (bm);
+ p1.set_varying (voidp);
+ bm = irange_bitmask (wi::zero (prec), mask);
+ p1.update_bitmask (bm);
+ p0.intersect (p1);
Isn't a check missing here,
ASSERT_TRUE (p0.varying_p ())
or something?
+
// [10,20] U [15, 30] => [10, 30].
r0 = range_int (10, 20);
r1 = range_int (15, 30);
[gcc] Deleted branch 'mikael/heads/pr93635-v2_Harald' in namespace 'refs/users'
The branch 'mikael/heads/pr93635-v2_Harald' in namespace 'refs/users' was deleted. It previously pointed to: 1ea6d9d7f54... Fortran: improve attribute conflict checking [PR93635] Diff: !!! WARNING: THE FOLLOWING COMMITS ARE NO LONGER ACCESSIBLE (LOST): --- 1ea6d9d... Fortran: improve attribute conflict checking [PR93635]
Re: [PATCH, v2] Fortran: improve attribute conflict checking [PR93635]
Le 23/05/2024 à 21:15, Harald Anlauf a écrit : Hi Mikael, On 5/23/24 09:49, Mikael Morin wrote: Le 13/05/2024 à 09:25, Mikael Morin a écrit : Le 10/05/2024 à 21:56, Harald Anlauf a écrit : Am 10.05.24 um 21:48 schrieb Harald Anlauf: Hi Mikael, Am 10.05.24 um 11:45 schrieb Mikael Morin: Le 09/05/2024 à 22:30, Harald Anlauf a écrit : I'll stop here... Thanks. Go figure, I have no problem reproducing today. It's PR99798 (and there is even a patch for it). this patch has rotten a bit: the type of gfc_reluease_symbol has changed to bool, this can be fixed. Unfortunately, applying the patch does not remove the ICEs here... Oops, I take that back! There was an error on my side applying the patch; and now it does fix the ICEs after correcting that hickup Now the PR99798 patch is ready to be pushed, but I won't be available for a few days. We can finish our discussion on this topic afterwards. Hello, I'm coming back to this. I think either one of Steve's patch or your variant in the PR is a better fix for the ICE as a first step; they seem less fragile at least. Then we can look at a possible reordering of conflict checks as with the patch you originally submitted in this thread. like the attached variant? Yes. The churn in the testsuite is actually not that bad. OK for master, thanks for the patch. I wouldn't push for backporting, but if you feel like doing it, it seems safe enough (depending on my own backport for PR99798 of course). Regarding the conflict check reordering, I'm tempted to just drop it at this point, or do you think it remains worth it?
[gcc(refs/users/mikael/heads/pr93635-v2_Harald)] Fortran: improve attribute conflict checking [PR93635]
https://gcc.gnu.org/g:1ea6d9d7f541844106e9dbec0b3962cdd8695696
commit 1ea6d9d7f541844106e9dbec0b3962cdd8695696
Author: Harald Anlauf
Date: Thu May 23 21:13:00 2024 +0200
Fortran: improve attribute conflict checking [PR93635]
gcc/fortran/ChangeLog:
PR fortran/93635
* symbol.cc (conflict_std): Helper function for reporting attribute
conflicts depending on the Fortran standard version.
(conf_std): Helper macro for checking standard-dependent conflicts.
(gfc_check_conflict): Use it.
gcc/testsuite/ChangeLog:
PR fortran/93635
* gfortran.dg/c-interop/c1255-2.f90: Adjust pattern.
* gfortran.dg/pr87907.f90: Likewise.
* gfortran.dg/pr93635.f90: New test.
Co-authored-by: Steven G. Kargl
Diff:
---
gcc/fortran/symbol.cc | 63 +++--
gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90 | 4 +-
gcc/testsuite/gfortran.dg/pr87907.f90 | 8 ++--
gcc/testsuite/gfortran.dg/pr93635.f90 | 19
4 files changed, 54 insertions(+), 40 deletions(-)
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 0a1646def67..5db3c887127 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -407,18 +407,36 @@ gfc_check_function_type (gfc_namespace *ns)
/ Symbol attribute stuff */
+/* Older standards produced conflicts for some attributes that are allowed
+ in newer standards. Check for the conflict and issue an error depending
+ on the standard in play. */
+
+static bool
+conflict_std (int standard, const char *a1, const char *a2, const char *name,
+ locus *where)
+{
+ if (name == NULL)
+{
+ return gfc_notify_std (standard, "%s attribute conflicts "
+"with %s attribute at %L", a1, a2,
+where);
+}
+ else
+{
+ return gfc_notify_std (standard, "%s attribute conflicts "
+"with %s attribute in %qs at %L",
+a1, a2, name, where);
+}
+}
+
/* This is a generic conflict-checker. We do this to avoid having a
single conflict in two places. */
#define conf(a, b) if (attr->a && attr->b) { a1 = a; a2 = b; goto conflict; }
#define conf2(a) if (attr->a) { a2 = a; goto conflict; }
-#define conf_std(a, b, std) if (attr->a && attr->b)\
- {\
-a1 = a;\
-a2 = b;\
-standard = std;\
-goto conflict_std;\
- }
+#define conf_std(a, b, std) if (attr->a && attr->b \
+ && !conflict_std (std, a, b, name, where)) \
+ return false;
bool
gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
@@ -451,7 +469,6 @@ gfc_check_conflict (symbol_attribute *attr, const char
*name, locus *where)
"OACC DECLARE DEVICE_RESIDENT";
const char *a1, *a2;
- int standard;
if (attr->artificial)
return true;
@@ -460,20 +477,10 @@ gfc_check_conflict (symbol_attribute *attr, const char
*name, locus *where)
where = _current_locus;
if (attr->pointer && attr->intent != INTENT_UNKNOWN)
-{
- a1 = pointer;
- a2 = intent;
- standard = GFC_STD_F2003;
- goto conflict_std;
-}
+conf_std (pointer, intent, GFC_STD_F2003);
- if (attr->in_namelist && (attr->allocatable || attr->pointer))
-{
- a1 = in_namelist;
- a2 = attr->allocatable ? allocatable : pointer;
- standard = GFC_STD_F2003;
- goto conflict_std;
-}
+ conf_std (in_namelist, allocatable, GFC_STD_F2003);
+ conf_std (in_namelist, pointer, GFC_STD_F2003);
/* Check for attributes not allowed in a BLOCK DATA. */
if (gfc_current_state () == COMP_BLOCK_DATA)
@@ -922,20 +929,6 @@ conflict:
a1, a2, name, where);
return false;
-
-conflict_std:
- if (name == NULL)
-{
- return gfc_notify_std (standard, "%s attribute conflicts "
- "with %s attribute at %L", a1, a2,
- where);
-}
- else
-{
- return gfc_notify_std (standard, "%s attribute conflicts "
-"with %s attribute in %qs at %L",
- a1, a2, name, where);
-}
}
#undef conf
diff --git a/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
b/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
index 0e5505a0183..feed2e7645f 100644
--- a/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
+++ b/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
@@ -92,12 +92,12 @@ module m2
end function
! function result is a type that is not interoperable
-function g (x) bind (c) ! { dg-error "BIND\\(C\\)" }
[gcc] Created branch 'mikael/heads/pr93635-v2_Harald' in namespace 'refs/users'
The branch 'mikael/heads/pr93635-v2_Harald' was created in namespace 'refs/users' pointing to: 1ea6d9d7f54... Fortran: improve attribute conflict checking [PR93635]
[gcc] Deleted branch 'mikael/heads/pr99798_v32' in namespace 'refs/users'
The branch 'mikael/heads/pr99798_v32' in namespace 'refs/users' was deleted. It previously pointed to: e13178f7fbd... fortran: Assume there is no cyclic reference with submodule Diff: !!! WARNING: THE FOLLOWING COMMITS ARE NO LONGER ACCESSIBLE (LOST): --- e13178f... fortran: Assume there is no cyclic reference with submodule
Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]
Le 23/05/2024 à 09:49, Mikael Morin a écrit : Le 13/05/2024 à 09:25, Mikael Morin a écrit : Le 10/05/2024 à 21:56, Harald Anlauf a écrit : Am 10.05.24 um 21:48 schrieb Harald Anlauf: Hi Mikael, Am 10.05.24 um 11:45 schrieb Mikael Morin: Le 09/05/2024 à 22:30, Harald Anlauf a écrit : I'll stop here... Thanks. Go figure, I have no problem reproducing today. It's PR99798 (and there is even a patch for it). this patch has rotten a bit: the type of gfc_reluease_symbol has changed to bool, this can be fixed. Unfortunately, applying the patch does not remove the ICEs here... Oops, I take that back! There was an error on my side applying the patch; and now it does fix the ICEs after correcting that hickup Now the PR99798 patch is ready to be pushed, but I won't be available for a few days. We can finish our discussion on this topic afterwards. Hello, I'm coming back to this. I think either one of Steve's patch or your variant in the PR is a better fix for the ICE as a first step; they seem less fragile at least. Then we can look at a possible reordering of conflict checks as with the patch you originally submitted in this thread. Replying to myself... It's not a great plan if we want to avoid unnecessary churn in the testsuite.
Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]
Le 13/05/2024 à 09:25, Mikael Morin a écrit : Le 10/05/2024 à 21:56, Harald Anlauf a écrit : Am 10.05.24 um 21:48 schrieb Harald Anlauf: Hi Mikael, Am 10.05.24 um 11:45 schrieb Mikael Morin: Le 09/05/2024 à 22:30, Harald Anlauf a écrit : I'll stop here... Thanks. Go figure, I have no problem reproducing today. It's PR99798 (and there is even a patch for it). this patch has rotten a bit: the type of gfc_reluease_symbol has changed to bool, this can be fixed. Unfortunately, applying the patch does not remove the ICEs here... Oops, I take that back! There was an error on my side applying the patch; and now it does fix the ICEs after correcting that hickup Now the PR99798 patch is ready to be pushed, but I won't be available for a few days. We can finish our discussion on this topic afterwards. Hello, I'm coming back to this. I think either one of Steve's patch or your variant in the PR is a better fix for the ICE as a first step; they seem less fragile at least. Then we can look at a possible reordering of conflict checks as with the patch you originally submitted in this thread. Mikael
[gcc r15-698] fortran: Assume there is no cyclic reference with submodule symbols [PR99798]
https://gcc.gnu.org/g:38d1761c0c94b77a081ccc180d6e039f7a670468
commit r15-698-g38d1761c0c94b77a081ccc180d6e039f7a670468
Author: Mikael Morin
Date: Sun May 12 15:16:23 2024 +0200
fortran: Assume there is no cyclic reference with submodule symbols
[PR99798]
This prevents a premature release of memory with procedure symbols from
submodules, causing random compiler crashes.
The problem is a fragile detection of cyclic references, which can match
with procedures host-associated from a module in submodules, in cases where
it
shouldn't. The formal namespace is released, and with it the dummy
arguments
symbols of the procedure. But there is no cyclic reference, so the
procedure
symbol itself is not released and remains, with pointers to its dummy
arguments
now dangling.
The fix adds a condition to avoid the case, and refactors to a new predicate
by the way. Part of the original condition is also removed, for lack of a
reason to keep it.
PR fortran/99798
gcc/fortran/ChangeLog:
* symbol.cc (gfc_release_symbol): Move the condition guarding
the handling cyclic references...
(cyclic_reference_break_needed): ... here as a new predicate.
Remove superfluous parts. Add a condition preventing any premature
release with submodule symbols.
gcc/testsuite/ChangeLog:
* gfortran.dg/submodule_33.f08: New test.
Diff:
---
gcc/fortran/symbol.cc | 54 --
gcc/testsuite/gfortran.dg/submodule_33.f08 | 20 +++
2 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 8f7deac1d1ee..0a1646def678 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3179,6 +3179,57 @@ gfc_free_symbol (gfc_symbol *)
}
+/* Returns true if the symbol SYM has, through its FORMAL_NS field, a reference
+ to itself which should be eliminated for the symbol memory to be released
+ via normal reference counting.
+
+ The implementation is crucial as it controls the proper release of symbols,
+ especially (contained) procedure symbols, which can represent a lot of
memory
+ through the namespace of their body.
+
+ We try to avoid freeing too much memory (causing dangling pointers), to not
+ leak too much (wasting memory), and to avoid expensive walks of the symbol
+ tree (which would be the correct way to check for a cycle). */
+
+bool
+cyclic_reference_break_needed (gfc_symbol *sym)
+{
+ /* Normal symbols don't reference themselves. */
+ if (sym->formal_ns == nullptr)
+return false;
+
+ /* Procedures at the root of the file do have a self reference, but they
don't
+ have a reference in a parent namespace preventing the release of the
+ procedure namespace, so they can use the normal reference counting. */
+ if (sym->formal_ns == sym->ns)
+return false;
+
+ /* If sym->refs == 1, we can use normal reference counting. If sym->refs >
2,
+ the symbol won't be freed anyway, with or without cyclic reference. */
+ if (sym->refs != 2)
+return false;
+
+ /* Procedure symbols host-associated from a module in submodules are special,
+ because the namespace of the procedure block in the submodule is different
+ from the FORMAL_NS namespace generated by host-association. So there are
+ two different namespaces representing the same procedure namespace. As
+ FORMAL_NS comes from host-association, which only imports symbols visible
+ from the outside (dummy arguments basically), we can assume there is no
+ self reference through FORMAL_NS in that case. */
+ if (sym->attr.host_assoc && sym->attr.used_in_submodule)
+return false;
+
+ /* We can assume that contained procedures have cyclic references, because
+ the symbol of the procedure itself is accessible in the procedure body
+ namespace. So we assume that symbols with a formal namespace different
+ from the declaration namespace and two references, one of which is about
+ to be removed, are procedures with just the self reference left. At this
+ point, the symbol SYM matches that pattern, so we return true here to
+ permit the release of SYM. */
+ return true;
+}
+
+
/* Decrease the reference counter and free memory when we reach zero.
Returns true if the symbol has been freed, false otherwise. */
@@ -3188,8 +3239,7 @@ gfc_release_symbol (gfc_symbol *)
if (sym == NULL)
return false;
- if (sym->formal_ns != NULL && sym->refs == 2 && sym->formal_ns != sym->ns
- && (!sym->attr.entry || !sym->module))
+ if (cyclic_reference_break_needed (sym))
{
/* As formal_ns contains a reference to sym, delete formal_ns just
before the deletion of sym. */
diff --git a/gcc/testsuite/gfortr
Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]
Le 10/05/2024 à 21:56, Harald Anlauf a écrit : Am 10.05.24 um 21:48 schrieb Harald Anlauf: Hi Mikael, Am 10.05.24 um 11:45 schrieb Mikael Morin: Le 09/05/2024 à 22:30, Harald Anlauf a écrit : I'll stop here... Thanks. Go figure, I have no problem reproducing today. It's PR99798 (and there is even a patch for it). this patch has rotten a bit: the type of gfc_reluease_symbol has changed to bool, this can be fixed. Unfortunately, applying the patch does not remove the ICEs here... Oops, I take that back! There was an error on my side applying the patch; and now it does fix the ICEs after correcting that hickup Now the PR99798 patch is ready to be pushed, but I won't be available for a few days. We can finish our discussion on this topic afterwards. We currently do not recover well from errors, and the prevention of corrupted namespaces is apparently a goal we aim at. Yes, and we are not there yet. But at least there is a sensible error message before the crash. True. But having a sensible error before ICEing does not improve user experience either. Are you planning to work again on PR99798? Cheers, Harald Cheers, Harald The patch therefore does not require any testsuite update and should not give any other surprises, so it should be very safe. The plan is also to leave the PR open for the time being. Regtesting on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald
[PATCH] fortran: Assume there is no cyclic reference with submodule symbols [PR99798]
Hello,
Here is my final patch to fix the ICE of PR99798.
It's maybe overly verbose with comments, but the memory management is
hopefully clarified.
I tested this with a full fortran regression test on x86_64-linux and a
manual check with valgrind on the testcase.
OK for master?
-- 8< --
This prevents a premature release of memory with procedure symbols from
submodules, causing random compiler crashes.
The problem is a fragile detection of cyclic references, which can match
with procedures host-associated from a module in submodules, in cases where it
shouldn't. The formal namespace is released, and with it the dummy arguments
symbols of the procedure. But there is no cyclic reference, so the procedure
symbol itself is not released and remains, with pointers to its dummy arguments
now dangling.
The fix adds a condition to avoid the case, and refactors to a new predicate
by the way. Part of the original condition is also removed, for lack of a
reason to keep it.
PR fortran/99798
gcc/fortran/ChangeLog:
* symbol.cc (gfc_release_symbol): Move the condition guarding
the handling cyclic references...
(cyclic_reference_break_needed): ... here as a new predicate.
Remove superfluous parts. Add a condition preventing any premature
release with submodule symbols.
gcc/testsuite/ChangeLog:
* gfortran.dg/submodule_33.f08: New test.
---
gcc/fortran/symbol.cc | 54 +-
gcc/testsuite/gfortran.dg/submodule_33.f08 | 20
2 files changed, 72 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/submodule_33.f08
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 8f7deac1d1e..0a1646def67 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3179,6 +3179,57 @@ gfc_free_symbol (gfc_symbol *)
}
+/* Returns true if the symbol SYM has, through its FORMAL_NS field, a reference
+ to itself which should be eliminated for the symbol memory to be released
+ via normal reference counting.
+
+ The implementation is crucial as it controls the proper release of symbols,
+ especially (contained) procedure symbols, which can represent a lot of
memory
+ through the namespace of their body.
+
+ We try to avoid freeing too much memory (causing dangling pointers), to not
+ leak too much (wasting memory), and to avoid expensive walks of the symbol
+ tree (which would be the correct way to check for a cycle). */
+
+bool
+cyclic_reference_break_needed (gfc_symbol *sym)
+{
+ /* Normal symbols don't reference themselves. */
+ if (sym->formal_ns == nullptr)
+return false;
+
+ /* Procedures at the root of the file do have a self reference, but they
don't
+ have a reference in a parent namespace preventing the release of the
+ procedure namespace, so they can use the normal reference counting. */
+ if (sym->formal_ns == sym->ns)
+return false;
+
+ /* If sym->refs == 1, we can use normal reference counting. If sym->refs >
2,
+ the symbol won't be freed anyway, with or without cyclic reference. */
+ if (sym->refs != 2)
+return false;
+
+ /* Procedure symbols host-associated from a module in submodules are special,
+ because the namespace of the procedure block in the submodule is different
+ from the FORMAL_NS namespace generated by host-association. So there are
+ two different namespaces representing the same procedure namespace. As
+ FORMAL_NS comes from host-association, which only imports symbols visible
+ from the outside (dummy arguments basically), we can assume there is no
+ self reference through FORMAL_NS in that case. */
+ if (sym->attr.host_assoc && sym->attr.used_in_submodule)
+return false;
+
+ /* We can assume that contained procedures have cyclic references, because
+ the symbol of the procedure itself is accessible in the procedure body
+ namespace. So we assume that symbols with a formal namespace different
+ from the declaration namespace and two references, one of which is about
+ to be removed, are procedures with just the self reference left. At this
+ point, the symbol SYM matches that pattern, so we return true here to
+ permit the release of SYM. */
+ return true;
+}
+
+
/* Decrease the reference counter and free memory when we reach zero.
Returns true if the symbol has been freed, false otherwise. */
@@ -3188,8 +3239,7 @@ gfc_release_symbol (gfc_symbol *)
if (sym == NULL)
return false;
- if (sym->formal_ns != NULL && sym->refs == 2 && sym->formal_ns != sym->ns
- && (!sym->attr.entry || !sym->module))
+ if (cyclic_reference_break_needed (sym))
{
/* As formal_ns contains a reference to sym, delete formal_ns just
before the deletion of sym. */
diff --git a/gcc/testsuite/gfortran.dg/submodule_33.f08
b/gcc/testsuite/gfortran.dg/submodule_33.f08
new file mode 100644
index 000..b61d750def1
---
[gcc(refs/users/mikael/heads/pr99798_v32)] fortran: Assume there is no cyclic reference with submodule symbols [PR99798]
https://gcc.gnu.org/g:e13178f7fbd977b71602d39c401adc671fd30d16
commit e13178f7fbd977b71602d39c401adc671fd30d16
Author: Mikael Morin
Date: Fri May 10 11:14:48 2024 +0200
fortran: Assume there is no cyclic reference with submodule symbols
[PR99798]
This prevents a premature release of memory with procedure symbols from
submodules, causing random compiler crashes.
The problem is a fragile detection of cyclic references, which can match
with procedures host-associated from a module in submodules, in cases where
it
shouldn't. The formal namespace is released, and with it the dummy
arguments
symbols of the procedure. But there is no cyclic reference, so the
procedure
symbol itself is not released and remains, with pointers to its dummy
arguments
now dangling.
The fix adds a condition to avoid the case, and refactors to a new predicate
by the way. Part of the original condition is also removed, for lack of a
reason to keep it.
PR fortran/99798
gcc/fortran/ChangeLog:
* symbol.cc (gfc_release_symbol): Move the condition guarding
the cyclic references handling...
(cyclic_reference_break_needed): ... here as a new predicate. Add
a condition preventing any premature release with submodule symbols.
gcc/testsuite/ChangeLog:
* gfortran.dg/submodule_33.f08: New test.
Diff:
---
gcc/fortran/symbol.cc | 54 --
gcc/testsuite/gfortran.dg/submodule_33.f08 | 20 +++
2 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 8f7deac1d1ee..0a1646def678 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3179,6 +3179,57 @@ gfc_free_symbol (gfc_symbol *)
}
+/* Returns true if the symbol SYM has, through its FORMAL_NS field, a reference
+ to itself which should be eliminated for the symbol memory to be released
+ via normal reference counting.
+
+ The implementation is crucial as it controls the proper release of symbols,
+ especially (contained) procedure symbols, which can represent a lot of
memory
+ through the namespace of their body.
+
+ We try to avoid freeing too much memory (causing dangling pointers), to not
+ leak too much (wasting memory), and to avoid expensive walks of the symbol
+ tree (which would be the correct way to check for a cycle). */
+
+bool
+cyclic_reference_break_needed (gfc_symbol *sym)
+{
+ /* Normal symbols don't reference themselves. */
+ if (sym->formal_ns == nullptr)
+return false;
+
+ /* Procedures at the root of the file do have a self reference, but they
don't
+ have a reference in a parent namespace preventing the release of the
+ procedure namespace, so they can use the normal reference counting. */
+ if (sym->formal_ns == sym->ns)
+return false;
+
+ /* If sym->refs == 1, we can use normal reference counting. If sym->refs >
2,
+ the symbol won't be freed anyway, with or without cyclic reference. */
+ if (sym->refs != 2)
+return false;
+
+ /* Procedure symbols host-associated from a module in submodules are special,
+ because the namespace of the procedure block in the submodule is different
+ from the FORMAL_NS namespace generated by host-association. So there are
+ two different namespaces representing the same procedure namespace. As
+ FORMAL_NS comes from host-association, which only imports symbols visible
+ from the outside (dummy arguments basically), we can assume there is no
+ self reference through FORMAL_NS in that case. */
+ if (sym->attr.host_assoc && sym->attr.used_in_submodule)
+return false;
+
+ /* We can assume that contained procedures have cyclic references, because
+ the symbol of the procedure itself is accessible in the procedure body
+ namespace. So we assume that symbols with a formal namespace different
+ from the declaration namespace and two references, one of which is about
+ to be removed, are procedures with just the self reference left. At this
+ point, the symbol SYM matches that pattern, so we return true here to
+ permit the release of SYM. */
+ return true;
+}
+
+
/* Decrease the reference counter and free memory when we reach zero.
Returns true if the symbol has been freed, false otherwise. */
@@ -3188,8 +3239,7 @@ gfc_release_symbol (gfc_symbol *)
if (sym == NULL)
return false;
- if (sym->formal_ns != NULL && sym->refs == 2 && sym->formal_ns != sym->ns
- && (!sym->attr.entry || !sym->module))
+ if (cyclic_reference_break_needed (sym))
{
/* As formal_ns contains a reference to sym, delete formal_ns just
before the deletion of sym. */
diff --git a/gcc/testsuite/gfortran.dg/submodule_33.f08
b/gcc/testsuite/gfort
[gcc] Created branch 'mikael/heads/pr99798_v32' in namespace 'refs/users'
The branch 'mikael/heads/pr99798_v32' was created in namespace 'refs/users' pointing to: e13178f7fbd9... fortran: Assume there is no cyclic reference with submodule
[gcc(refs/users/mikael/heads/pr99798_v66)] fortran: Fix leaked symbol
https://gcc.gnu.org/g:4b2e3dff5615a16532f42459fc5d0400c6d61f05 commit 4b2e3dff5615a16532f42459fc5d0400c6d61f05 Author: Mikael Morin Date: Fri May 10 11:17:41 2024 +0200 fortran: Fix leaked symbol For a symbol we create, this adds a reference to a it in a namespace, so that its memory is not leaked. A hidden name is used to avoid polluting the namespace. gcc/fortran/ChangeLog: * decl.cc (get_proc_name): Reference the interface symbol in the namespace under a hidden name. Diff: --- gcc/fortran/decl.cc | 4 1 file changed, 4 insertions(+) diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index b8308aeee550..ce0fb6bae594 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -1335,6 +1335,10 @@ get_proc_name (const char *name, gfc_symbol **result, bool module_fcn_entry) /* Create a partially populated interface symbol to carry the characteristics of the procedure and the result. */ sym->tlink = gfc_new_symbol (name, sym->ns); + gfc_symtree *s = gfc_get_unique_symtree (sym->ns); + s->n.sym = sym->tlink; + s->n.sym->refs++; + gfc_add_type (sym->tlink, &(sym->ts), _current_locus); gfc_copy_attr (>tlink->attr, >attr, NULL); if (sym->attr.dimension)
[gcc(refs/users/mikael/heads/pr99798_v66)] fortran: Assume there is no cyclic reference with submodule symbols [PR99798]
https://gcc.gnu.org/g:934742d5d2706d3dc3df1dad7cd7ad1cfd6d6370
commit 934742d5d2706d3dc3df1dad7cd7ad1cfd6d6370
Author: Mikael Morin
Date: Fri May 10 11:14:48 2024 +0200
fortran: Assume there is no cyclic reference with submodule symbols
[PR99798]
This prevents a premature release of memory with procedure symbols from
submodules, causing random compiler crashes.
The problem is a fragile detection of cyclic references, which can match
with procedures host-associated from a module in submodules, in cases where
it
shouldn't. The formal namespace is released, and with it the dummy
arguments
symbols of the procedure. But there is no cyclic reference, so the
procedure
symbol itself is not released and remains, with pointers to its dummy
arguments
now dangling.
The fix adds a condition to avoid the case, and refactors to a new
predicate by
the way.
PR fortran/99798
gcc/fortran/ChangeLog:
* symbol.cc (gfc_release_symbol): Move the condition guarding
the cyclic references handling...
(cyclic_reference_break_needed): ... here as a new predicate. Add
a condition preventing any premature release with submodule symbols.
gcc/testsuite/ChangeLog:
* gfortran.dg/submodule_33.f08: New test.
Diff:
---
gcc/fortran/symbol.cc | 54 --
gcc/testsuite/gfortran.dg/submodule_33.f08 | 20 +++
2 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 8f7deac1d1ee..0a1646def678 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3179,6 +3179,57 @@ gfc_free_symbol (gfc_symbol *)
}
+/* Returns true if the symbol SYM has, through its FORMAL_NS field, a reference
+ to itself which should be eliminated for the symbol memory to be released
+ via normal reference counting.
+
+ The implementation is crucial as it controls the proper release of symbols,
+ especially (contained) procedure symbols, which can represent a lot of
memory
+ through the namespace of their body.
+
+ We try to avoid freeing too much memory (causing dangling pointers), to not
+ leak too much (wasting memory), and to avoid expensive walks of the symbol
+ tree (which would be the correct way to check for a cycle). */
+
+bool
+cyclic_reference_break_needed (gfc_symbol *sym)
+{
+ /* Normal symbols don't reference themselves. */
+ if (sym->formal_ns == nullptr)
+return false;
+
+ /* Procedures at the root of the file do have a self reference, but they
don't
+ have a reference in a parent namespace preventing the release of the
+ procedure namespace, so they can use the normal reference counting. */
+ if (sym->formal_ns == sym->ns)
+return false;
+
+ /* If sym->refs == 1, we can use normal reference counting. If sym->refs >
2,
+ the symbol won't be freed anyway, with or without cyclic reference. */
+ if (sym->refs != 2)
+return false;
+
+ /* Procedure symbols host-associated from a module in submodules are special,
+ because the namespace of the procedure block in the submodule is different
+ from the FORMAL_NS namespace generated by host-association. So there are
+ two different namespaces representing the same procedure namespace. As
+ FORMAL_NS comes from host-association, which only imports symbols visible
+ from the outside (dummy arguments basically), we can assume there is no
+ self reference through FORMAL_NS in that case. */
+ if (sym->attr.host_assoc && sym->attr.used_in_submodule)
+return false;
+
+ /* We can assume that contained procedures have cyclic references, because
+ the symbol of the procedure itself is accessible in the procedure body
+ namespace. So we assume that symbols with a formal namespace different
+ from the declaration namespace and two references, one of which is about
+ to be removed, are procedures with just the self reference left. At this
+ point, the symbol SYM matches that pattern, so we return true here to
+ permit the release of SYM. */
+ return true;
+}
+
+
/* Decrease the reference counter and free memory when we reach zero.
Returns true if the symbol has been freed, false otherwise. */
@@ -3188,8 +3239,7 @@ gfc_release_symbol (gfc_symbol *)
if (sym == NULL)
return false;
- if (sym->formal_ns != NULL && sym->refs == 2 && sym->formal_ns != sym->ns
- && (!sym->attr.entry || !sym->module))
+ if (cyclic_reference_break_needed (sym))
{
/* As formal_ns contains a reference to sym, delete formal_ns just
before the deletion of sym. */
diff --git a/gcc/testsuite/gfortran.dg/submodule_33.f08
b/gcc/testsuite/gfortran.dg/submodule_33.f08
new file mode 100644
index ..b61d750def16
--- /dev/n
[gcc] Created branch 'mikael/heads/pr99798_v66' in namespace 'refs/users'
The branch 'mikael/heads/pr99798_v66' was created in namespace 'refs/users' pointing to: 4b2e3dff5615... fortran: Fix leaked symbol
Re: [PATCH] Fortran: fix dependency checks for inquiry refs [PR115039]
Le 10/05/2024 à 21:24, Harald Anlauf a écrit : Dear all, the attached simple and obvious patch fixes a bogus recursion error with inquiry refs used statement functions. The commit message says all there is to say... Regtested on x86_64-pc-linux-gnu. I intend to commit to mainline within the next 24 hours unless someone screams... Will also backport to 14-branch after a decent delay. Thanks, Harald I agree that the return value change makes sense, but the function is generic, broader than just dependency checking, so the comment feels a bit out of place.
Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]
Le 09/05/2024 à 22:30, Harald Anlauf a écrit :
Hi Mikael,
Am 09.05.24 um 21:51 schrieb Mikael Morin:
Hello,
Le 06/05/2024 à 21:33, Harald Anlauf a écrit :
Dear all,
I've been contemplating whether to submit the attached patch.
It addresses an ICE-on-invalid as reported in the PR, and also
fixes an accepts-invalid (see testcase), plus maybe some more,
related due to incomplete checking of symbol attribute conflicts.
The fix does not fully address the general issue, which is
analyzed by Steve: some of the checks do depend on the selected
Fortran standard, and under circumstances such as in the testcase
the checking of other, standard-version-independent conflicts
simply does not occur.
Steve's solution would fix that, but unfortunately leads to issues
with error recovery in notoriously fragile parts of the FE: e.g.
testcase pr87907.f90 needs adjusting, and minor variations
of it will lead to various other horrendous ICEs that remind
of existing PRs where parsing or resolution goes sideways.
I therefore propose a much simpler approach: move - if possible -
selected of the standard-version-dependent checks after the
version-independent ones. I think this could help in getting more
consistent error reporting and recovery. However, I did *not*
move those checks that are critical when processing interfaces.
(-> pr87907.f90 / (sub)modules)
Your patch looks clean, but I'm concerned that the order of the checks
should be the important ones first, regardless of their standard
version. I'm trying to look at the ICE caused by your other tentative
patch at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93635#c6 but I
can't reproduce the problem. Do you by any chance have around some of
the variations causing "horrendous" ICEs?
Oh, that's easy. Just move the block
conf_std (allocatable, dummy, GFC_STD_F2003);
conf_std (allocatable, function, GFC_STD_F2003);
conf_std (allocatable, result, GFC_STD_F2003);
towards the end of the gfc_check_conflict before the return true.
While the error messages for the original gfortran.dg/pr87907.f90
look harmless, commenting out the main program p I get:
pr87907.f90:15:18:
15 | subroutine g(x) ! { dg-error "mismatch in argument" }
| 1
Error: FUNCTION attribute conflicts with SUBROUTINE attribute in 'g' at (1)
f951: internal compiler error: Segmentation fault
0x13b8ec2 crash_signal
../../gcc-trunk/gcc/toplev.cc:319
0xba530e free_sym_tree
../../gcc-trunk/gcc/fortran/symbol.cc:4026
0xba5319 free_sym_tree
../../gcc-trunk/gcc/fortran/symbol.cc:4026
0xba5319 free_sym_tree
../../gcc-trunk/gcc/fortran/symbol.cc:4026
0xba5319 free_sym_tree
../../gcc-trunk/gcc/fortran/symbol.cc:4026
0xba5609 gfc_free_namespace(gfc_namespace*&)
../../gcc-trunk/gcc/fortran/symbol.cc:4168
0xba39c1 gfc_free_symbol(gfc_symbol*&)
../../gcc-trunk/gcc/fortran/symbol.cc:3173
0xba3b89 gfc_release_symbol(gfc_symbol*&)
../../gcc-trunk/gcc/fortran/symbol.cc:3216
0xba5339 free_sym_tree
../../gcc-trunk/gcc/fortran/symbol.cc:4029
0xba5609 gfc_free_namespace(gfc_namespace*&)
../../gcc-trunk/gcc/fortran/symbol.cc:4168
0xba58ef gfc_symbol_done_2()
../../gcc-trunk/gcc/fortran/symbol.cc:4236
0xb12ec8 gfc_done_2()
../../gcc-trunk/gcc/fortran/misc.cc:387
0xb4ac7f clean_up_modules
../../gcc-trunk/gcc/fortran/parse.cc:7057
0xb4af02 translate_all_program_units
../../gcc-trunk/gcc/fortran/parse.cc:7122
0xb4b735 gfc_parse_file()
../../gcc-trunk/gcc/fortran/parse.cc:7413
0xbb626f gfc_be_parse_file
../../gcc-trunk/gcc/fortran/f95-lang.cc:241
Restoring the main program but simply adding "end subroutine g"
where it is naively expected gives:
pr87907.f90:15:18:
15 | subroutine g(x) ! { dg-error "mismatch in argument" }
| 1
Error: FUNCTION attribute conflicts with SUBROUTINE attribute in 'g' at (1)
pr87907.f90:16:9:
16 | end subroutine g
| 1
Error: Expecting END SUBMODULE statement at (1)
pr87907.f90:20:7:
20 | use m ! { dg-error "has a type" }
| 1
21 | integer :: x = 3
22 | call g(x) ! { dg-error "which is not consistent
with" }
|
2
Error: 'g' at (1) has a type, which is not consistent with the CALL at (2)
f951: internal compiler error: in gfc_free_namespace, at
fortran/symbol.cc:4164
0xba55e1 gfc_free_namespace(gfc_namespace*&)
../../gcc-trunk/gcc/fortran/symbol.cc:4164
0xba39c1 gfc_free_symbol(gfc_symbol*&)
../../gcc-trunk/gcc/fortran/symbol.cc:3173
0xba3b89 gfc_release_symbol(gfc_symbol*&)
../../gcc-trunk/gcc/fortran/symbol.cc:3216
0xba5339 free_sym_tree
../../gcc-trunk/gcc/fortran/symbol.cc:4029
0xba5609 gfc_free_namespace(gfc_namespace*&)
../../gcc-tru
Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]
Hello, Le 06/05/2024 à 21:33, Harald Anlauf a écrit : Dear all, I've been contemplating whether to submit the attached patch. It addresses an ICE-on-invalid as reported in the PR, and also fixes an accepts-invalid (see testcase), plus maybe some more, related due to incomplete checking of symbol attribute conflicts. The fix does not fully address the general issue, which is analyzed by Steve: some of the checks do depend on the selected Fortran standard, and under circumstances such as in the testcase the checking of other, standard-version-independent conflicts simply does not occur. Steve's solution would fix that, but unfortunately leads to issues with error recovery in notoriously fragile parts of the FE: e.g. testcase pr87907.f90 needs adjusting, and minor variations of it will lead to various other horrendous ICEs that remind of existing PRs where parsing or resolution goes sideways. I therefore propose a much simpler approach: move - if possible - selected of the standard-version-dependent checks after the version-independent ones. I think this could help in getting more consistent error reporting and recovery. However, I did *not* move those checks that are critical when processing interfaces. (-> pr87907.f90 / (sub)modules) Your patch looks clean, but I'm concerned that the order of the checks should be the important ones first, regardless of their standard version. I'm trying to look at the ICE caused by your other tentative patch at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93635#c6 but I can't reproduce the problem. Do you by any chance have around some of the variations causing "horrendous" ICEs? The patch therefore does not require any testsuite update and should not give any other surprises, so it should be very safe. The plan is also to leave the PR open for the time being. Regtesting on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald
Re: [PATCHv2] Value range: Add range op for __builtin_isfinite
Le 09/05/2024 à 10:47, HAO CHEN GUI a écrit : Hi Mikael, Thanks for your comments. 在 2024/5/9 16:03, Mikael Morin 写道: I think the canonical API behaviour sets R to varying and returns true instead of just returning false if nothing is known about the range. I'm not sure whether it makes any difference; Aldy can probably tell. But if the type is bool, varying is [0,1] which is better than unknown range. Should the varying be set by caller when fold_range returns false? Just like following codes in value-query.cc. if (!op.fold_range (r, type, r0, r1)) r.set_varying (type); Err, well, that's a good question. Looking more closely, there are at least two operators in range-op.cc that follow this pattern of returning false and letting the caller handle. And in gimple-range-op.cc many more of them, so your patch is canonical enough in any case, sorry for the false alarm. Thanks Gui Haochen
Re: [PATCHv2] Value range: Add range op for __builtin_isfinite
Hello,
Le 07/05/2024 à 04:37, HAO CHEN GUI a écrit :
Hi,
The former patch adds isfinite optab for __builtin_isfinite.
https://gcc.gnu.org/pipermail/gcc-patches/2024-April/649339.html
Thus the builtin might not be folded at front end. The range op for
isfinite is needed for value range analysis. This patch adds them.
Compared to last version, this version fixes a typo.
Bootstrapped and tested on x86 and powerpc64-linux BE and LE with no
regressions. Is it OK for the trunk?
Thanks
Gui Haochen
ChangeLog
Value Range: Add range op for builtin isfinite
The former patch adds optab for builtin isfinite. Thus builtin isfinite might
not be folded at front end. So the range op for isfinite is needed for value
range analysis. This patch adds range op for builtin isfinite.
gcc/
* gimple-range-op.cc (class cfn_isfinite): New.
(op_cfn_finite): New variables.
(gimple_range_op_handler::maybe_builtin_call): Handle
CFN_BUILT_IN_ISFINITE.
gcc/testsuite/
* gcc/testsuite/gcc.dg/tree-ssa/range-isfinite.c: New test.
patch.diff
diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc
index 9de130b4022..99c511728d3 100644
--- a/gcc/gimple-range-op.cc
+++ b/gcc/gimple-range-op.cc
@@ -1192,6 +1192,56 @@ public:
}
} op_cfn_isinf;
+//Implement range operator for CFN_BUILT_IN_ISFINITE
+class cfn_isfinite : public range_operator
+{
+public:
+ using range_operator::fold_range;
+ using range_operator::op1_range;
+ virtual bool fold_range (irange , tree type, const frange ,
+ const irange &, relation_trio) const override
+ {
+if (op1.undefined_p ())
+ return false;
+
+if (op1.known_isfinite ())
+ {
+ r.set_nonzero (type);
+ return true;
+ }
+
+if (op1.known_isnan ()
+ || op1.known_isinf ())
+ {
+ r.set_zero (type);
+ return true;
+ }
+
+return false;
I think the canonical API behaviour sets R to varying and returns true
instead of just returning false if nothing is known about the range.
I'm not sure whether it makes any difference; Aldy can probably tell.
But if the type is bool, varying is [0,1] which is better than unknown
range.
+ }
+ virtual bool op1_range (frange , tree type, const irange ,
+ const frange &, relation_trio) const override
+ {
+if (lhs.zero_p ())
+ {
+ // The range is [-INF,-INF][+INF,+INF] NAN, but it can't be represented.
+ // Set range to varying
+ r.set_varying (type);
+ return true;
+ }
+
+if (!range_includes_zero_p ())
+ {
+ nan_state nan (false);
+ r.set (type, real_min_representable (type),
+ real_max_representable (type), nan);
+ return true;
+ }
+
+return false;
Same here.
+ }
+} op_cfn_isfinite;
+
// Implement range operator for CFN_BUILT_IN_
class cfn_parity : public range_operator
{
Re: [PATCH 2/4] fortran: Teach get_real_kind_from_node for Power 128 fp modes [PR112993]
Hello, Le 08/05/2024 à 07:27, Kewen.Lin a écrit : Hi, Previously effective target fortran_real_c_float128 never passes on Power regardless of the default 128 long double is ibmlongdouble or ieeelongdouble. It's due to that TF mode is always used for kind 16 real, which has precision 127, while the node float128_type_node for c_float128 has 128 type precision, get_real_kind_from_node can't find a matching as it only checks gfc_real_kinds[i].mode_precision and type precision. With changing TFmode/IFmode/KFmode to have the same mode precision 128, now fortran_real_c_float12 can pass with ieeelongdouble enabled by default and test cases guarded with it get tested accordingly. But with ibmlongdouble enabled by default, since TFmode has precision 128 which is the same as type precision 128 of float128_type_node, get_real_kind_from_node considers kind for TFmode matches float128_type_node, but it's wrong as at this time point TFmode is with ibm extended format. So this patch is to teach get_real_kind_from_node to check one more field which can be differentiable from the underlying real format, it can avoid the unexpected matching when there more than one modes have the same precision. Bootstrapped and regress-tested on: - powerpc64-linux-gnu P8/P9 (with ibm128 by default) - powerpc64le-linux-gnu P9/P10 (with ibm128 by default) - powerpc64le-linux-gnu P9 (with ieee128 by default) OK from the fortran point of view. Thanks. BR, Kewen
Re: [Patch, fortran] PR114859 - [14/15 Regression] Seeing new segmentation fault in same_type_as since r14-9752
Hello, Le 28/04/2024 à 23:37, Paul Richard Thomas a écrit : Hi All, Could this be looked at quickly? The timing of this regression is more than a little embarrassing on the eve of the 14.1 release. The testcase and the comment in gfc_trans_class_init_assign explain what this problem is all about and how the patch fixes it. OK for 15-branch and backporting to 14-branch (hopefully to the RC as well)? Paul Fortran: Fix regression caused by r14-9752 [PR114959] 2024-04-28 Paul Thomas mailto:pa...@gcc.gnu.org>> You can drop the mailto:… thing. ;-) gcc/fortran PR fortran/114959 * trans-expr.cc (gfc_trans_class_init_assign): Return NULL_TREE if the default initializer has all NULL fields. Guard this by a requirement that the code be EXEC_INIT_ASSIGN and that the object be an INTENT_IN dummy. In the patch, the code requirement is different from EXEC_ALLOCATE and the intent is INTENT_OUT, not INTENT_IN. * trans-stmt.cc (gfc_trans_allocate): Change the initializer code for allocate with mold to EXEC_ASSIGN to allow initializer with all NULL fields.. In the patch it's EXEC_ALLOCATE, not EXEC_ASSIGN. OK for master with the ChangeLog fixed. For 14, you need release manager approval, I think. Thanks for the quick fix. Mikael
[gcc r11-11305] fortran: Ignore use statements on error [PR107426]
https://gcc.gnu.org/g:3d05b9ac1c6ad950339f9487702c3165c189fe9e
commit r11-11305-g3d05b9ac1c6ad950339f9487702c3165c189fe9e
Author: Mikael Morin
Date: Thu Mar 21 17:27:54 2024 +0100
fortran: Ignore use statements on error [PR107426]
This fixes an access to freed memory on the testcase from the PR.
The problem comes from an invalid subroutine statement in an interface,
which is ignored and causes the following statements forming the procedure
body to be rejected. One of them use-associates the intrinsic ISO_C_BINDING
module, which imports new symbols in a namespace that is freed at the time
the statement is rejected. However, this creates dangling pointers as
ISO_C_BINDING is special and its import creates a reference to the imported
C_PTR symbol in the return type of the global intrinsic symbol for C_LOC
(see the function create_intrinsic_function).
This change saves and restores the list of use statements, so that rejected
use statements are removed before they have a chance to be applied to the
current namespace and create dangling pointers.
PR fortran/107426
gcc/fortran/ChangeLog:
* gfortran.h (gfc_save_module_list, gfc_restore_old_module_list):
New declarations.
* module.c (old_module_list_tail): New global variable.
(gfc_save_module_list, gfc_restore_old_module_list): New functions.
(gfc_use_modules): Set module_list and old_module_list_tail.
* parse.c (next_statement): Save module_list before doing any work.
(reject_statement): Restore module_list to its saved value.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr89943_3.f90: Update error pattern.
* gfortran.dg/pr89943_4.f90: Likewise.
* gfortran.dg/use_31.f90: New test.
(cherry picked from commit a44d7e8a52007c2d45217709ca02947c6600de87)
Diff:
---
gcc/fortran/gfortran.h | 2 ++
gcc/fortran/module.c| 31 +++
gcc/fortran/parse.c | 4
gcc/testsuite/gfortran.dg/pr89943_3.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr89943_4.f90 | 2 +-
gcc/testsuite/gfortran.dg/use_31.f90| 26 ++
6 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 0436c4f308f..e5a0bc2be60 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3578,6 +3578,8 @@ void gfc_module_done_2 (void);
void gfc_dump_module (const char *, int);
bool gfc_check_symbol_access (gfc_symbol *);
void gfc_free_use_stmts (gfc_use_list *);
+void gfc_save_module_list ();
+void gfc_restore_old_module_list ();
const char *gfc_dt_lower_string (const char *);
const char *gfc_dt_upper_string (const char *);
diff --git a/gcc/fortran/module.c b/gcc/fortran/module.c
index 089453caa03..42bd585434f 100644
--- a/gcc/fortran/module.c
+++ b/gcc/fortran/module.c
@@ -195,7 +195,12 @@ static const char *module_name;
/* The name of the .smod file that the submodule will write to. */
static const char *submodule_name;
+/* The list of use statements to apply to the current namespace
+ before parsing the non-use statements. */
static gfc_use_list *module_list;
+/* The end of the MODULE_LIST list above at the time the recognition
+ of the current statement started. */
+static gfc_use_list **old_module_list_tail;
/* If we're reading an intrinsic module, this is its ID. */
static intmod_id current_intmod;
@@ -7476,6 +7481,8 @@ gfc_use_modules (void)
gfc_use_module (module_list);
free (module_list);
}
+ module_list = NULL;
+ old_module_list_tail = _list;
gfc_rename_list = NULL;
}
@@ -7499,6 +7506,30 @@ gfc_free_use_stmts (gfc_use_list *use_stmts)
}
+/* Remember the end of the MODULE_LIST list, so that the list can be restored
+ to its previous state if the current statement is erroneous. */
+
+void
+gfc_save_module_list ()
+{
+ gfc_use_list **tail = _list;
+ while (*tail != NULL)
+tail = &(*tail)->next;
+ old_module_list_tail = tail;
+}
+
+
+/* Restore the MODULE_LIST list to its previous value and free the use
+ statements that are no longer part of the list. */
+
+void
+gfc_restore_old_module_list ()
+{
+ gfc_free_use_stmts (*old_module_list_tail);
+ *old_module_list_tail = NULL;
+}
+
+
void
gfc_module_init_2 (void)
{
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 6893557733b..65fd0827c01 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -1519,6 +1519,7 @@ next_statement (void)
locus old_locus;
gfc_enforce_clean_symbol_state ();
+ gfc_save_module_list ();
gfc_new_block = NULL;
@@ -2674,6 +2675,9 @@ reject_statement (void)
gfc_reject_data (gfc_current_ns);
+ /* Don't queue use-association of a module if we reject the use statement.
*/
+ gfc_restore_old_modu
[gcc r12-10305] fortran: Ignore use statements on error [PR107426]
https://gcc.gnu.org/g:38dd703d368c9e60159e6f19cfc8303ad639b557
commit r12-10305-g38dd703d368c9e60159e6f19cfc8303ad639b557
Author: Mikael Morin
Date: Thu Mar 21 17:27:54 2024 +0100
fortran: Ignore use statements on error [PR107426]
This fixes an access to freed memory on the testcase from the PR.
The problem comes from an invalid subroutine statement in an interface,
which is ignored and causes the following statements forming the procedure
body to be rejected. One of them use-associates the intrinsic ISO_C_BINDING
module, which imports new symbols in a namespace that is freed at the time
the statement is rejected. However, this creates dangling pointers as
ISO_C_BINDING is special and its import creates a reference to the imported
C_PTR symbol in the return type of the global intrinsic symbol for C_LOC
(see the function create_intrinsic_function).
This change saves and restores the list of use statements, so that rejected
use statements are removed before they have a chance to be applied to the
current namespace and create dangling pointers.
PR fortran/107426
gcc/fortran/ChangeLog:
* gfortran.h (gfc_save_module_list, gfc_restore_old_module_list):
New declarations.
* module.cc (old_module_list_tail): New global variable.
(gfc_save_module_list, gfc_restore_old_module_list): New functions.
(gfc_use_modules): Set module_list and old_module_list_tail.
* parse.cc (next_statement): Save module_list before doing any work.
(reject_statement): Restore module_list to its saved value.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr89943_3.f90: Update error pattern.
* gfortran.dg/pr89943_4.f90: Likewise.
* gfortran.dg/use_31.f90: New test.
(cherry picked from commit a44d7e8a52007c2d45217709ca02947c6600de87)
Diff:
---
gcc/fortran/gfortran.h | 2 ++
gcc/fortran/module.cc | 31 +++
gcc/fortran/parse.cc| 4
gcc/testsuite/gfortran.dg/pr89943_3.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr89943_4.f90 | 2 +-
gcc/testsuite/gfortran.dg/use_31.f90| 26 ++
6 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 7bf1d5a0452..98c0cd39503 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3800,6 +3800,8 @@ void gfc_module_done_2 (void);
void gfc_dump_module (const char *, int);
bool gfc_check_symbol_access (gfc_symbol *);
void gfc_free_use_stmts (gfc_use_list *);
+void gfc_save_module_list ();
+void gfc_restore_old_module_list ();
const char *gfc_dt_lower_string (const char *);
const char *gfc_dt_upper_string (const char *);
diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc
index 85aa153bd77..7b06acb3133 100644
--- a/gcc/fortran/module.cc
+++ b/gcc/fortran/module.cc
@@ -195,7 +195,12 @@ static const char *module_name;
/* The name of the .smod file that the submodule will write to. */
static const char *submodule_name;
+/* The list of use statements to apply to the current namespace
+ before parsing the non-use statements. */
static gfc_use_list *module_list;
+/* The end of the MODULE_LIST list above at the time the recognition
+ of the current statement started. */
+static gfc_use_list **old_module_list_tail;
/* If we're reading an intrinsic module, this is its ID. */
static intmod_id current_intmod;
@@ -7542,6 +7547,8 @@ gfc_use_modules (void)
gfc_use_module (module_list);
free (module_list);
}
+ module_list = NULL;
+ old_module_list_tail = _list;
gfc_rename_list = NULL;
}
@@ -7565,6 +7572,30 @@ gfc_free_use_stmts (gfc_use_list *use_stmts)
}
+/* Remember the end of the MODULE_LIST list, so that the list can be restored
+ to its previous state if the current statement is erroneous. */
+
+void
+gfc_save_module_list ()
+{
+ gfc_use_list **tail = _list;
+ while (*tail != NULL)
+tail = &(*tail)->next;
+ old_module_list_tail = tail;
+}
+
+
+/* Restore the MODULE_LIST list to its previous value and free the use
+ statements that are no longer part of the list. */
+
+void
+gfc_restore_old_module_list ()
+{
+ gfc_free_use_stmts (*old_module_list_tail);
+ *old_module_list_tail = NULL;
+}
+
+
void
gfc_module_init_2 (void)
{
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index 3e9c6514c80..2b3a1a91fd9 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -1600,6 +1600,7 @@ next_statement (void)
locus old_locus;
gfc_enforce_clean_symbol_state ();
+ gfc_save_module_list ();
gfc_new_block = NULL;
@@ -2875,6 +2876,9 @@ reject_statement (void)
gfc_reject_data (gfc_current_ns);
+ /* Don't queue use-association of a module if we reject the use statement.
*/
+ gfc_restore_old_modu
[gcc r13-8543] fortran: Ignore use statements on error [PR107426]
https://gcc.gnu.org/g:fc5c603da3c9b186308fb3afef7bcf3f3bf695e8
commit r13-8543-gfc5c603da3c9b186308fb3afef7bcf3f3bf695e8
Author: Mikael Morin
Date: Thu Mar 21 17:27:54 2024 +0100
fortran: Ignore use statements on error [PR107426]
This fixes an access to freed memory on the testcase from the PR.
The problem comes from an invalid subroutine statement in an interface,
which is ignored and causes the following statements forming the procedure
body to be rejected. One of them use-associates the intrinsic ISO_C_BINDING
module, which imports new symbols in a namespace that is freed at the time
the statement is rejected. However, this creates dangling pointers as
ISO_C_BINDING is special and its import creates a reference to the imported
C_PTR symbol in the return type of the global intrinsic symbol for C_LOC
(see the function create_intrinsic_function).
This change saves and restores the list of use statements, so that rejected
use statements are removed before they have a chance to be applied to the
current namespace and create dangling pointers.
PR fortran/107426
gcc/fortran/ChangeLog:
* gfortran.h (gfc_save_module_list, gfc_restore_old_module_list):
New declarations.
* module.cc (old_module_list_tail): New global variable.
(gfc_save_module_list, gfc_restore_old_module_list): New functions.
(gfc_use_modules): Set module_list and old_module_list_tail.
* parse.cc (next_statement): Save module_list before doing any work.
(reject_statement): Restore module_list to its saved value.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr89943_3.f90: Update error pattern.
* gfortran.dg/pr89943_4.f90: Likewise.
* gfortran.dg/use_31.f90: New test.
(cherry picked from commit a44d7e8a52007c2d45217709ca02947c6600de87)
Diff:
---
gcc/fortran/gfortran.h | 2 ++
gcc/fortran/module.cc | 31 +++
gcc/fortran/parse.cc| 4
gcc/testsuite/gfortran.dg/pr89943_3.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr89943_4.f90 | 2 +-
gcc/testsuite/gfortran.dg/use_31.f90| 26 ++
6 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index e6939056a7c..47414f73254 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3850,6 +3850,8 @@ void gfc_module_done_2 (void);
void gfc_dump_module (const char *, int);
bool gfc_check_symbol_access (gfc_symbol *);
void gfc_free_use_stmts (gfc_use_list *);
+void gfc_save_module_list ();
+void gfc_restore_old_module_list ();
const char *gfc_dt_lower_string (const char *);
const char *gfc_dt_upper_string (const char *);
diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc
index 601497e0998..21141e9422d 100644
--- a/gcc/fortran/module.cc
+++ b/gcc/fortran/module.cc
@@ -195,7 +195,12 @@ static const char *module_name;
/* The name of the .smod file that the submodule will write to. */
static const char *submodule_name;
+/* The list of use statements to apply to the current namespace
+ before parsing the non-use statements. */
static gfc_use_list *module_list;
+/* The end of the MODULE_LIST list above at the time the recognition
+ of the current statement started. */
+static gfc_use_list **old_module_list_tail;
/* If we're reading an intrinsic module, this is its ID. */
static intmod_id current_intmod;
@@ -7542,6 +7547,8 @@ gfc_use_modules (void)
gfc_use_module (module_list);
free (module_list);
}
+ module_list = NULL;
+ old_module_list_tail = _list;
gfc_rename_list = NULL;
}
@@ -7565,6 +7572,30 @@ gfc_free_use_stmts (gfc_use_list *use_stmts)
}
+/* Remember the end of the MODULE_LIST list, so that the list can be restored
+ to its previous state if the current statement is erroneous. */
+
+void
+gfc_save_module_list ()
+{
+ gfc_use_list **tail = _list;
+ while (*tail != NULL)
+tail = &(*tail)->next;
+ old_module_list_tail = tail;
+}
+
+
+/* Restore the MODULE_LIST list to its previous value and free the use
+ statements that are no longer part of the list. */
+
+void
+gfc_restore_old_module_list ()
+{
+ gfc_free_use_stmts (*old_module_list_tail);
+ *old_module_list_tail = NULL;
+}
+
+
void
gfc_module_init_2 (void)
{
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index 83bb8a6f58b..0d366894643 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -1609,6 +1609,7 @@ next_statement (void)
locus old_locus;
gfc_enforce_clean_symbol_state ();
+ gfc_save_module_list ();
gfc_new_block = NULL;
@@ -2901,6 +2902,9 @@ reject_statement (void)
gfc_reject_data (gfc_current_ns);
+ /* Don't queue use-association of a module if we reject the use statement.
*/
+ gfc_restore_old_modu
[gcc r14-9703] fortran: Fix specification expression check in submodules [PR114475]
https://gcc.gnu.org/g:7f233feafd657250340be3b3500d2697948ae3ed
commit r14-9703-g7f233feafd657250340be3b3500d2697948ae3ed
Author: Mikael Morin
Date: Wed Mar 27 16:30:42 2024 +0100
fortran: Fix specification expression check in submodules [PR114475]
The patch fixing PR111781 made the check of specification expressions more
restrictive, disallowing local variables in specification expressions of
dummy arguments. PR114475 showed an example where that change regressed,
disallowing in submodules expressions that had been allowed in the parent
module. In submodules indeed, the hierarchy of namespaces inherited from
the parent module is not reproduced so the host-association of symbols
can't be recognized by checking the nesting of namespaces.
This change fixes the problem by allowing in specification expressions
all the symbols in a submodule that are inherited from the parent module.
PR fortran/111781
PR fortran/114475
gcc/fortran/ChangeLog:
* expr.cc (check_restricted): In submodules, allow variables host-
associated from the parent module.
gcc/testsuite/ChangeLog:
* gfortran.dg/spec_expr_10.f90: New test.
Co-authored-by: Harald Anlauf
Diff:
---
gcc/fortran/expr.cc| 1 +
gcc/testsuite/gfortran.dg/spec_expr_10.f90 | 46 ++
2 files changed, 47 insertions(+)
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 9a042cd7040..09d1ebd95d2 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -3517,6 +3517,7 @@ check_restricted (gfc_expr *e)
if (e->error
|| sym->attr.in_common
|| sym->attr.use_assoc
+ || sym->attr.used_in_submodule
|| sym->attr.dummy
|| sym->attr.implied_index
|| sym->attr.flavor == FL_PARAMETER
diff --git a/gcc/testsuite/gfortran.dg/spec_expr_10.f90
b/gcc/testsuite/gfortran.dg/spec_expr_10.f90
new file mode 100644
index 000..287b5a8d6cc
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/spec_expr_10.f90
@@ -0,0 +1,46 @@
+! { dg-do compile }
+!
+! PR fortran/114475
+! The array specification of PP in OL_EVAL used to be rejected in the submodule
+! because the compiler was not able to see the host-association of N_EXTERNAL
+! there.
+!
+! Contributed by Jürgen Reuter .
+
+module t1
+ use, intrinsic :: iso_c_binding
+ implicit none
+ private
+ public :: t1_t
+ integer :: N_EXTERNAL = 0
+
+ type :: t1_t
+ contains
+procedure :: set_n_external => t1_set_n_external
+ end type t1_t
+
+ abstract interface
+ subroutine ol_eval (id, pp, emitter) bind(C)
+ import
+ real(kind = c_double), intent(in) :: pp(5 * N_EXTERNAL)
+ end subroutine ol_eval
+ end interface
+ interface
+module subroutine t1_set_n_external (object, n)
+ class(t1_t), intent(inout) :: object
+ integer, intent(in) :: n
+end subroutine t1_set_n_external
+ end interface
+
+end module t1
+
+submodule (t1) t1_s
+ implicit none
+contains
+ module subroutine t1_set_n_external (object, n)
+class(t1_t), intent(inout) :: object
+integer, intent(in) :: n
+N_EXTERNAL = n
+ end subroutine t1_set_n_external
+
+end submodule t1_s
[PATCH] fortran: Fix specification expression check in submodules [PR114475]
Hell(o),
it didn't take long for my recent patch for PR111781 to show a regression.
The fix proposed here is actually the one Harald posted in the PR.
I can't imagine a case where it wouldn't do the right thing.
Regression tested on x86_64-pc-linux-gnu.
OK for master?
-- >8 --
The patch fixing PR111781 made the check of specification expressions more
restrictive, disallowing local variables in specification expressions of
dummy arguments. PR114475 showed an example where that change regressed,
disallowing in submodules expressions that had been allowed in the parent
module. In submodules indeed, the hierarchy of namespaces inherited from
the parent module is not reproduced so the host-association of symbols
can't be recognized by checking the nesting of namespaces.
This change fixes the problem by allowing in specification expressions
all the symbols in a submodule that are inherited from the parent module.
PR fortran/111781
PR fortran/114475
gcc/fortran/ChangeLog:
* expr.cc (check_restricted): In submodules, allow variables host-
associated from the parent module.
gcc/testsuite/ChangeLog:
* gfortran.dg/spec_expr_10.f90: New test.
Co-authored-by: Harald Anlauf
---
gcc/fortran/expr.cc| 1 +
gcc/testsuite/gfortran.dg/spec_expr_10.f90 | 46 ++
2 files changed, 47 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_10.f90
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 9a042cd7040..09d1ebd95d2 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -3517,6 +3517,7 @@ check_restricted (gfc_expr *e)
if (e->error
|| sym->attr.in_common
|| sym->attr.use_assoc
+ || sym->attr.used_in_submodule
|| sym->attr.dummy
|| sym->attr.implied_index
|| sym->attr.flavor == FL_PARAMETER
diff --git a/gcc/testsuite/gfortran.dg/spec_expr_10.f90
b/gcc/testsuite/gfortran.dg/spec_expr_10.f90
new file mode 100644
index 000..287b5a8d6cc
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/spec_expr_10.f90
@@ -0,0 +1,46 @@
+! { dg-do compile }
+!
+! PR fortran/114475
+! The array specification of PP in OL_EVAL used to be rejected in the submodule
+! because the compiler was not able to see the host-association of N_EXTERNAL
+! there.
+!
+! Contributed by Jürgen Reuter .
+
+module t1
+ use, intrinsic :: iso_c_binding
+ implicit none
+ private
+ public :: t1_t
+ integer :: N_EXTERNAL = 0
+
+ type :: t1_t
+ contains
+procedure :: set_n_external => t1_set_n_external
+ end type t1_t
+
+ abstract interface
+ subroutine ol_eval (id, pp, emitter) bind(C)
+ import
+ real(kind = c_double), intent(in) :: pp(5 * N_EXTERNAL)
+ end subroutine ol_eval
+ end interface
+ interface
+module subroutine t1_set_n_external (object, n)
+ class(t1_t), intent(inout) :: object
+ integer, intent(in) :: n
+end subroutine t1_set_n_external
+ end interface
+
+end module t1
+
+submodule (t1) t1_s
+ implicit none
+contains
+ module subroutine t1_set_n_external (object, n)
+class(t1_t), intent(inout) :: object
+integer, intent(in) :: n
+N_EXTERNAL = n
+ end subroutine t1_set_n_external
+
+end submodule t1_s
--
2.43.0
[gcc r14-9619] fortran: Ignore use statements on error [PR107426]
https://gcc.gnu.org/g:a44d7e8a52007c2d45217709ca02947c6600de87
commit r14-9619-ga44d7e8a52007c2d45217709ca02947c6600de87
Author: Mikael Morin
Date: Thu Mar 21 17:27:54 2024 +0100
fortran: Ignore use statements on error [PR107426]
This fixes an access to freed memory on the testcase from the PR.
The problem comes from an invalid subroutine statement in an interface,
which is ignored and causes the following statements forming the procedure
body to be rejected. One of them use-associates the intrinsic ISO_C_BINDING
module, which imports new symbols in a namespace that is freed at the time
the statement is rejected. However, this creates dangling pointers as
ISO_C_BINDING is special and its import creates a reference to the imported
C_PTR symbol in the return type of the global intrinsic symbol for C_LOC
(see the function create_intrinsic_function).
This change saves and restores the list of use statements, so that rejected
use statements are removed before they have a chance to be applied to the
current namespace and create dangling pointers.
PR fortran/107426
gcc/fortran/ChangeLog:
* gfortran.h (gfc_save_module_list, gfc_restore_old_module_list):
New declarations.
* module.cc (old_module_list_tail): New global variable.
(gfc_save_module_list, gfc_restore_old_module_list): New functions.
(gfc_use_modules): Set module_list and old_module_list_tail.
* parse.cc (next_statement): Save module_list before doing any work.
(reject_statement): Restore module_list to its saved value.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr89943_3.f90: Update error pattern.
* gfortran.dg/pr89943_4.f90: Likewise.
* gfortran.dg/use_31.f90: New test.
Diff:
---
gcc/fortran/gfortran.h | 2 ++
gcc/fortran/module.cc | 31 +++
gcc/fortran/parse.cc| 4
gcc/testsuite/gfortran.dg/pr89943_3.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr89943_4.f90 | 2 +-
gcc/testsuite/gfortran.dg/use_31.f90| 26 ++
6 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 26aa56b3358..58505446bac 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3928,6 +3928,8 @@ void gfc_module_done_2 (void);
void gfc_dump_module (const char *, int);
bool gfc_check_symbol_access (gfc_symbol *);
void gfc_free_use_stmts (gfc_use_list *);
+void gfc_save_module_list ();
+void gfc_restore_old_module_list ();
const char *gfc_dt_lower_string (const char *);
const char *gfc_dt_upper_string (const char *);
diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc
index d1de53cbdb4..c565b84d61b 100644
--- a/gcc/fortran/module.cc
+++ b/gcc/fortran/module.cc
@@ -195,7 +195,12 @@ static const char *module_name;
/* The name of the .smod file that the submodule will write to. */
static const char *submodule_name;
+/* The list of use statements to apply to the current namespace
+ before parsing the non-use statements. */
static gfc_use_list *module_list;
+/* The end of the MODULE_LIST list above at the time the recognition
+ of the current statement started. */
+static gfc_use_list **old_module_list_tail;
/* If we're reading an intrinsic module, this is its ID. */
static intmod_id current_intmod;
@@ -7561,6 +7566,8 @@ gfc_use_modules (void)
gfc_use_module (module_list);
free (module_list);
}
+ module_list = NULL;
+ old_module_list_tail = _list;
gfc_rename_list = NULL;
}
@@ -7584,6 +7591,30 @@ gfc_free_use_stmts (gfc_use_list *use_stmts)
}
+/* Remember the end of the MODULE_LIST list, so that the list can be restored
+ to its previous state if the current statement is erroneous. */
+
+void
+gfc_save_module_list ()
+{
+ gfc_use_list **tail = _list;
+ while (*tail != NULL)
+tail = &(*tail)->next;
+ old_module_list_tail = tail;
+}
+
+
+/* Restore the MODULE_LIST list to its previous value and free the use
+ statements that are no longer part of the list. */
+
+void
+gfc_restore_old_module_list ()
+{
+ gfc_free_use_stmts (*old_module_list_tail);
+ *old_module_list_tail = NULL;
+}
+
+
void
gfc_module_init_2 (void)
{
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index a2bf328f681..79c810c86ba 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -1800,6 +1800,7 @@ next_statement (void)
locus old_locus;
gfc_enforce_clean_symbol_state ();
+ gfc_save_module_list ();
gfc_new_block = NULL;
@@ -3104,6 +3105,9 @@ reject_statement (void)
gfc_reject_data (gfc_current_ns);
+ /* Don't queue use-association of a module if we reject the use statement.
*/
+ gfc_restore_old_module_list ();
+
gfc_new_block = NULL;
gfc_undo_symbols ();
gfc_clear_warning (
[gcc r14-9618] fortran: Fix specification expression error with dummy procedures [PR111781]
https://gcc.gnu.org/g:44c0398e65347def316700911a51ca8b4ec0a411
commit r14-9618-g44c0398e65347def316700911a51ca8b4ec0a411
Author: Mikael Morin
Date: Fri Mar 22 12:32:34 2024 +0100
fortran: Fix specification expression error with dummy procedures [PR111781]
This fixes a spurious invalid variable in specification expression error.
The error was caused on the testcase from the PR by two different bugs.
First, the call to is_parent_of_current_ns was unable to recognize
correct host association and returned false. Second, an ad-hoc
condition coming next was using a global variable previously improperly
restored to false (instead of restoring it to its initial value). The
latter happened on the testcase because one dummy argument was a procedure,
and checking that argument what causing a check of all its arguments with
the (improper) reset of the flag at the end, and that preceded the check of
the next argument.
For the first bug, the wrong result of is_parent_of_current_ns is fixed by
correcting the namespaces that function deals with, both the one passed
as argument and the current one tracked in the gfc_current_ns global. Two
new functions are introduced to select the right namespace.
Regarding the second bug, the problematic condition is removed, together
with the formal_arg_flag associated with it. Indeed, that condition was
(wrongly) allowing local variables to be used in array bounds of dummy
arguments.
PR fortran/111781
gcc/fortran/ChangeLog:
* symbol.cc (gfc_get_procedure_ns, gfc_get_spec_ns): New functions.
* gfortran.h (gfc_get_procedure_ns, gfc_get_spec ns): Declare them.
(gfc_is_formal_arg): Remove.
* expr.cc (check_restricted): Remove special case allowing local
variable in dummy argument bound expressions. Use gfc_get_spec_ns
to get the right namespace.
* resolve.cc (gfc_is_formal_arg, formal_arg_flag): Remove.
(gfc_resolve_formal_arglist): Set gfc_current_ns. Quit loop and
restore gfc_current_ns instead of early returning.
(resolve_symbol): Factor common array spec resolution code to...
(resolve_symbol_array_spec): ... this new function. Additionnally
set and restore gfc_current_ns.
gcc/testsuite/ChangeLog:
* gfortran.dg/spec_expr_8.f90: New test.
* gfortran.dg/spec_expr_9.f90: New test.
Diff:
---
gcc/fortran/expr.cc | 8 +---
gcc/fortran/gfortran.h| 4 +-
gcc/fortran/resolve.cc| 77 ++-
gcc/fortran/symbol.cc | 58 +++
gcc/testsuite/gfortran.dg/spec_expr_8.f90 | 24 ++
gcc/testsuite/gfortran.dg/spec_expr_9.f90 | 19
6 files changed, 140 insertions(+), 50 deletions(-)
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index e4b1e8307e3..9a042cd7040 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -3514,19 +3514,13 @@ check_restricted (gfc_expr *e)
if (!check_references (e->ref, _restricted))
break;
- /* gfc_is_formal_arg broadcasts that a formal argument list is being
-processed in resolve.cc(resolve_formal_arglist). This is done so
-that host associated dummy array indices are accepted (PR23446).
-This mechanism also does the same for the specification expressions
-of array-valued functions. */
if (e->error
|| sym->attr.in_common
|| sym->attr.use_assoc
|| sym->attr.dummy
|| sym->attr.implied_index
|| sym->attr.flavor == FL_PARAMETER
- || is_parent_of_current_ns (sym->ns)
- || (gfc_is_formal_arg () && (sym->ns == gfc_current_ns)))
+ || is_parent_of_current_ns (gfc_get_spec_ns (sym)))
{
t = true;
break;
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index c7039730fad..26aa56b3358 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3612,6 +3612,9 @@ bool gfc_is_associate_pointer (gfc_symbol*);
gfc_symbol * gfc_find_dt_in_generic (gfc_symbol *);
gfc_formal_arglist *gfc_sym_get_dummy_args (gfc_symbol *);
+gfc_namespace * gfc_get_procedure_ns (gfc_symbol *);
+gfc_namespace * gfc_get_spec_ns (gfc_symbol *);
+
/* intrinsic.cc -- true if working in an init-expr, false otherwise. */
extern bool gfc_init_expr_flag;
@@ -3821,7 +3824,6 @@ bool gfc_resolve_iterator (gfc_iterator *, bool, bool);
bool find_forall_index (gfc_expr *, gfc_symbol *, int);
bool gfc_resolve_index (gfc_expr *, int);
bool gfc_resolve_dim_arg (gfc_expr *);
-bool gfc_is_formal_arg (void);
bool gfc_resolve_substring (gfc_ref *, bool *);
void gfc_resolve_substring_charlen (gfc_expr *);
gfc_expr *gfc_expr_to_
[gcc r14-9617] testsuite: Declare fortran array bound variables
https://gcc.gnu.org/g:ebace32a26424884789ccf585a24ac6a5703a323
commit r14-9617-gebace32a26424884789ccf585a24ac6a5703a323
Author: Mikael Morin
Date: Fri Mar 22 12:32:17 2024 +0100
testsuite: Declare fortran array bound variables
This fixes invalid undeclared fortran array bound variables
in the testsuite.
gcc/testsuite/ChangeLog:
* gfortran.dg/graphite/pr107865.f90: Declare array bound variable(s)
as dummy argument(s).
* gfortran.dg/pr101267.f90: Likewise.
* gfortran.dg/pr112404.f90: Likewise.
* gfortran.dg/pr78061.f: Likewise.
* gfortran.dg/pr79315.f90: Likewise.
* gfortran.dg/vect/pr90681.f: Likewise.
* gfortran.dg/vect/pr97761.f90: Likewise.
* gfortran.dg/vect/pr99746.f90: Likewise.
Diff:
---
gcc/testsuite/gfortran.dg/graphite/pr107865.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr101267.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr112404.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr78061.f | 2 +-
gcc/testsuite/gfortran.dg/pr79315.f90 | 6 +-
gcc/testsuite/gfortran.dg/vect/pr90681.f| 2 +-
gcc/testsuite/gfortran.dg/vect/pr97761.f90 | 2 +-
gcc/testsuite/gfortran.dg/vect/pr99746.f90 | 2 +-
8 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
b/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
index 6bddb17a1be..323d8092ad2 100644
--- a/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
+++ b/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-O1 -floop-parallelize-all -ftree-parallelize-loops=2" }
- SUBROUTINE FNC (F)
+ SUBROUTINE FNC (F,N)
IMPLICIT REAL (A-H)
DIMENSION F(N)
diff --git a/gcc/testsuite/gfortran.dg/pr101267.f90
b/gcc/testsuite/gfortran.dg/pr101267.f90
index 12723cf9c22..99a6dcfa342 100644
--- a/gcc/testsuite/gfortran.dg/pr101267.f90
+++ b/gcc/testsuite/gfortran.dg/pr101267.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-Ofast" }
! { dg-additional-options "-march=znver2" { target x86_64-*-* i?86-*-* } }
- SUBROUTINE sfddagd( regime, znt,ite ,jte )
+ SUBROUTINE sfddagd( regime, znt,ite ,jte, ime, IN )
REAL, DIMENSION( ime, IN) :: regime, znt
REAL, DIMENSION( ite, jte) :: wndcor_u
LOGICAL wrf_dm_on_monitor
diff --git a/gcc/testsuite/gfortran.dg/pr112404.f90
b/gcc/testsuite/gfortran.dg/pr112404.f90
index 573fa28164a..4508bbc8738 100644
--- a/gcc/testsuite/gfortran.dg/pr112404.f90
+++ b/gcc/testsuite/gfortran.dg/pr112404.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-Ofast" }
! { dg-additional-options "-mavx2" { target avx2 } }
- SUBROUTINE sfddagd( regime, znt, ite, jte )
+ SUBROUTINE sfddagd( regime, znt, ite, jte, ime, IN )
REAL, DIMENSION( ime, IN) :: regime, znt
REAL, DIMENSION( ite, jte) :: wndcor_u
LOGICAL wrf_dm_on_monitor
diff --git a/gcc/testsuite/gfortran.dg/pr78061.f
b/gcc/testsuite/gfortran.dg/pr78061.f
index 7e4dd3de8b5..9061dea74da 100644
--- a/gcc/testsuite/gfortran.dg/pr78061.f
+++ b/gcc/testsuite/gfortran.dg/pr78061.f
@@ -1,6 +1,6 @@
! { dg-do compile }
! { dg-options "-O3 -fsplit-loops" }
- SUBROUTINE SSYMM(C)
+ SUBROUTINE SSYMM(C,LDC)
REAL C(LDC,*)
LOGICAL LSAME
LOGICAL UPPER
diff --git a/gcc/testsuite/gfortran.dg/pr79315.f90
b/gcc/testsuite/gfortran.dg/pr79315.f90
index 8cd89691ce9..b754a2b3274 100644
--- a/gcc/testsuite/gfortran.dg/pr79315.f90
+++ b/gcc/testsuite/gfortran.dg/pr79315.f90
@@ -10,7 +10,11 @@ SUBROUTINE wsm32D(t, &
its,&
ite, &
kts, &
- kte &
+ kte, &
+ ims, &
+ ime, &
+ kms, &
+ kme &
)
REAL, DIMENSION( its:ite , kts:kte ), &
INTENT(INOUT) :: &
diff --git a/gcc/testsuite/gfortran.dg/vect/pr90681.f
b/gcc/testsuite/gfortran.dg/vect/pr90681.f
index 03d3987b146..49f1d50ab8f 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr90681.f
+++ b/gcc/testsuite/gfortran.dg/vect/pr90681.f
@@ -1,6 +1,6 @@
C { dg-do compile }
C { dg-additional-options "-march=armv8.2-a+sve" { target { aarch64*-*-* } } }
- SUBROUTINE HMU (H1)
+ SUBROUTINE HMU (H1,NORBS)
COMMON DD(107)
DIMENSION H1(NORBS,*)
DO 70 J1 = IA,I1
diff --git a/gcc/testsuite/gfortran.dg/vect/pr97761.f90
b/gcc/testsuite/gfortran.dg/vect/pr97761.f90
index 250e2bf016e..401ef06e422 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr97761.f90
+++ b/gcc/testsuite/gfortran.dg/vect/pr97761.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-additional-options "-O1" }
-subroutine ni (ps)
+subroutine ni (ps, inout)
type vector
real x, y
end type
diff --git a/gcc/testsuite/gfortran.dg/vect/pr99
[PATCH] fortran: Ignore use statements on error [PR107426]
Hello,
here is a fix for an ICE caused by dangling pointers to ISO_C_BINDING's
C_PTR symbol in the global intrinsic symbol for C_LOC.
I tried to fix it by making the intrinsic symbol use its own copy of
C_PTR, but it regressed heavily.
Instead, I propose this which is based on a patch I attached to the PR
one year ago. It's sufficient to remove the access to freed memory.
However, an underlying problem remains that successive use-associations
of ISO_C_BINDING's symbols in different scopes cause the return type
of the C_LOC global intrinsic symbol to be set to the C_PTR from each
scope successively, with the last one "winning". Not very pretty.
Anyway, there are two changed messages in the testsuite as a side-effect
of the proposed change. I regard them as acceptable, albeit slightly worse.
No regression otherwise on x86_64-pc-linux-gnu.
Ok for 14 master?
Mikael
-- >8 --
This fixes an access to freed memory on the testcase from the PR.
The problem comes from an invalid subroutine statement in an interface,
which is ignored and causes the following statements forming the procedure
body to be rejected. One of them use-associates the intrinsic ISO_C_BINDING
module, which imports new symbols in a namespace that is freed at the time
the statement is rejected. However, this creates dangling pointers as
ISO_C_BINDING is special and its import creates a reference to the imported
C_PTR symbol in the return type of the global intrinsic symbol for C_LOC
(see the function create_intrinsic_function).
This change saves and restores the list of use statements, so that rejected
use statements are removed before they have a chance to be applied to the
current namespace and create dangling pointers.
PR fortran/107426
gcc/fortran/ChangeLog:
* gfortran.h (gfc_save_module_list, gfc_restore_old_module_list):
New declarations.
* module.cc (old_module_list_tail): New global variable.
(gfc_save_module_list, gfc_restore_old_module_list): New functions.
(gfc_use_modules): Set module_list and old_module_list_tail.
* parse.cc (next_statement): Save module_list before doing any work.
(reject_statement): Restore module_list to its saved value.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr89943_3.f90: Update error pattern.
* gfortran.dg/pr89943_4.f90: Likewise.
* gfortran.dg/use_31.f90: New test.
---
gcc/fortran/gfortran.h | 2 ++
gcc/fortran/module.cc | 31 +
gcc/fortran/parse.cc| 4
gcc/testsuite/gfortran.dg/pr89943_3.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr89943_4.f90 | 2 +-
gcc/testsuite/gfortran.dg/use_31.f90| 25
6 files changed, 64 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/use_31.f90
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index c7039730fad..fec7b53ff1a 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3926,6 +3926,8 @@ void gfc_module_done_2 (void);
void gfc_dump_module (const char *, int);
bool gfc_check_symbol_access (gfc_symbol *);
void gfc_free_use_stmts (gfc_use_list *);
+void gfc_save_module_list ();
+void gfc_restore_old_module_list ();
const char *gfc_dt_lower_string (const char *);
const char *gfc_dt_upper_string (const char *);
diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc
index d1de53cbdb4..c565b84d61b 100644
--- a/gcc/fortran/module.cc
+++ b/gcc/fortran/module.cc
@@ -195,7 +195,12 @@ static const char *module_name;
/* The name of the .smod file that the submodule will write to. */
static const char *submodule_name;
+/* The list of use statements to apply to the current namespace
+ before parsing the non-use statements. */
static gfc_use_list *module_list;
+/* The end of the MODULE_LIST list above at the time the recognition
+ of the current statement started. */
+static gfc_use_list **old_module_list_tail;
/* If we're reading an intrinsic module, this is its ID. */
static intmod_id current_intmod;
@@ -7561,6 +7566,8 @@ gfc_use_modules (void)
gfc_use_module (module_list);
free (module_list);
}
+ module_list = NULL;
+ old_module_list_tail = _list;
gfc_rename_list = NULL;
}
@@ -7584,6 +7591,30 @@ gfc_free_use_stmts (gfc_use_list *use_stmts)
}
+/* Remember the end of the MODULE_LIST list, so that the list can be restored
+ to its previous state if the current statement is erroneous. */
+
+void
+gfc_save_module_list ()
+{
+ gfc_use_list **tail = _list;
+ while (*tail != NULL)
+tail = &(*tail)->next;
+ old_module_list_tail = tail;
+}
+
+
+/* Restore the MODULE_LIST list to its previous value and free the use
+ statements that are no longer part of the list. */
+
+void
+gfc_restore_old_module_list ()
+{
+ gfc_free_use_stmts (*old_module_list_tail);
+ *old_module_list_tail = NULL;
+}
+
+
void
gfc_module_init_2 (void)
{
diff --git a/gcc/fortran/parse.cc
Re: [PATCH, v3] Fortran: improve array component description in runtime error message [PR30802]
Le 20/03/2024 à 21:24, Harald Anlauf a écrit : Hi Mikael, all, here's now the third version of the patch that implements the following scheme: On 3/15/24 20:29, Mikael Morin wrote: Le 15/03/2024 à 18:26, Harald Anlauf a écrit : OK, that sounds interesting. To clarify the options: - for ordinary array x it would stay 'x' - when z is a DT scalar, and z%x is the array in question, use 'z%x' (here z...%x would look strange to me) Yes, the ellipsis would look strange to me as well. - when z is a DT array, and x some component further down, 'z...%x' This case also applies when z is a DT scalar and x is more than one level deep. I would rather not make the error message text vary too much to avoid to run into issues with translation. Would it be fine with you to have ... dimension 1 of array 'z...%x' above array bound ... only? OK, let's drop "component". Anything else? No, I think you covered everything. I've created a new helper function that centralizes the generation of the abbreviated name of the array (component) and use it to simplify related code in multiple places. If we change our mind how a bounds violation error message should look like, it will be easier to adjust in the future. Is this OK for 14-mainline? Yes, thanks. Thanks, Harald
[PATCH v3 2/2] fortran: Fix specification expression error with dummy procedures [PR111781]
This fixes a spurious invalid variable in specification expression error.
The error was caused on the testcase from the PR by two different bugs.
First, the call to is_parent_of_current_ns was unable to recognize
correct host association and returned false. Second, an ad-hoc
condition coming next was using a global variable previously improperly
restored to false (instead of restoring it to its initial value). The
latter happened on the testcase because one dummy argument was a procedure,
and checking that argument what causing a check of all its arguments with
the (improper) reset of the flag at the end, and that preceded the check of
the next argument.
For the first bug, the wrong result of is_parent_of_current_ns is fixed by
correcting the namespaces that function deals with, both the one passed
as argument and the current one tracked in the gfc_current_ns global. Two
new functions are introduced to select the right namespace.
Regarding the second bug, the problematic condition is removed, together
with the formal_arg_flag associated with it. Indeed, that condition was
(wrongly) allowing local variables to be used in array bounds of dummy
arguments.
PR fortran/111781
gcc/fortran/ChangeLog:
* symbol.cc (gfc_get_procedure_ns, gfc_get_spec_ns): New functions.
* gfortran.h (gfc_get_procedure_ns, gfc_get_spec ns): Declare them.
(gfc_is_formal_arg): Remove.
* expr.cc (check_restricted): Remove special case allowing local
variable in dummy argument bound expressions. Use gfc_get_spec_ns
to get the right namespace.
* resolve.cc (gfc_is_formal_arg, formal_arg_flag): Remove.
(gfc_resolve_formal_arglist): Set gfc_current_ns. Quit loop and
restore gfc_current_ns instead of early returning.
(resolve_symbol): Factor common array spec resolution code to...
(resolve_symbol_array_spec): ... this new function. Additionnally
set and restore gfc_current_ns.
gcc/testsuite/ChangeLog:
* gfortran.dg/spec_expr_8.f90: New test.
* gfortran.dg/spec_expr_9.f90: New test.
---
gcc/fortran/expr.cc | 8 +--
gcc/fortran/gfortran.h| 4 +-
gcc/fortran/resolve.cc| 77 +++
gcc/fortran/symbol.cc | 58 +
gcc/testsuite/gfortran.dg/spec_expr_8.f90 | 24 +++
gcc/testsuite/gfortran.dg/spec_expr_9.f90 | 19 ++
6 files changed, 140 insertions(+), 50 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_8.f90
create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_9.f90
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index e4b1e8307e3..9a042cd7040 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -3514,19 +3514,13 @@ check_restricted (gfc_expr *e)
if (!check_references (e->ref, _restricted))
break;
- /* gfc_is_formal_arg broadcasts that a formal argument list is being
-processed in resolve.cc(resolve_formal_arglist). This is done so
-that host associated dummy array indices are accepted (PR23446).
-This mechanism also does the same for the specification expressions
-of array-valued functions. */
if (e->error
|| sym->attr.in_common
|| sym->attr.use_assoc
|| sym->attr.dummy
|| sym->attr.implied_index
|| sym->attr.flavor == FL_PARAMETER
- || is_parent_of_current_ns (sym->ns)
- || (gfc_is_formal_arg () && (sym->ns == gfc_current_ns)))
+ || is_parent_of_current_ns (gfc_get_spec_ns (sym)))
{
t = true;
break;
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index c7039730fad..26aa56b3358 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3612,6 +3612,9 @@ bool gfc_is_associate_pointer (gfc_symbol*);
gfc_symbol * gfc_find_dt_in_generic (gfc_symbol *);
gfc_formal_arglist *gfc_sym_get_dummy_args (gfc_symbol *);
+gfc_namespace * gfc_get_procedure_ns (gfc_symbol *);
+gfc_namespace * gfc_get_spec_ns (gfc_symbol *);
+
/* intrinsic.cc -- true if working in an init-expr, false otherwise. */
extern bool gfc_init_expr_flag;
@@ -3821,7 +3824,6 @@ bool gfc_resolve_iterator (gfc_iterator *, bool, bool);
bool find_forall_index (gfc_expr *, gfc_symbol *, int);
bool gfc_resolve_index (gfc_expr *, int);
bool gfc_resolve_dim_arg (gfc_expr *);
-bool gfc_is_formal_arg (void);
bool gfc_resolve_substring (gfc_ref *, bool *);
void gfc_resolve_substring_charlen (gfc_expr *);
gfc_expr *gfc_expr_to_initialize (gfc_expr *);
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index c5ae826bd6e..50d51b06c92 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -72,9 +72,6 @@ static bool first_actual_arg = false;
static int omp_workshare_flag;
-/* True if we are processing a formal arglist. The corresponding function
- resets the flag each
[PATCH v3 1/2] testsuite: Declare fortran array bound variables
This fixes invalid undeclared fortran array bound variables
in the testsuite.
gcc/testsuite/ChangeLog:
* gfortran.dg/graphite/pr107865.f90: Declare array bound variable(s)
as dummy argument(s).
* gfortran.dg/pr101267.f90: Likewise.
* gfortran.dg/pr112404.f90: Likewise.
* gfortran.dg/pr78061.f: Likewise.
* gfortran.dg/pr79315.f90: Likewise.
* gfortran.dg/vect/pr90681.f: Likewise.
* gfortran.dg/vect/pr97761.f90: Likewise.
* gfortran.dg/vect/pr99746.f90: Likewise.
---
gcc/testsuite/gfortran.dg/graphite/pr107865.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr101267.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr112404.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr78061.f | 2 +-
gcc/testsuite/gfortran.dg/pr79315.f90 | 6 +-
gcc/testsuite/gfortran.dg/vect/pr90681.f| 2 +-
gcc/testsuite/gfortran.dg/vect/pr97761.f90 | 2 +-
gcc/testsuite/gfortran.dg/vect/pr99746.f90 | 2 +-
8 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
b/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
index 6bddb17a1be..323d8092ad2 100644
--- a/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
+++ b/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-O1 -floop-parallelize-all -ftree-parallelize-loops=2" }
- SUBROUTINE FNC (F)
+ SUBROUTINE FNC (F,N)
IMPLICIT REAL (A-H)
DIMENSION F(N)
diff --git a/gcc/testsuite/gfortran.dg/pr101267.f90
b/gcc/testsuite/gfortran.dg/pr101267.f90
index 12723cf9c22..99a6dcfa342 100644
--- a/gcc/testsuite/gfortran.dg/pr101267.f90
+++ b/gcc/testsuite/gfortran.dg/pr101267.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-Ofast" }
! { dg-additional-options "-march=znver2" { target x86_64-*-* i?86-*-* } }
- SUBROUTINE sfddagd( regime, znt,ite ,jte )
+ SUBROUTINE sfddagd( regime, znt,ite ,jte, ime, IN )
REAL, DIMENSION( ime, IN) :: regime, znt
REAL, DIMENSION( ite, jte) :: wndcor_u
LOGICAL wrf_dm_on_monitor
diff --git a/gcc/testsuite/gfortran.dg/pr112404.f90
b/gcc/testsuite/gfortran.dg/pr112404.f90
index 573fa28164a..4508bbc8738 100644
--- a/gcc/testsuite/gfortran.dg/pr112404.f90
+++ b/gcc/testsuite/gfortran.dg/pr112404.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-Ofast" }
! { dg-additional-options "-mavx2" { target avx2 } }
- SUBROUTINE sfddagd( regime, znt, ite, jte )
+ SUBROUTINE sfddagd( regime, znt, ite, jte, ime, IN )
REAL, DIMENSION( ime, IN) :: regime, znt
REAL, DIMENSION( ite, jte) :: wndcor_u
LOGICAL wrf_dm_on_monitor
diff --git a/gcc/testsuite/gfortran.dg/pr78061.f
b/gcc/testsuite/gfortran.dg/pr78061.f
index 7e4dd3de8b5..9061dea74da 100644
--- a/gcc/testsuite/gfortran.dg/pr78061.f
+++ b/gcc/testsuite/gfortran.dg/pr78061.f
@@ -1,6 +1,6 @@
! { dg-do compile }
! { dg-options "-O3 -fsplit-loops" }
- SUBROUTINE SSYMM(C)
+ SUBROUTINE SSYMM(C,LDC)
REAL C(LDC,*)
LOGICAL LSAME
LOGICAL UPPER
diff --git a/gcc/testsuite/gfortran.dg/pr79315.f90
b/gcc/testsuite/gfortran.dg/pr79315.f90
index 8cd89691ce9..b754a2b3274 100644
--- a/gcc/testsuite/gfortran.dg/pr79315.f90
+++ b/gcc/testsuite/gfortran.dg/pr79315.f90
@@ -10,7 +10,11 @@ SUBROUTINE wsm32D(t, &
its,&
ite, &
kts, &
- kte &
+ kte, &
+ ims, &
+ ime, &
+ kms, &
+ kme &
)
REAL, DIMENSION( its:ite , kts:kte ), &
INTENT(INOUT) :: &
diff --git a/gcc/testsuite/gfortran.dg/vect/pr90681.f
b/gcc/testsuite/gfortran.dg/vect/pr90681.f
index 03d3987b146..49f1d50ab8f 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr90681.f
+++ b/gcc/testsuite/gfortran.dg/vect/pr90681.f
@@ -1,6 +1,6 @@
C { dg-do compile }
C { dg-additional-options "-march=armv8.2-a+sve" { target { aarch64*-*-* } } }
- SUBROUTINE HMU (H1)
+ SUBROUTINE HMU (H1,NORBS)
COMMON DD(107)
DIMENSION H1(NORBS,*)
DO 70 J1 = IA,I1
diff --git a/gcc/testsuite/gfortran.dg/vect/pr97761.f90
b/gcc/testsuite/gfortran.dg/vect/pr97761.f90
index 250e2bf016e..401ef06e422 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr97761.f90
+++ b/gcc/testsuite/gfortran.dg/vect/pr97761.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-additional-options "-O1" }
-subroutine ni (ps)
+subroutine ni (ps, inout)
type vector
real x, y
end type
diff --git a/gcc/testsuite/gfortran.dg/vect/pr99746.f90
b/gcc/testsuite/gfortran.dg/vect/pr99746.f90
index fe947ae7ccf..121d67d564d 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr99746.f90
+++ b/gcc/testsuite/gfortran.dg/vect/pr99746.f90
@@ -1,6 +1,6 @@
! { dg-do compile }
! { dg-additional-options "-march=armv8.3-a" { target aarch64-*-* } }
-SUBROUTINE CLAREF(A, WANTZ, Z, ICOL1, ITMP1, ITMP2, T1, T2, V2)
+SUBROUTINE CLAREF(A, WANTZ, Z, ICOL1, ITMP1, ITMP2, T1, T2, V2, LDA)
[PATCH v3 0/2] fortran: Fix specification checks [PR111781]
Hello, these patches correct diagnostics dealing with variables in specification expressions. The first patch is a testsuite change, which fixes invalid specification expressions that the second patch would diagnose. The second patch removes a spurious diagnostic when a dummy procedure is involved, and enables more valid ones, as visible in the testcases from the first patch. I haven't tested it again (same code as v2), but I plan to do it before the final push. Ok for master? Mikael v2 -> v3 changes: - Correct first name in testcase comment - Clarify and correct log and changelog text from second patch - Target current stage (stage4) instead of next (stage1) v1 -> v2 changes: - Fix condition guarding sym->result access. Mikael Morin (2): testsuite: Declare fortran array bound variables fortran: Fix specification expression error with dummy procedures [PR111781] gcc/fortran/expr.cc | 8 +- gcc/fortran/gfortran.h| 4 +- gcc/fortran/resolve.cc| 77 +-- gcc/fortran/symbol.cc | 58 ++ .../gfortran.dg/graphite/pr107865.f90 | 2 +- gcc/testsuite/gfortran.dg/pr101267.f90| 2 +- gcc/testsuite/gfortran.dg/pr112404.f90| 2 +- gcc/testsuite/gfortran.dg/pr78061.f | 2 +- gcc/testsuite/gfortran.dg/pr79315.f90 | 6 +- gcc/testsuite/gfortran.dg/spec_expr_8.f90 | 24 ++ gcc/testsuite/gfortran.dg/spec_expr_9.f90 | 19 + gcc/testsuite/gfortran.dg/vect/pr90681.f | 2 +- gcc/testsuite/gfortran.dg/vect/pr97761.f90| 2 +- gcc/testsuite/gfortran.dg/vect/pr99746.f90| 2 +- 14 files changed, 152 insertions(+), 58 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_8.f90 create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_9.f90 -- 2.43.0
Re: [PATCH, v2] Fortran: fix for absent array argument passed to optional dummy [PR101135]
Le 17/03/2024 à 23:10, Harald Anlauf a écrit : Hi Mikael, On 3/17/24 22:04, Mikael Morin wrote: diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 3673fa40720..a7717a8107e 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -7526,6 +7526,17 @@ gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset, /* Set the target data pointer. */ offset = gfc_build_addr_expr (gfc_array_dataptr_type (desc), tmp); + + /* Check for optional dummy argument being present. Arguments of BIND(C) + procedures are excepted here since they are handled differently. */ + if (expr->expr_type == EXPR_VARIABLE + && expr->symtree->n.sym->attr.dummy + && expr->symtree->n.sym->attr.optional + && !is_CFI_desc (NULL, expr)) I think the condition could additionally check the lack of subreferences. But it's maybe not worth the trouble, and the patch is conservatively correct as is, so OK. I have thought about the conditions here for some time and did not find better ones. They need to be broad enough to catch the case in gfortran.dg/missing_optional_dummy_6a.f90 that (according to the tree-dump) was not properly handled previously and would have triggered ubsan at some point in the future when someone tried to change that testcase from currently dg-do compile to dg-do run... No problem, as said it is conservatively correct. (After the patch it would pass, but I didn't dare to change the dg-do). Did it include cases not covered by the new testcase (which was quite complete already)? I have pushed the patch as-is, but feel free to post testcases not covered (or improperly covered) to narrow this down further... The case I had in mind would only be a missed optimization, and probably not that important, so let's move on.
Re: RE: [PATCH] [X86_64]: Enable support for next generation AMD Zen5 CPU with znver5 scheduler Model
Hello,
Le 22/02/2024 à 19:29, Anbazhagan, Karthiban a écrit :
(...)
gcc/config/i386/{znver4.md => zn4zn5.md} | 858 +-
looks like the patch pushed to master lost the file rename.
I get a bootstrap failure caused by the missing zn4zn5.md file.
Can you have a look?
Re: [PATCH v2 2/2] fortran: Fix specification expression error with dummy procedures [PR111781]
Le 17/03/2024 à 21:57, Harald Anlauf a écrit :
Hi Mikael,
thanks for the patch!
Regarding the first part of the patch, I think that fixing bad testcases
can be done at any time. Retaining identified, broken testcases means
that one may hit bogus regressions, hindering progress.
The second part of the patch looks at first glance fine to me. And as
the patch changes less than its size suggests, in particular due to
code refactoring, I don't see a reason to postpone it to stage 1.
What worries me a bit is the changes of gfc_current_ns.
It's the kind of change that has broad impact and can uncover weird
behaviors.
(On the contrary, deferring it to stage 1 might make future backports
from 15 for later patches on top of that code harder.
This is my opinion, and others might see this differently.)
To be honest, I posted it now in the hope that someone would be willing
to push it before stage 1 so that I can get rid of it sooner.
On 3/17/24 17:57, Mikael Morin wrote:
* expr.cc (check_restricted): Remove the case where symbol is dummy
and declared in the current ns. Use gfc_get_spec_ns to get the
right namespace.
Looking at the original and new code, I don't fully understand
that part of the commit message: the changed check comes into play
when the symbol is *not* in_common, ..., a dummy, ...
So technically, we didn't access the (now removed) formal_arg_flag
here in those cases.
Or am I missing something?
Oh, do you really read all of my prose?
I wrote it from vague memories of debugging the problem weeks ago,
without thinking very much about it.
Actually, there are two dummy arguments here.
There is the one we are checking, and its bounds should be a
specification expression, so could possibly be another dummy variable
(the "sym" variable in this context).
The condition was allowing variables declared in the same scope to
appear in specification expression of dummy arguments.
I will try to rephrase the sentence, as it sounds more clear (and
sensible) as expressed here.
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 82a642b01f7..0852bc5f493 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -3509,19 +3509,13 @@ check_restricted (gfc_expr *e)
if (!check_references (e->ref, _restricted))
break;
- /* gfc_is_formal_arg broadcasts that a formal argument list is
being
- processed in resolve.cc(resolve_formal_arglist). This is done so
- that host associated dummy array indices are accepted (PR23446).
- This mechanism also does the same for the specification expressions
- of array-valued functions. */
if (e->error
|| sym->attr.in_common
|| sym->attr.use_assoc
|| sym->attr.dummy
|| sym->attr.implied_index
|| sym->attr.flavor == FL_PARAMETER
- || is_parent_of_current_ns (sym->ns)
- || (gfc_is_formal_arg () && (sym->ns == gfc_current_ns)))
+ || is_parent_of_current_ns (gfc_get_spec_ns (sym)))
{
t = true;
break;
diff --git a/gcc/testsuite/gfortran.dg/spec_expr_8.f90
b/gcc/testsuite/gfortran.dg/spec_expr_8.f90
new file mode 100644
index 000..5885810d421
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/spec_expr_8.f90
@@ -0,0 +1,24 @@
+! { dg-do compile }
+!
+! PR fortran/111781
+! We used to reject the example below because the dummy procedure g was
+! setting the current namespace without properly restoring it, which
broke
+! the specification expression check for the dimension of A later on.
+!
+! Contributed by Markus Vikhamar-Sandberg
Is the reporter's first name Markus or Rasmus?
Rasmus.
Reviews are sometimes helpful, more often than not.
I don't know where the Markus name comes from.
Thanks for the review.
Re: [PATCH, v2] Fortran: fix for absent array argument passed to optional dummy [PR101135]
Le 15/03/2024 à 20:32, Harald Anlauf a écrit : Dear all, as there has been some good progress in the handling of optional dummy arguments, I looked again at this PR and a patch for it that I withdrew as it turned out incomplete. It turned out that it now needs only a minor adjustment for optional dummy arguments of procedures with bind(c) attribute so that ubsan checking does not trigger. Along this way I extended the previous testcase to exercise to some extent combinations of bind(c) and non-bind(c) procedures and found one failure (since at least gcc-9) that is genuine: passing a missing optional from a bind(c) procedure to an assumed-rank dummy, see PR114355. The corresponding test is commented in the testcase. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald (...) From b3079a82a220477704f8156207239e4af4103ea9 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 15 Mar 2024 20:14:07 +0100 Subject: [PATCH] Fortran: fix for absent array argument passed to optional dummy [PR101135] gcc/fortran/ChangeLog: PR fortran/101135 * trans-array.cc (gfc_get_dataptr_offset): Check for optional arguments being present before dereferencing data pointer. gcc/testsuite/ChangeLog: PR fortran/101135 * gfortran.dg/missing_optional_dummy_6a.f90: Adjust diagnostic pattern. * gfortran.dg/ubsan/missing_optional_dummy_8.f90: New test. --- gcc/fortran/trans-array.cc| 11 ++ .../gfortran.dg/missing_optional_dummy_6a.f90 | 2 +- .../ubsan/missing_optional_dummy_8.f90| 108 ++ 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/ubsan/missing_optional_dummy_8.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 3673fa40720..a7717a8107e 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -7526,6 +7526,17 @@ gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset, /* Set the target data pointer. */ offset = gfc_build_addr_expr (gfc_array_dataptr_type (desc), tmp); + + /* Check for optional dummy argument being present. Arguments of BIND(C) + procedures are excepted here since they are handled differently. */ + if (expr->expr_type == EXPR_VARIABLE + && expr->symtree->n.sym->attr.dummy + && expr->symtree->n.sym->attr.optional + && !is_CFI_desc (NULL, expr)) I think the condition could additionally check the lack of subreferences. But it's maybe not worth the trouble, and the patch is conservatively correct as is, so OK. Thanks for the patch. +offset = build3_loc (input_location, COND_EXPR, TREE_TYPE (offset), +gfc_conv_expr_present (expr->symtree->n.sym), offset, +fold_convert (TREE_TYPE (offset), gfc_index_zero_node)); + gfc_conv_descriptor_data_set (block, parm, offset); }
[PATCH v2 2/2] fortran: Fix specification expression error with dummy procedures [PR111781]
This fixes a spurious invalid variable in specification expression error.
The problem is caused by improper restoration of formal_arg_flag to false
(instead of restoring it to its previous value). This happens with the
testcase from the PR where a dummy argument is itself a procedure with dummy
arguments. At the time that dummy procedure has been resolved, its dummy
arguments have been resolved and the flag has been reset to false. The
condition checking that flag when resolving the array spec expressions of
the next dummy then triggers the error.
The fix removes a condition disabling proper check of specification
expressions, together with the formal_arg_flag global variable associated
with it. As the specification expression checking code is dependent on the
current namespace, the latter is set before array spec resolution (and
restored after). Two new functions are introduced to select the right
namespace for that.
PR fortran/111781
gcc/fortran/ChangeLog:
* symbol.cc (gfc_get_procedure_ns, gfc_get_spec_ns): New functions.
* gfortran.h (gfc_get_procedure_ns, gfc_get_spec ns): Declare them.
(gfc_is_formal_arg): Remove.
* expr.cc (check_restricted): Remove the case where symbol is dummy
and declared in the current ns. Use gfc_get_spec_ns to get the
right namespace.
* resolve.cc (gfc_is_formal_arg, formal_arg_flag): Remove.
(gfc_resolve_formal_arglist): Set gfc_current_ns. Quit loop and
restore gfc_current_ns instead of early returning.
(resolve_symbol): Factor common array spec resolution code to...
(resolve_symbol_array_spec): ... this new function. Additionnally
set and restore gfc_current_ns.
gcc/testsuite/ChangeLog:
* gfortran.dg/spec_expr_8.f90: New test.
* gfortran.dg/spec_expr_9.f90: New test.
---
gcc/fortran/expr.cc | 8 +--
gcc/fortran/gfortran.h| 4 +-
gcc/fortran/resolve.cc| 77 +++
gcc/fortran/symbol.cc | 58 +
gcc/testsuite/gfortran.dg/spec_expr_8.f90 | 24 +++
gcc/testsuite/gfortran.dg/spec_expr_9.f90 | 19 ++
6 files changed, 140 insertions(+), 50 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_8.f90
create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_9.f90
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 82a642b01f7..0852bc5f493 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -3509,19 +3509,13 @@ check_restricted (gfc_expr *e)
if (!check_references (e->ref, _restricted))
break;
- /* gfc_is_formal_arg broadcasts that a formal argument list is being
-processed in resolve.cc(resolve_formal_arglist). This is done so
-that host associated dummy array indices are accepted (PR23446).
-This mechanism also does the same for the specification expressions
-of array-valued functions. */
if (e->error
|| sym->attr.in_common
|| sym->attr.use_assoc
|| sym->attr.dummy
|| sym->attr.implied_index
|| sym->attr.flavor == FL_PARAMETER
- || is_parent_of_current_ns (sym->ns)
- || (gfc_is_formal_arg () && (sym->ns == gfc_current_ns)))
+ || is_parent_of_current_ns (gfc_get_spec_ns (sym)))
{
t = true;
break;
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 32b792f85fb..f954b7a8802 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3605,6 +3605,9 @@ bool gfc_is_associate_pointer (gfc_symbol*);
gfc_symbol * gfc_find_dt_in_generic (gfc_symbol *);
gfc_formal_arglist *gfc_sym_get_dummy_args (gfc_symbol *);
+gfc_namespace * gfc_get_procedure_ns (gfc_symbol *);
+gfc_namespace * gfc_get_spec_ns (gfc_symbol *);
+
/* intrinsic.cc -- true if working in an init-expr, false otherwise. */
extern bool gfc_init_expr_flag;
@@ -3813,7 +3816,6 @@ bool gfc_resolve_iterator (gfc_iterator *, bool, bool);
bool find_forall_index (gfc_expr *, gfc_symbol *, int);
bool gfc_resolve_index (gfc_expr *, int);
bool gfc_resolve_dim_arg (gfc_expr *);
-bool gfc_is_formal_arg (void);
bool gfc_resolve_substring (gfc_ref *, bool *);
void gfc_resolve_substring_charlen (gfc_expr *);
gfc_expr *gfc_expr_to_initialize (gfc_expr *);
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 02acc4aef31..6bdb56038bb 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -72,9 +72,6 @@ static bool first_actual_arg = false;
static int omp_workshare_flag;
-/* True if we are processing a formal arglist. The corresponding function
- resets the flag each time that it is read. */
-static bool formal_arg_flag = false;
/* True if we are resolving a specification expression. */
static bool specification_expr = false;
@@ -89,12 +86,6 @@ static bitmap_obstack labels_obstack;
static bool
[PATCH v2 1/2] testsuite: Declare fortran array bound variables
This fixes invalid undeclared fortran array bound variables
in the testsuite.
gcc/testsuite/ChangeLog:
* gfortran.dg/graphite/pr107865.f90: Declare array bound variable(s)
as dummy argument(s).
* gfortran.dg/pr101267.f90: Likewise.
* gfortran.dg/pr112404.f90: Likewise.
* gfortran.dg/pr78061.f: Likewise.
* gfortran.dg/pr79315.f90: Likewise.
* gfortran.dg/vect/pr90681.f: Likewise.
* gfortran.dg/vect/pr97761.f90: Likewise.
* gfortran.dg/vect/pr99746.f90: Likewise.
---
gcc/testsuite/gfortran.dg/graphite/pr107865.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr101267.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr112404.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr78061.f | 2 +-
gcc/testsuite/gfortran.dg/pr79315.f90 | 6 +-
gcc/testsuite/gfortran.dg/vect/pr90681.f| 2 +-
gcc/testsuite/gfortran.dg/vect/pr97761.f90 | 2 +-
gcc/testsuite/gfortran.dg/vect/pr99746.f90 | 2 +-
8 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
b/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
index 6bddb17a1be..323d8092ad2 100644
--- a/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
+++ b/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-O1 -floop-parallelize-all -ftree-parallelize-loops=2" }
- SUBROUTINE FNC (F)
+ SUBROUTINE FNC (F,N)
IMPLICIT REAL (A-H)
DIMENSION F(N)
diff --git a/gcc/testsuite/gfortran.dg/pr101267.f90
b/gcc/testsuite/gfortran.dg/pr101267.f90
index 12723cf9c22..99a6dcfa342 100644
--- a/gcc/testsuite/gfortran.dg/pr101267.f90
+++ b/gcc/testsuite/gfortran.dg/pr101267.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-Ofast" }
! { dg-additional-options "-march=znver2" { target x86_64-*-* i?86-*-* } }
- SUBROUTINE sfddagd( regime, znt,ite ,jte )
+ SUBROUTINE sfddagd( regime, znt,ite ,jte, ime, IN )
REAL, DIMENSION( ime, IN) :: regime, znt
REAL, DIMENSION( ite, jte) :: wndcor_u
LOGICAL wrf_dm_on_monitor
diff --git a/gcc/testsuite/gfortran.dg/pr112404.f90
b/gcc/testsuite/gfortran.dg/pr112404.f90
index 573fa28164a..4508bbc8738 100644
--- a/gcc/testsuite/gfortran.dg/pr112404.f90
+++ b/gcc/testsuite/gfortran.dg/pr112404.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-Ofast" }
! { dg-additional-options "-mavx2" { target avx2 } }
- SUBROUTINE sfddagd( regime, znt, ite, jte )
+ SUBROUTINE sfddagd( regime, znt, ite, jte, ime, IN )
REAL, DIMENSION( ime, IN) :: regime, znt
REAL, DIMENSION( ite, jte) :: wndcor_u
LOGICAL wrf_dm_on_monitor
diff --git a/gcc/testsuite/gfortran.dg/pr78061.f
b/gcc/testsuite/gfortran.dg/pr78061.f
index 7e4dd3de8b5..9061dea74da 100644
--- a/gcc/testsuite/gfortran.dg/pr78061.f
+++ b/gcc/testsuite/gfortran.dg/pr78061.f
@@ -1,6 +1,6 @@
! { dg-do compile }
! { dg-options "-O3 -fsplit-loops" }
- SUBROUTINE SSYMM(C)
+ SUBROUTINE SSYMM(C,LDC)
REAL C(LDC,*)
LOGICAL LSAME
LOGICAL UPPER
diff --git a/gcc/testsuite/gfortran.dg/pr79315.f90
b/gcc/testsuite/gfortran.dg/pr79315.f90
index 8cd89691ce9..b754a2b3274 100644
--- a/gcc/testsuite/gfortran.dg/pr79315.f90
+++ b/gcc/testsuite/gfortran.dg/pr79315.f90
@@ -10,7 +10,11 @@ SUBROUTINE wsm32D(t, &
its,&
ite, &
kts, &
- kte &
+ kte, &
+ ims, &
+ ime, &
+ kms, &
+ kme &
)
REAL, DIMENSION( its:ite , kts:kte ), &
INTENT(INOUT) :: &
diff --git a/gcc/testsuite/gfortran.dg/vect/pr90681.f
b/gcc/testsuite/gfortran.dg/vect/pr90681.f
index 03d3987b146..49f1d50ab8f 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr90681.f
+++ b/gcc/testsuite/gfortran.dg/vect/pr90681.f
@@ -1,6 +1,6 @@
C { dg-do compile }
C { dg-additional-options "-march=armv8.2-a+sve" { target { aarch64*-*-* } } }
- SUBROUTINE HMU (H1)
+ SUBROUTINE HMU (H1,NORBS)
COMMON DD(107)
DIMENSION H1(NORBS,*)
DO 70 J1 = IA,I1
diff --git a/gcc/testsuite/gfortran.dg/vect/pr97761.f90
b/gcc/testsuite/gfortran.dg/vect/pr97761.f90
index 250e2bf016e..401ef06e422 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr97761.f90
+++ b/gcc/testsuite/gfortran.dg/vect/pr97761.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-additional-options "-O1" }
-subroutine ni (ps)
+subroutine ni (ps, inout)
type vector
real x, y
end type
diff --git a/gcc/testsuite/gfortran.dg/vect/pr99746.f90
b/gcc/testsuite/gfortran.dg/vect/pr99746.f90
index fe947ae7ccf..121d67d564d 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr99746.f90
+++ b/gcc/testsuite/gfortran.dg/vect/pr99746.f90
@@ -1,6 +1,6 @@
! { dg-do compile }
! { dg-additional-options "-march=armv8.3-a" { target aarch64-*-* } }
-SUBROUTINE CLAREF(A, WANTZ, Z, ICOL1, ITMP1, ITMP2, T1, T2, V2)
+SUBROUTINE CLAREF(A, WANTZ, Z, ICOL1, ITMP1, ITMP2, T1, T2, V2, LDA)
[PATCH v2 0/2] fortran: Fix specification checks [PR111781]
Meh, the first version contained out-of-date patches. these patches correct diagnostics dealing with variables in specification expressions. The first patch is a testsuite change, which fixes invalid specification expressions that the second patch would diagnose. The second patch removes a spurious diagnostic when a dummy procedure is involved, and enables more valid ones, as visible in the testcases from the first patch. The patch is not completely trivial, and fix what is not really a regression, so it is more for stage1, I think. Tested for regression on x86_64-pc-linux-gnu. Ok for master when stage1 opens? Mikael v1 -> v2 changes: - Fix condition guarding sym->result access. Mikael Morin (2): testsuite: Declare fortran array bound variables fortran: Fix specification expression error with dummy procedures [PR111781] gcc/fortran/expr.cc | 8 +- gcc/fortran/gfortran.h| 4 +- gcc/fortran/resolve.cc| 77 +-- gcc/fortran/symbol.cc | 58 ++ .../gfortran.dg/graphite/pr107865.f90 | 2 +- gcc/testsuite/gfortran.dg/pr101267.f90| 2 +- gcc/testsuite/gfortran.dg/pr112404.f90| 2 +- gcc/testsuite/gfortran.dg/pr78061.f | 2 +- gcc/testsuite/gfortran.dg/pr79315.f90 | 6 +- gcc/testsuite/gfortran.dg/spec_expr_8.f90 | 24 ++ gcc/testsuite/gfortran.dg/spec_expr_9.f90 | 19 + gcc/testsuite/gfortran.dg/vect/pr90681.f | 2 +- gcc/testsuite/gfortran.dg/vect/pr97761.f90| 2 +- gcc/testsuite/gfortran.dg/vect/pr99746.f90| 2 +- 14 files changed, 152 insertions(+), 58 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_8.f90 create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_9.f90 -- 2.43.0
[PATCH 2/2] fortran: Fix specification expression error with dummy procedures [PR111781]
This fixes a spurious invalid variable in specification expression error.
The problem is caused by improper restoration of formal_arg_flag to false
(instead of restoring it to its previous value). This happens with the
testcase from the PR where a dummy argument is itself a procedure with dummy
arguments. At the time that dummy procedure has been resolved, its dummy
arguments have been resolved and the flag has been reset to false. The
condition checking that flag when resolving the array spec expressions of
the next dummy then triggers the error.
The fix removes a condition disabling proper check of specification
expressions, together with the formal_arg_flag global variable associated
with it. As the specification expression checking code is dependent on the
current namespace, the latter is set before array spec resolution (and
restored after). Two new functions are introduced to select the right
namespace for that.
PR fortran/111781
gcc/fortran/ChangeLog:
* symbol.cc (gfc_get_procedure_ns, gfc_get_spec_ns): New functions.
* gfortran.h (gfc_get_procedure_ns, gfc_get_spec ns): Declare them.
(gfc_is_formal_arg): Remove.
* expr.cc (check_restricted): Remove the case where symbol is dummy
and declared in the current ns. Use gfc_get_spec_ns to get the
right namespace.
* resolve.cc (gfc_is_formal_arg, formal_arg_flag): Remove.
(gfc_resolve_formal_arglist): Set gfc_current_ns. Quit loop and
restore gfc_current_ns instead of early returning.
(resolve_symbol): Factor common array spec resolution code to...
(resolve_symbol_array_spec): ... this new function. Additionnally
set and restore gfc_current_ns.
gcc/testsuite/ChangeLog:
* gfortran.dg/spec_expr_8.f90: New test.
* gfortran.dg/spec_expr_9.f90: New test.
---
gcc/fortran/expr.cc | 8 +--
gcc/fortran/gfortran.h| 4 +-
gcc/fortran/resolve.cc| 77 +++
gcc/fortran/symbol.cc | 57 +
gcc/testsuite/gfortran.dg/spec_expr_8.f90 | 24 +++
gcc/testsuite/gfortran.dg/spec_expr_9.f90 | 19 ++
6 files changed, 139 insertions(+), 50 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_8.f90
create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_9.f90
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 82a642b01f7..0852bc5f493 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -3509,19 +3509,13 @@ check_restricted (gfc_expr *e)
if (!check_references (e->ref, _restricted))
break;
- /* gfc_is_formal_arg broadcasts that a formal argument list is being
-processed in resolve.cc(resolve_formal_arglist). This is done so
-that host associated dummy array indices are accepted (PR23446).
-This mechanism also does the same for the specification expressions
-of array-valued functions. */
if (e->error
|| sym->attr.in_common
|| sym->attr.use_assoc
|| sym->attr.dummy
|| sym->attr.implied_index
|| sym->attr.flavor == FL_PARAMETER
- || is_parent_of_current_ns (sym->ns)
- || (gfc_is_formal_arg () && (sym->ns == gfc_current_ns)))
+ || is_parent_of_current_ns (gfc_get_spec_ns (sym)))
{
t = true;
break;
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 32b792f85fb..f954b7a8802 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3605,6 +3605,9 @@ bool gfc_is_associate_pointer (gfc_symbol*);
gfc_symbol * gfc_find_dt_in_generic (gfc_symbol *);
gfc_formal_arglist *gfc_sym_get_dummy_args (gfc_symbol *);
+gfc_namespace * gfc_get_procedure_ns (gfc_symbol *);
+gfc_namespace * gfc_get_spec_ns (gfc_symbol *);
+
/* intrinsic.cc -- true if working in an init-expr, false otherwise. */
extern bool gfc_init_expr_flag;
@@ -3813,7 +3816,6 @@ bool gfc_resolve_iterator (gfc_iterator *, bool, bool);
bool find_forall_index (gfc_expr *, gfc_symbol *, int);
bool gfc_resolve_index (gfc_expr *, int);
bool gfc_resolve_dim_arg (gfc_expr *);
-bool gfc_is_formal_arg (void);
bool gfc_resolve_substring (gfc_ref *, bool *);
void gfc_resolve_substring_charlen (gfc_expr *);
gfc_expr *gfc_expr_to_initialize (gfc_expr *);
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 02acc4aef31..6bdb56038bb 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -72,9 +72,6 @@ static bool first_actual_arg = false;
static int omp_workshare_flag;
-/* True if we are processing a formal arglist. The corresponding function
- resets the flag each time that it is read. */
-static bool formal_arg_flag = false;
/* True if we are resolving a specification expression. */
static bool specification_expr = false;
@@ -89,12 +86,6 @@ static bitmap_obstack labels_obstack;
static bool
[PATCH 1/2] testsuite: Declare fortran array bound variables
This fixes invalid undeclared fortran array bound variables
in the testsuite.
gcc/testsuite/ChangeLog:
* gfortran.dg/graphite/pr107865.f90: Declare array bound variable(s)
as dummy argument(s).
* gfortran.dg/pr101267.f90: Likewise.
* gfortran.dg/pr112404.f90: Likewise.
* gfortran.dg/pr78061.f: Likewise.
* gfortran.dg/pr79315.f90: Likewise.
* gfortran.dg/vect/pr90681.f: Likewise.
* gfortran.dg/vect/pr97761.f90: Likewise.
* gfortran.dg/vect/pr99746.f90: Likewise.
---
gcc/testsuite/gfortran.dg/graphite/pr107865.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr101267.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr112404.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr78061.f | 2 +-
gcc/testsuite/gfortran.dg/pr79315.f90 | 6 +-
gcc/testsuite/gfortran.dg/vect/pr90681.f| 2 +-
gcc/testsuite/gfortran.dg/vect/pr97761.f90 | 2 +-
gcc/testsuite/gfortran.dg/vect/pr99746.f90 | 2 +-
8 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
b/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
index 6bddb17a1be..323d8092ad2 100644
--- a/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
+++ b/gcc/testsuite/gfortran.dg/graphite/pr107865.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-O1 -floop-parallelize-all -ftree-parallelize-loops=2" }
- SUBROUTINE FNC (F)
+ SUBROUTINE FNC (F,N)
IMPLICIT REAL (A-H)
DIMENSION F(N)
diff --git a/gcc/testsuite/gfortran.dg/pr101267.f90
b/gcc/testsuite/gfortran.dg/pr101267.f90
index 12723cf9c22..99a6dcfa342 100644
--- a/gcc/testsuite/gfortran.dg/pr101267.f90
+++ b/gcc/testsuite/gfortran.dg/pr101267.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-Ofast" }
! { dg-additional-options "-march=znver2" { target x86_64-*-* i?86-*-* } }
- SUBROUTINE sfddagd( regime, znt,ite ,jte )
+ SUBROUTINE sfddagd( regime, znt,ite ,jte, ime, IN )
REAL, DIMENSION( ime, IN) :: regime, znt
REAL, DIMENSION( ite, jte) :: wndcor_u
LOGICAL wrf_dm_on_monitor
diff --git a/gcc/testsuite/gfortran.dg/pr112404.f90
b/gcc/testsuite/gfortran.dg/pr112404.f90
index 573fa28164a..4508bbc8738 100644
--- a/gcc/testsuite/gfortran.dg/pr112404.f90
+++ b/gcc/testsuite/gfortran.dg/pr112404.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-options "-Ofast" }
! { dg-additional-options "-mavx2" { target avx2 } }
- SUBROUTINE sfddagd( regime, znt, ite, jte )
+ SUBROUTINE sfddagd( regime, znt, ite, jte, ime, IN )
REAL, DIMENSION( ime, IN) :: regime, znt
REAL, DIMENSION( ite, jte) :: wndcor_u
LOGICAL wrf_dm_on_monitor
diff --git a/gcc/testsuite/gfortran.dg/pr78061.f
b/gcc/testsuite/gfortran.dg/pr78061.f
index 7e4dd3de8b5..9061dea74da 100644
--- a/gcc/testsuite/gfortran.dg/pr78061.f
+++ b/gcc/testsuite/gfortran.dg/pr78061.f
@@ -1,6 +1,6 @@
! { dg-do compile }
! { dg-options "-O3 -fsplit-loops" }
- SUBROUTINE SSYMM(C)
+ SUBROUTINE SSYMM(C,LDC)
REAL C(LDC,*)
LOGICAL LSAME
LOGICAL UPPER
diff --git a/gcc/testsuite/gfortran.dg/pr79315.f90
b/gcc/testsuite/gfortran.dg/pr79315.f90
index 8cd89691ce9..b754a2b3274 100644
--- a/gcc/testsuite/gfortran.dg/pr79315.f90
+++ b/gcc/testsuite/gfortran.dg/pr79315.f90
@@ -10,7 +10,11 @@ SUBROUTINE wsm32D(t, &
its,&
ite, &
kts, &
- kte &
+ kte, &
+ ims, &
+ ime, &
+ kms, &
+ kme &
)
REAL, DIMENSION( its:ite , kts:kte ), &
INTENT(INOUT) :: &
diff --git a/gcc/testsuite/gfortran.dg/vect/pr90681.f
b/gcc/testsuite/gfortran.dg/vect/pr90681.f
index 03d3987b146..49f1d50ab8f 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr90681.f
+++ b/gcc/testsuite/gfortran.dg/vect/pr90681.f
@@ -1,6 +1,6 @@
C { dg-do compile }
C { dg-additional-options "-march=armv8.2-a+sve" { target { aarch64*-*-* } } }
- SUBROUTINE HMU (H1)
+ SUBROUTINE HMU (H1,NORBS)
COMMON DD(107)
DIMENSION H1(NORBS,*)
DO 70 J1 = IA,I1
diff --git a/gcc/testsuite/gfortran.dg/vect/pr97761.f90
b/gcc/testsuite/gfortran.dg/vect/pr97761.f90
index 250e2bf016e..401ef06e422 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr97761.f90
+++ b/gcc/testsuite/gfortran.dg/vect/pr97761.f90
@@ -1,7 +1,7 @@
! { dg-do compile }
! { dg-additional-options "-O1" }
-subroutine ni (ps)
+subroutine ni (ps, inout)
type vector
real x, y
end type
diff --git a/gcc/testsuite/gfortran.dg/vect/pr99746.f90
b/gcc/testsuite/gfortran.dg/vect/pr99746.f90
index fe947ae7ccf..121d67d564d 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr99746.f90
+++ b/gcc/testsuite/gfortran.dg/vect/pr99746.f90
@@ -1,6 +1,6 @@
! { dg-do compile }
! { dg-additional-options "-march=armv8.3-a" { target aarch64-*-* } }
-SUBROUTINE CLAREF(A, WANTZ, Z, ICOL1, ITMP1, ITMP2, T1, T2, V2)
+SUBROUTINE CLAREF(A, WANTZ, Z, ICOL1, ITMP1, ITMP2, T1, T2, V2, LDA)
[PATCH 0/2] fortran: Fix specification checks [PR111781]
Hello, these patches correct diagnostics dealing with variables in specification expressions. The first patch is a testsuite change, which fixes invalid specification expressions that the second patch would diagnose. The second patch removes a spurious diagnostic when a dummy procedure is involved, and enables more valid ones, as visible in the testcases from the first patch. The patch is not completely trivial, and fix what is not really a regression, so it is more for stage1, I think. Tested for regression on x86_64-pc-linux-gnu. Ok for master when stage1 opens? Mikael Morin (2): testsuite: Declare fortran array bound variables fortran: Fix specification expression error with dummy procedures [PR111781] gcc/fortran/expr.cc | 8 +- gcc/fortran/gfortran.h| 4 +- gcc/fortran/resolve.cc| 77 +-- gcc/fortran/symbol.cc | 57 ++ .../gfortran.dg/graphite/pr107865.f90 | 2 +- gcc/testsuite/gfortran.dg/pr101267.f90| 2 +- gcc/testsuite/gfortran.dg/pr112404.f90| 2 +- gcc/testsuite/gfortran.dg/pr78061.f | 2 +- gcc/testsuite/gfortran.dg/pr79315.f90 | 6 +- gcc/testsuite/gfortran.dg/spec_expr_8.f90 | 24 ++ gcc/testsuite/gfortran.dg/spec_expr_9.f90 | 19 + gcc/testsuite/gfortran.dg/vect/pr90681.f | 2 +- gcc/testsuite/gfortran.dg/vect/pr97761.f90| 2 +- gcc/testsuite/gfortran.dg/vect/pr99746.f90| 2 +- 14 files changed, 151 insertions(+), 58 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_8.f90 create mode 100644 gcc/testsuite/gfortran.dg/spec_expr_9.f90 -- 2.43.0
Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
Le 15/03/2024 à 18:26, Harald Anlauf a écrit : Hi Mikael, On 3/15/24 17:31, Mikael Morin wrote: Le 10/03/2024 à 22:31, Harald Anlauf a écrit : Dear all, after playing for some time with NAG and Intel, and an off-list discussion with Jerry, I am getting more and more convinced that simpler runtime error messages (also simpler to parse by a human) are superior to awkward solutions. This is also what Intel does: use only the name of the array (component) in the message whose indices are out of bounds. (NAG's solution appears also inconsistent for nested derived types.) So no x%z, or x%_data, etc. in runtime error messages any more. That's a pity. What about providing the root variable and the failing component only? ... dimension 1 of array component 'z...%x' above array bound ... The data reference doesn't look great, but it provides valuable (in my opinion) information. OK, that sounds interesting. To clarify the options: - for ordinary array x it would stay 'x' - when z is a DT scalar, and z%x is the array in question, use 'z%x' (here z...%x would look strange to me) Yes, the ellipsis would look strange to me as well. - when z is a DT array, and x some component further down, 'z...%x' This case also applies when z is a DT scalar and x is more than one level deep. I would rather not make the error message text vary too much to avoid to run into issues with translation. Would it be fine with you to have ... dimension 1 of array 'z...%x' above array bound ... only? OK, let's drop "component". Anything else? No, I think you covered everything. Cheers, Harald Please give it a spin... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald On 1/30/24 11:46, Mikael Morin wrote: Le 30/01/2024 à 11:38, Mikael Morin a écrit : Another (easier) way to clarify the data reference would be rephrasing the message so that the array part is separate from the scalar part, like so (there are too many 'of', but I lack inspiration): Index '0' of dimension 1 of component 'zz' of element from 'x1%vv' below lower bound of 1 This has the same number of 'of' but sounds better maybe: Out of bounds accessing component 'zz' of element from 'x1%yy': index '0' of dimension 1 below lower bound of 1
Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
Le 10/03/2024 à 22:31, Harald Anlauf a écrit : Dear all, after playing for some time with NAG and Intel, and an off-list discussion with Jerry, I am getting more and more convinced that simpler runtime error messages (also simpler to parse by a human) are superior to awkward solutions. This is also what Intel does: use only the name of the array (component) in the message whose indices are out of bounds. (NAG's solution appears also inconsistent for nested derived types.) So no x%z, or x%_data, etc. in runtime error messages any more. That's a pity. What about providing the root variable and the failing component only? ... dimension 1 of array component 'z...%x' above array bound ... The data reference doesn't look great, but it provides valuable (in my opinion) information. Please give it a spin... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald On 1/30/24 11:46, Mikael Morin wrote: Le 30/01/2024 à 11:38, Mikael Morin a écrit : Another (easier) way to clarify the data reference would be rephrasing the message so that the array part is separate from the scalar part, like so (there are too many 'of', but I lack inspiration): Index '0' of dimension 1 of component 'zz' of element from 'x1%vv' below lower bound of 1 This has the same number of 'of' but sounds better maybe: Out of bounds accessing component 'zz' of element from 'x1%yy': index '0' of dimension 1 below lower bound of 1
Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
Le 30/01/2024 à 11:38, Mikael Morin a écrit : Another (easier) way to clarify the data reference would be rephrasing the message so that the array part is separate from the scalar part, like so (there are too many 'of', but I lack inspiration): Index '0' of dimension 1 of component 'zz' of element from 'x1%vv' below lower bound of 1 This has the same number of 'of' but sounds better maybe: Out of bounds accessing component 'zz' of element from 'x1%yy': index '0' of dimension 1 below lower bound of 1
Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
Le 29/01/2024 à 21:50, Harald Anlauf a écrit :
Am 29.01.24 um 18:25 schrieb Harald Anlauf:
I was talking about the generated format strings of runtime error
messages.
program p
implicit none
type t
real :: zzz(10) = 42
end type t
class(t), allocatable :: xx(:)
integer :: j
j = 0
allocate (t :: xx(1))
print *, xx(1)% zzz(j)
end
This is generating the following error at runtime since at least gcc-7:
Fortran runtime error: Index '0' of dimension 1 of array 'xx%_data%zzz'
below lower bound of 1
Of course this is easily suppressed by:
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 1e0d698a949..fa0e00a28a6 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -4054,7 +4054,8 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref *
ar, gfc_expr *expr,
{
if (ref->type == REF_ARRAY && >u.ar == ar)
break;
- if (ref->type == REF_COMPONENT)
+ if (ref->type == REF_COMPONENT
+ && strcmp (ref->u.c.component->name, "_data") != 0)
{
strcat (var_name, "%%");
strcat (var_name, ref->u.c.component->name);
I have been contemplating the generation the full chain of references as
suggested by Mikael and supported by NAG.
To be clear, my suggestion was to have the question marks (or dashes,
dots, stars, whatever) literally in the array reference, without the
actual values of the array indexes.
Another (easier) way to clarify the data reference would be rephrasing
the message so that the array part is separate from the scalar part,
like so (there are too many 'of', but I lack inspiration):
Index '0' of dimension 1 of component 'zz' of element from 'x1%vv'
below lower bound of 1
The main issue is: how do I easily generate that call?
gfc_trans_runtime_check is a vararg function, but what I would rather
have is a function that takes either a (chained?) list of trees or
an array of trees holding the (co-)indices of the reference.
Is there an example, or a recommendation which variant to prefer?
None that I know.
For a scalarized expression, the values are present (among others) in
the gfc_loopinfo::ss linked list, maybe just use that?
In any case, I agree it would be nice to have, but it would probably be
a non-negligible amount of new error-prone code; I would rather not
attempt this during the stage4 stabilization phase as we are currently.
Re: [PATCH] Fortran: NULL actual to optional dummy with VALUE attribute [PR113377]
Le 25/01/2024 à 22:26, Harald Anlauf a écrit : Dear all, this is the third patch in a series that addresses dummy arguments with the VALUE attribute, now handling the passing of NULL actual arguments. It is based on the refactoring in the previous patch and reuses the handling of absent arguments. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald OK, thanks.
Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
Le 24/01/2024 à 22:39, Harald Anlauf a écrit : Dear all, this patch is actually only a followup fix to generate the proper name of an array reference in derived-type components for the runtime error message generated for the bounds-checking code. Without the proper part ref, not only a user may get confused: I was, too... The testcase is compile-only, as it is only important to check the strings used in the error messages. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald Hello, the change proper looks good, and is an improvement. But I'm a little concerned by the production of references like in the test x1%vv%z which could be confusing and is strictly speaking invalid fortran (multiple non-scalar components). Did you consider generating x1%vv(?,?)%zz or x1%vv(...)%z or similar? There is another nit about the test, which has dg-output and dg-shouldfail despite being only a compile-time test. Otherwise looks good. Mikael
Re: [PATCH] Fortran: passing of optional dummies to elemental procedures [PR113377]
Le 23/01/2024 à 21:36, Harald Anlauf a écrit :
Dear all,
here's the second part of a series for the treatment of missing
optional arguments passed to optional dummies, now fixing the
case that the latter procedures are elemental. Adjustments
were necessary when the missing dummy has the VALUE attribute.
I factored the code for the treatment of VALUE, hoping that the
monster loop in gfc_conv_procedure_call will become slightly
easier to overlook.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Looks good, but...
Thanks,
Harald
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 128add47516..0fac0523670 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6392,12 +6479,23 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
}
}
+ /* Scalar dummy arguments of intrinsic type with VALUE attribute. */
+ if (fsym
+ && fsym->attr.value
+ && !fsym->attr.dimension
+ // && (fsym->ts.type != BT_CHARACTER
+ // || gfc_length_one_character_type_p (>ts))
... please remove the commented code here. OK with that change.
The !fsym->attr.dimension condition could be removed as well as we are
in the case of an elemental procedure at this point, but it doesn't harm
if you prefer keeping it.
Thanks for the patch.
Mikael
+ && fsym->ts.type != BT_DERIVED
+ && fsym->ts.type != BT_CLASS)
+ conv_dummy_value (, e, fsym, optionalargs);
+
/* If we are passing an absent array as optional dummy to an
elemental procedure, make sure that we pass NULL when the data
pointer is NULL. We need this extra conditional because of
scalarization which passes arrays elements to the procedure,
ignoring the fact that the array can be absent/unallocated/... */
- if (ss->info->can_be_null_ref && ss->info->type != GFC_SS_REFERENCE)
+ else if (ss->info->can_be_null_ref
+ && ss->info->type != GFC_SS_REFERENCE)
{
tree descriptor_data;
Re: [PATCH] Fortran: passing of optional scalar arguments with VALUE attribute [PR113377]
Hello,
Le 20/01/2024 à 22:58, Harald Anlauf a écrit :
Dear all,
here's the first part of an attempt to fix issues with optional
dummy arguments as actual arguments to optional dummies. This patch
rectifies the case of scalar dummies with the VALUE attribute,
which in gfortran's argument passing convention are passed on the
stack when they are of intrinsic type, and have a hidden variable
for the presence status.
The testcase tries to cover valid combinations of actual and dummy
argument. A few tests that are not standard-conforming but would
still work with gfortran (due to the argument passing convention)
are left there but commented out with a pointer to the standard
(thanks, Mikael!).
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Well, not yet.
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 9dd1f4086f4..2f47a75955c 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6526,6 +6526,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
gfc_init_se (, NULL);
argse.want_pointer = 1;
gfc_conv_expr (, e);
+ if (e->symtree->n.sym->attr.dummy
+ && POINTER_TYPE_P (TREE_TYPE (argse.expr)))
+ argse.expr = gfc_build_addr_expr (NULL_TREE,
+ argse.expr);
The second part of the condition looks superfluous: if
argse.want_pointer was set, we can expect to get a pointer result.
But more important, I don't understand the need for this whole part, the
new test seems to pass without it.
And here is an example that regresses with it.
program p
type :: t
integer, allocatable :: c
end type
call s2(t())
contains
subroutine s1(a)
integer, value, optional :: a
if (present(a)) stop 1
end subroutine
subroutine s2(a)
type(t) :: a
call s1(a%c)
end subroutine
end program
cond = fold_convert (TREE_TYPE (argse.expr),
null_pointer_node);
cond = fold_build2_loc (input_location, NE_EXPR,
@@ -7256,6 +7260,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
&& e->symtree->n.sym->attr.optional
&& (((e->rank != 0 && elemental_proc)
|| e->representation.length || e->ts.type == BT_CHARACTER
+ || (e->rank == 0 && e->symtree->n.sym->attr.value)
This looks good.
|| (e->rank != 0
&& (fsym == NULL
|| (fsym->as
diff --git a/gcc/testsuite/gfortran.dg/optional_absent_9.f90
b/gcc/testsuite/gfortran.dg/optional_absent_9.f90
new file mode 100644
index 000..495a6c00d7f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/optional_absent_9.f90
@@ -0,0 +1,324 @@
+! { dg-do run }
+! PR fortran/113377
+!
+! Test passing of missing optional scalar dummies of intrinsic type
+
+module m_int
+ implicit none
+contains
+ subroutine test_int ()
+integer :: k = 1
+call one (k)
+call one_val (k)
+call one_all (k)
+call one_ptr (k)
+ end
+
+ subroutine one (i, j)
+integer, intent(in) :: i
+integer ,optional :: j
+integer, allocatable :: aa
+integer, pointer :: pp => NULL()
+if (present (j)) error stop "j is present"
+call two (i, j)
+call two_val (i, j)
+call two (i, aa)
+call two (i, pp)
To be complete, you could check two_val(i, aa) and two_val(i, pp) as well.
Both seem to pass already without the patch, so not absolutely needed.
Your call.
+ end
+
I think the patch is OK with the first trans-expr.cc hunk removed.
Thanks.
Mikael
[PATCH] fortran: Restore current interface info on error [PR111291]
Hello,
I tested this on x86_64-pc-linux-gnu without regression.
There is no new test, as the problem is visible on an
existing test with valgrind or an asan-instrumented compiler.
OK for master?
-- >8 --
This change is a followup to the fix for PR48776 (namely
r14-3572-gd58150452976c4ca65ddc811fac78ef956fa96b0 AKA
fortran: Restore interface to its previous state on error [PR48776]),
which cleaned up new changes from interfaces upon error.
Unfortunately, there is one case in that fix that is mishandled, visible
on unexpected_interface.f90 with valgrind or an asan-instrumented gfortran.
when an interface statement is found while parsing an interface body (which
is invalid), the current interface is replaced by the one from the new
statement, and as parsing continues, new procedures are added
to the new interface, which has been rejected and freed, instead of the
original one.
This change restores the current interface pointer to its previous value
on each rejected statement.
PR fortran/48776
PR fortran/111291
gcc/fortran/ChangeLog:
* parse.cc: Restore current interface to its previous value on error.
---
gcc/fortran/parse.cc | 1 +
1 file changed, 1 insertion(+)
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index abd3a424f38..51e89e10e2d 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -4033,6 +4033,7 @@ loop:
default:
gfc_error ("Unexpected %s statement in INTERFACE block at %C",
gfc_ascii_statement (st));
+ current_interface = save;
reject_statement ();
gfc_free_namespace (gfc_current_ns);
goto loop;
--
2.43.0
Re: [PATCH] Fortran: copy-out for possibly missing OPTIONAL CLASS arguments [PR112772]
Hello, Le 30/11/2023 à 22:06, Harald Anlauf a écrit : the attached rather obvious patch fixes the first testcase of pr112772: we unconditionally generated copy-out code for optional class arguments, while the copy-in properly checked the presence of arguments. Regtested on x86_64-pc-linux-gnu. OK for mainline? Looks good. Thanks. (The second testcase is a different issue.) Maybe use a separate PR? Mikael
Re: [PATCH] Fortran: avoid obsolescence warning for COMMON with submodule [PR111880]
Hello, Le 23/11/2023 à 22:59, Harald Anlauf a écrit : Dear all, the PR is about a redundant obsolescence warning for COMMON when a symbols appears in the scope of a submodule. As we did not warn for use-associated symbols, it seemed natural to extend this to symbols that are used in a submodule. Or am I missing anything? Regtests cleanly on x86_64-pc-linux-gnu. OK for mainline? Yes, looks good. Obvious I'm tempted to say. The PR is marked as a regression (the warning appeared in gcc-9). It looks simple enough for backporting, or does anybody see any risk here? No, you can go ahead. Thanks Mikael
Re: [PATCH, testsuite, fortran] fix invalid testcases (missing MOLD argument to NULL)
Hello,
Le 22/11/2023 à 22:02, Harald Anlauf a écrit :
Dear all,
testcases assumed_rank_8.f90 and assumed_rank_10.f90 are invalid:
NULL() is passed without MOLD to an assumed-rank dummy argument.
This is detected by NAG, but not yet by gfortran (see pr104819).
gfortran even ignores the MOLD argument; the dump-tree is identical
if MOLD is there or not.
Now these testcases are { dg-do run }. Therefore I would like to
fix these testcases, independent of the work on fixing pr104819.
Comments?
Makes sense; OK from my point of view.
Mikael
Re: [PATCH, v4] Fortran: restrictions on integer arguments to SYSTEM_CLOCK [PR112609]
Le 22/11/2023 à 21:36, Harald Anlauf a écrit :
Hi Mikael!
On 11/22/23 10:36, Mikael Morin wrote:
(...)
diff --git a/gcc/fortran/error.cc b/gcc/fortran/error.cc
index 2ac51e95e4d..be715b50469 100644
--- a/gcc/fortran/error.cc
+++ b/gcc/fortran/error.cc
@@ -980,7 +980,11 @@ char const*
notify_std_msg(int std)
{
- if (std & GFC_STD_F2018_DEL)
+ if (std & GFC_STD_F2023_DEL)
+ return _("Fortran 2023 deleted feature:");
As there are officially no deleted feature in f2023, maybe use a
slightly different wording? Say "Not allowed in fortran 2023" or
"forbidden in Fortran 2023" or similar?
+ else if (std & GFC_STD_F2023)
+ return _("Fortran 2023:");
+ else if (std & GFC_STD_F2018_DEL)
return _("Fortran 2018 deleted feature:");
else if (std & GFC_STD_F2018_OBS)
return _("Fortran 2018 obsolescent feature:");
I skimmed over existing error messages, and since "forbidden" did
not show up and since "Not allowed" exists but not at the beginning
of a message, I found that
"Prohibited in Fortran 2023"
appeared to be a good alternative.
Not being a native speaker, I hope that someone speaks up if this
is not appropriate. And since I do not explicitly verify that part
in the testcase, it can be changed.
diff --git a/gcc/fortran/libgfortran.h b/gcc/fortran/libgfortran.h
index bdddb317ab0..af7a170c2b1 100644
--- a/gcc/fortran/libgfortran.h
+++ b/gcc/fortran/libgfortran.h
@@ -19,9 +19,10 @@ along with GCC; see the file COPYING3. If not see
/* Flags to specify which standard/extension contains a feature.
- Note that no features were obsoleted nor deleted in F2003 nor in
F2023.
+ Note that no features were obsoleted nor deleted in F2003.
I think we can add a comment that F2023 has no deleted feature, but some
more stringent restrictions in f2023 forbid some previously valid code.
Please remember to keep those definitions in sync with
gfortran.texi. */
+#define GFC_STD_F2023_DEL (1<<13) /* Deleted in F2023. */
#define GFC_STD_F2023 (1<<12) /* New in F2023. */
#define GFC_STD_F2018_DEL (1<<11) /* Deleted in F2018. */
#define GFC_STD_F2018_OBS (1<<10) /* Obsolescent in F2018. */
@@ -41,12 +42,13 @@ along with GCC; see the file COPYING3. If not see
* are allowed with a certain -std option. */
#define GFC_STD_OPT_F95 (GFC_STD_F77 | GFC_STD_F95 |
GFC_STD_F95_OBS \
| GFC_STD_F2008_OBS | GFC_STD_F2018_OBS \
- | GFC_STD_F2018_DEL)
+ | GFC_STD_F2018_DEL | GFC_STD_F2023_DEL)
#define GFC_STD_OPT_F03 (GFC_STD_OPT_F95 | GFC_STD_F2003)
#define GFC_STD_OPT_F08 (GFC_STD_OPT_F03 | GFC_STD_F2008)
#define GFC_STD_OPT_F18 ((GFC_STD_OPT_F08 | GFC_STD_F2018) \
& (~GFC_STD_F2018_DEL))
F03, F08 and F18 should have GFC_STD_F2023_DEL (and also F03 and F08
should have GFC_STD_F2018_DEL).
Well, these macros do an incremental bitwise-or, so the bit representing
GFC_STD_F2023_DEL is included everywhere. I also ran the testcases with
different -std= options to check.
Ah, yes. I confused the GFC_STD_OPT* values with the GFC_STD_* ones.
OK with this fixed (and the previous comments as you wish), if Steve has
no more comments.
Thanks for the patch.
If there are no further comments, I will commit once I am able to
fully build again with --disable-bootstrap and -march=native ...
Thanks,
Harald
Thanks again.
Re: [PATCH, v3] Fortran: restrictions on integer arguments to SYSTEM_CLOCK [PR112609]
Le 21/11/2023 à 23:09, Harald Anlauf a écrit :
Uhh, it happened again. Attached a wrong patch.
Only looked at the -v3 ... My bad.
Sorry!
Harald
On 11/21/23 22:54, Harald Anlauf wrote:
Hi Mikael, Steve,
On 11/21/23 12:33, Mikael Morin wrote:
Harald, you mentioned the lack of GFC_STD_F2023_DEL feature group in
your first message, but I don't quite understand why you didn't add one.
It seems to me the most natural way to do this.
thanks for insisting on this variant.
In my first attack at this problem, I overlooked one place in
libgfortran.h, which I now was able to find and adjust.
Now everything falls into place.
I suggest we emit a warning by default, error with -std=f2023 (I agree
with Steve that we should push towards strict f2023 conformance), and no
diagnostic with -std=gnu or -std=f2018 or lower.
As the majority agrees on this, I accept it. The attached patch
now does this and fixes the testcases accordingly.
It seems that the solution is to fix the code in the testsuite.
Agreed, these seem to explicitly test mismatching kinds, so add an
option to prevent error.
Done.
I also fixed a few issues in the documentation in gfortran.texi .
As I currently cannot build a full compiler (see PR112643),
patch V3 is not properly regtested yet, but appears to give
results as discussed.
Comments?
Mikael
Thanks,
Harald
(...)
diff --git a/gcc/fortran/error.cc b/gcc/fortran/error.cc
index 2ac51e95e4d..be715b50469 100644
--- a/gcc/fortran/error.cc
+++ b/gcc/fortran/error.cc
@@ -980,7 +980,11 @@ char const*
notify_std_msg(int std)
{
- if (std & GFC_STD_F2018_DEL)
+ if (std & GFC_STD_F2023_DEL)
+return _("Fortran 2023 deleted feature:");
As there are officially no deleted feature in f2023, maybe use a
slightly different wording? Say "Not allowed in fortran 2023" or
"forbidden in Fortran 2023" or similar?
+ else if (std & GFC_STD_F2023)
+return _("Fortran 2023:");
+ else if (std & GFC_STD_F2018_DEL)
return _("Fortran 2018 deleted feature:");
else if (std & GFC_STD_F2018_OBS)
return _("Fortran 2018 obsolescent feature:");
diff --git a/gcc/fortran/libgfortran.h b/gcc/fortran/libgfortran.h
index bdddb317ab0..af7a170c2b1 100644
--- a/gcc/fortran/libgfortran.h
+++ b/gcc/fortran/libgfortran.h
@@ -19,9 +19,10 @@ along with GCC; see the file COPYING3. If not see
/* Flags to specify which standard/extension contains a feature.
- Note that no features were obsoleted nor deleted in F2003 nor in F2023.
+ Note that no features were obsoleted nor deleted in F2003.
I think we can add a comment that F2023 has no deleted feature, but some
more stringent restrictions in f2023 forbid some previously valid code.
Please remember to keep those definitions in sync with
gfortran.texi. */
+#define GFC_STD_F2023_DEL (1<<13) /* Deleted in F2023. */
#define GFC_STD_F2023 (1<<12) /* New in F2023. */
#define GFC_STD_F2018_DEL (1<<11) /* Deleted in F2018. */
#define GFC_STD_F2018_OBS (1<<10) /* Obsolescent in F2018. */
@@ -41,12 +42,13 @@ along with GCC; see the file COPYING3. If not see
* are allowed with a certain -std option. */
#define GFC_STD_OPT_F95(GFC_STD_F77 | GFC_STD_F95 |
GFC_STD_F95_OBS \
| GFC_STD_F2008_OBS | GFC_STD_F2018_OBS \
- | GFC_STD_F2018_DEL)
+ | GFC_STD_F2018_DEL | GFC_STD_F2023_DEL)
#define GFC_STD_OPT_F03(GFC_STD_OPT_F95 | GFC_STD_F2003)
#define GFC_STD_OPT_F08(GFC_STD_OPT_F03 | GFC_STD_F2008)
#define GFC_STD_OPT_F18((GFC_STD_OPT_F08 | GFC_STD_F2018) \
& (~GFC_STD_F2018_DEL))
F03, F08 and F18 should have GFC_STD_F2023_DEL (and also F03 and F08
should have GFC_STD_F2018_DEL).
OK with this fixed (and the previous comments as you wish), if Steve has
no more comments.
Thanks for the patch.
Re: [PATCH, v2] Fortran: restrictions on integer arguments to SYSTEM_CLOCK [PR112609]
Hello,
Le 20/11/2023 à 20:02, Steve Kargl a écrit :
Harald,
Sorry about delayed response. Got side-tracked by Family this weekend.
On Sun, Nov 19, 2023 at 09:46:46PM +0100, Harald Anlauf wrote:
On 11/19/23 01:04, Steve Kargl wrote:
On Sat, Nov 18, 2023 at 11:12:55PM +0100, Harald Anlauf wrote:
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Not in its current form.
{
+ int first_int_kind = -1;
+ bool f2023 = ((gfc_option.allow_std & GFC_STD_F2023) != 0
+ && (gfc_option.allow_std & GFC_STD_GNU) == 0);
+
If you use the gfc_notify_std(), then you should not need the
above check on GFC_STD_GNU as it should include GFC_STD_F2023.
this is actually the question (and problem). For all new features,
-std=gnu shall include everything allowed by -std=f2023.
Yes.
Harald, you mentioned the lack of GFC_STD_F2023_DEL feature group in
your first message, but I don't quite understand why you didn't add one.
It seems to me the most natural way to do this.
Here we have the problem that the testcase is valid F2018 and is
silently accepted by gfortran-13 for -std=gnu and -std=f2018.
F2023 is the Fortran standard and supercedes previous Fortran standards.
If there is a conflict between the standing standard and an old standard,
then the standing standard should take precedence unless one specifically
uses, for example, -std=f2018.
After 20+ years of contributing to gfortran, I've come to believe
that the default -std= should be the current standard, and -std=gnu
should be deprecated. All GNU extensions should require an option
to active. For example,
write(*,*), 'hello'
end
gfortran12 -o z a.f90
a.f90:1:10:
1 | write(*,*), 'hello'
| 1
Warning: Legacy Extension: Comma before i/o item list at (1)
This should be an error unless the -fallow-write-stmt-comma is used.
The option would simply degrade the error to a warning. Why, you ask?
To encourage people to write standard conforming code. Unfortunately,
that horse has left the barn.
I prefer to keep it that way also for gfortran-14, and apply the
new restrictions only for -std=f2023. Do we agree on this?
I suggest we emit a warning by default, error with -std=f2023 (I agree
with Steve that we should push towards strict f2023 conformance), and no
diagnostic with -std=gnu or -std=f2018 or lower.
If gfortran wants to maintain the status quo for 14, then
it should probably remove the -std=f2023 patch and wait for
the branch to 15.
Now that should happen for -std=gnu -pedantic (-w)?
-pedantic is not a very effective option and should be ignored.
>> I have thought some more and came up with the revised attached
patch, which still has the above condition. It now marks the
diagnostics as GNU extensions beyond F2023 for -std=f2023.
The mask f2023 in the above form suppresses new warnings even
for -pedantic; one would normally use -w to suppress them.
Now if you remove the second part of the condition, we will
regress on testcases system_clock_1.f90 and system_clock_3.f90
because they would emit GNU extension warnings because the
testsuite runs with -pedantic.
It seems that the solution is to fix the code in the testsuite.
Agreed, these seem to explicitly test mismatching kinds, so add an
option to prevent error.
Mikael
Re: [Patch] Fortran: Accept -std=f2023, update line-length for Fortran 2023
Le 17/11/2023 à 12:38, Tobias Burnus a écrit :
Unless there are follow up comments, I will commit it later today.
I skimmed quickly through the patch, and noticed one typo to fix:
> diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi
> index 10387e39501..5f87b330a22 100644
> --- a/gcc/fortran/invoke.texi
> +++ b/gcc/fortran/invoke.texi
> @@ -958,8 +959,8 @@ The following example will trigger the warning.
> @item -Wampersand
> Warn about missing ampersand in continued character constants. The
> warning is given with @option{-Wampersand}, @option{-pedantic},
> -@option{-std=f95}, @option{-std=f2003}, @option{-std=f2008} and
> -@option{-std=f2018}. Note: With no ampersand given in a continued
> +@option{-std=f95}, @option{-std=f2003}, @option{-std=f2008},
@option{-std=f2018}
> +and @option{-std=f203}. Note: With no ampersand given in a continued
> character constant, GNU Fortran assumes continuation at the first
> non-comment, non-whitespace character after the ampersand that
> initiated the continuation.
s/f203/f2023/
Re: [PATCH v2 0/3] libgfortran: empty array fixes
Le 07/11/2023 à 19:16, Harald Anlauf a écrit : Hi Mikael, this is OK. Thanks for the patches! Harald Patches pushed. Thanks for the (fruitful) review.
[PATCH v2 2/3] libgfortran: Remove early return if extent is zero [PR112371]
Remove the early return present in function templates for transformational functions doing a (masked) reduction of an array along a dimension. This early return, which triggered if the extent in the reduction dimension was zero, was wrong because even if the reduction operation degenerates to a constant value in that case, one has to loop anyway along the other dimensions to initialize every element of the resulting array with that constant value. The case of negative extent (not sure whether it may happen in practice) which was also early returning, is handled by clamping to zero. The offending piece of code was present in several places, and this removes them all. Namely, the impacted m4 files are ifunction.m4 for regular functions and types, ifunction-s.m4 for character minloc and maxloc, and ifunction-s2.m4 for character minval and maxval. PR fortran/112371 libgfortran/ChangeLog: * m4/ifunction.m4 (START_MASKED_ARRAY_FUNCTION): Remove early return if extent is zero or less, and clamp negative value to zero. * m4/ifunction-s.m4 (START_MASKED_ARRAY_FUNCTION): Ditto. * m4/ifunction-s2.m4 (START_MASKED_ARRAY_FUNCTION): Ditto. * generated/iall_i1.c: Regenerate. * generated/iall_i16.c: Regenerate. * generated/iall_i2.c: Regenerate. * generated/iall_i4.c: Regenerate. * generated/iall_i8.c: Regenerate. * generated/iany_i1.c: Regenerate. * generated/iany_i16.c: Regenerate. * generated/iany_i2.c: Regenerate. * generated/iany_i4.c: Regenerate. * generated/iany_i8.c: Regenerate. * generated/iparity_i1.c: Regenerate. * generated/iparity_i16.c: Regenerate. * generated/iparity_i2.c: Regenerate. * generated/iparity_i4.c: Regenerate. * generated/iparity_i8.c: Regenerate. * generated/maxloc1_16_i1.c: Regenerate. * generated/maxloc1_16_i16.c: Regenerate. * generated/maxloc1_16_i2.c: Regenerate. * generated/maxloc1_16_i4.c: Regenerate. * generated/maxloc1_16_i8.c: Regenerate. * generated/maxloc1_16_r10.c: Regenerate. * generated/maxloc1_16_r16.c: Regenerate. * generated/maxloc1_16_r17.c: Regenerate. * generated/maxloc1_16_r4.c: Regenerate. * generated/maxloc1_16_r8.c: Regenerate. * generated/maxloc1_16_s1.c: Regenerate. * generated/maxloc1_16_s4.c: Regenerate. * generated/maxloc1_4_i1.c: Regenerate. * generated/maxloc1_4_i16.c: Regenerate. * generated/maxloc1_4_i2.c: Regenerate. * generated/maxloc1_4_i4.c: Regenerate. * generated/maxloc1_4_i8.c: Regenerate. * generated/maxloc1_4_r10.c: Regenerate. * generated/maxloc1_4_r16.c: Regenerate. * generated/maxloc1_4_r17.c: Regenerate. * generated/maxloc1_4_r4.c: Regenerate. * generated/maxloc1_4_r8.c: Regenerate. * generated/maxloc1_4_s1.c: Regenerate. * generated/maxloc1_4_s4.c: Regenerate. * generated/maxloc1_8_i1.c: Regenerate. * generated/maxloc1_8_i16.c: Regenerate. * generated/maxloc1_8_i2.c: Regenerate. * generated/maxloc1_8_i4.c: Regenerate. * generated/maxloc1_8_i8.c: Regenerate. * generated/maxloc1_8_r10.c: Regenerate. * generated/maxloc1_8_r16.c: Regenerate. * generated/maxloc1_8_r17.c: Regenerate. * generated/maxloc1_8_r4.c: Regenerate. * generated/maxloc1_8_r8.c: Regenerate. * generated/maxloc1_8_s1.c: Regenerate. * generated/maxloc1_8_s4.c: Regenerate. * generated/maxval1_s1.c: Regenerate. * generated/maxval1_s4.c: Regenerate. * generated/maxval_i1.c: Regenerate. * generated/maxval_i16.c: Regenerate. * generated/maxval_i2.c: Regenerate. * generated/maxval_i4.c: Regenerate. * generated/maxval_i8.c: Regenerate. * generated/maxval_r10.c: Regenerate. * generated/maxval_r16.c: Regenerate. * generated/maxval_r17.c: Regenerate. * generated/maxval_r4.c: Regenerate. * generated/maxval_r8.c: Regenerate. * generated/minloc1_16_i1.c: Regenerate. * generated/minloc1_16_i16.c: Regenerate. * generated/minloc1_16_i2.c: Regenerate. * generated/minloc1_16_i4.c: Regenerate. * generated/minloc1_16_i8.c: Regenerate. * generated/minloc1_16_r10.c: Regenerate. * generated/minloc1_16_r16.c: Regenerate. * generated/minloc1_16_r17.c: Regenerate. * generated/minloc1_16_r4.c: Regenerate. * generated/minloc1_16_r8.c: Regenerate. * generated/minloc1_16_s1.c: Regenerate. * generated/minloc1_16_s4.c: Regenerate. * generated/minloc1_4_i1.c: Regenerate. * generated/minloc1_4_i16.c: Regenerate. * generated/minloc1_4_i2.c: Regenerate. * generated/minloc1_4_i4.c: Regenerate. * generated/minloc1_4_i8.c: Regenerate. *
[PATCH v2 0/3] libgfortran: empty array fixes
Hello, Harald's review of the previous version [1] of these patches spotted a possible misbehaving case in one patch, and a latent bug in the area of the second patch. So here is the second try, bootstraped and regression tested on x86_64-pc-linux-gnu. OK for master? Mikael [1]: https://gcc.gnu.org/pipermail/fortran/2023-November/059896.html https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635305.html Changes from version 1: * Add patch 1/3 to the series fixing the unallocated empty result issue. * In patch 2/3 (formerly 1/2) clamp negative extent to zero. Mikael Morin (3): libgfortran: Don't skip allocation if size is zero [PR112412] libgfortran: Remove early return if extent is zero [PR112371] libgfortran: Remove empty array descriptor first dimension overwrite [PR112371] gcc/testsuite/gfortran.dg/allocated_4.f90 | 195 +++ gcc/testsuite/gfortran.dg/bound_10.f90| 207 gcc/testsuite/gfortran.dg/bound_11.f90| 588 ++ libgfortran/generated/all_l1.c| 9 +- libgfortran/generated/all_l16.c | 9 +- libgfortran/generated/all_l2.c| 9 +- libgfortran/generated/all_l4.c| 9 +- libgfortran/generated/all_l8.c| 9 +- libgfortran/generated/any_l1.c| 9 +- libgfortran/generated/any_l16.c | 9 +- libgfortran/generated/any_l2.c| 9 +- libgfortran/generated/any_l4.c| 9 +- libgfortran/generated/any_l8.c| 9 +- libgfortran/generated/count_16_l.c| 9 +- libgfortran/generated/count_1_l.c | 9 +- libgfortran/generated/count_2_l.c | 9 +- libgfortran/generated/count_4_l.c | 9 +- libgfortran/generated/count_8_l.c | 9 +- libgfortran/generated/findloc1_c10.c | 18 +- libgfortran/generated/findloc1_c16.c | 18 +- libgfortran/generated/findloc1_c17.c | 18 +- libgfortran/generated/findloc1_c4.c | 18 +- libgfortran/generated/findloc1_c8.c | 18 +- libgfortran/generated/findloc1_i1.c | 18 +- libgfortran/generated/findloc1_i16.c | 18 +- libgfortran/generated/findloc1_i2.c | 18 +- libgfortran/generated/findloc1_i4.c | 18 +- libgfortran/generated/findloc1_i8.c | 18 +- libgfortran/generated/findloc1_r10.c | 18 +- libgfortran/generated/findloc1_r16.c | 18 +- libgfortran/generated/findloc1_r17.c | 18 +- libgfortran/generated/findloc1_r4.c | 18 +- libgfortran/generated/findloc1_r8.c | 18 +- libgfortran/generated/findloc1_s1.c | 18 +- libgfortran/generated/findloc1_s4.c | 18 +- libgfortran/generated/iall_i1.c | 30 +- libgfortran/generated/iall_i16.c | 30 +- libgfortran/generated/iall_i2.c | 30 +- libgfortran/generated/iall_i4.c | 30 +- libgfortran/generated/iall_i8.c | 30 +- libgfortran/generated/iany_i1.c | 30 +- libgfortran/generated/iany_i16.c | 30 +- libgfortran/generated/iany_i2.c | 30 +- libgfortran/generated/iany_i4.c | 30 +- libgfortran/generated/iany_i8.c | 30 +- libgfortran/generated/iparity_i1.c| 30 +- libgfortran/generated/iparity_i16.c | 30 +- libgfortran/generated/iparity_i2.c| 30 +- libgfortran/generated/iparity_i4.c| 30 +- libgfortran/generated/iparity_i8.c| 30 +- libgfortran/generated/maxloc1_16_i1.c | 30 +- libgfortran/generated/maxloc1_16_i16.c| 30 +- libgfortran/generated/maxloc1_16_i2.c | 30 +- libgfortran/generated/maxloc1_16_i4.c | 30 +- libgfortran/generated/maxloc1_16_i8.c | 30 +- libgfortran/generated/maxloc1_16_r10.c| 30 +- libgfortran/generated/maxloc1_16_r16.c| 30 +- libgfortran/generated/maxloc1_16_r17.c| 30 +- libgfortran/generated/maxloc1_16_r4.c | 30 +- libgfortran/generated/maxloc1_16_r8.c | 30 +- libgfortran/generated/maxloc1_16_s1.c | 30 +- libgfortran/generated/maxloc1_16_s4.c | 30 +- libgfortran/generated/maxloc1_4_i1.c | 30 +- libgfortran/generated/maxloc1_4_i16.c | 30 +- libgfortran/generated/maxloc1_4_i2.c | 30 +- libgfortran/generated/maxloc1_4_i4.c | 30 +- libgfortran/generated/maxloc1_4_i8.c | 30 +- libgfortran/generated/maxloc1_4_r10.c | 30 +- libgfortran/generated/maxloc1_4_r16.c | 30 +- libgfortran/generated/maxloc1_4_r17.c | 30 +- libgfortran/generated/maxloc1_4_r4.c | 30 +- libgfortran/generated/maxloc1_4_r8.c | 30 +- libgfortran/generated/maxloc1_4_s1.c | 30 +- libgfortran/generated/maxloc1_4_s4.c | 30 +- libgfortran/generated/maxloc1_8_i1.c | 30 +- libgfortran/generated/maxloc1_8_i16.c | 30 +- libgfortran/generated/maxloc1_8_i2.c | 30 +- libgfortran/generated/maxloc1_8_i4.c | 30 +- libgfortran/generated/maxloc1_8_i8.c | 30 +- libgfortran/generated/maxloc1_8_r10.c
Re: [PATCH 2/2] libgfortran: Remove empty array descriptor first dimension overwrite [PR112371]
Le 06/11/2023 à 19:20, Harald Anlauf a écrit :
Hi Mikael,
Am 06.11.23 um 12:43 schrieb Mikael Morin:
Remove the forced overwrite of the first dimension of the result array
descriptor to set it to zero extent, in the function templates for
transformational functions doing an array reduction along a
dimension. This
overwrite, which happened before early returning in case the result array
was empty, was wrong because an array may have a non-zero extent in the
first dimension and still be empty if it has a zero extent in a higher
dimension. Overwriting the dimension was resulting in wrong array result
upper bound for the first dimension in that case.
The offending piece of code was present in several places, and this
removes
them all. More precisely, there is only one case to fix for logical
reduction functions, and there are three cases for other reduction
functions, corresponding to non-masked reduction, reduction with array
mask,
and reduction with scalar mask. The impacted m4 files are
ifunction_logical.m4 for logical reduction functions, ifunction.m4 for
regular functions and types, ifunction-s.m4 for character minloc and
maxloc,
ifunction-s2.m4 for character minval and maxval, and ifindloc1.m4 for
findloc.
while your fix seems mechanical and correct, I wonder if you looked
at the following pre-existing irregularity which can be seen in
this snippet:
diff --git a/libgfortran/m4/ifunction.m4 b/libgfortran/m4/ifunction.m4
index 480649cf691..abc15b430ab 100644
--- a/libgfortran/m4/ifunction.m4
+++ b/libgfortran/m4/ifunction.m4
@@ -96,12 +96,7 @@ name`'rtype_qual`_'atype_code` ('rtype` * const
restrict retarray,
retarray->base_addr = xmallocarray (alloc_size, sizeof
(rtype_name));
if (alloc_size == 0)
- {
- /* Make sure we have a zero-sized array. */
- GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1);
- return;
-
- }
+ return;
}
else
{
This is all enclosed in a block which has
if (retarray->base_addr == NULL)
but allocates and sets retarray->base_addr, while
@@ -290,11 +285,7 @@ m'name`'rtype_qual`_'atype_code` ('rtype` * const
restrict retarray,
retarray->dtype.rank = rank;
if (alloc_size == 0)
- {
- /* Make sure we have a zero-sized array. */
- GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1);
- return;
- }
+ return;
else
retarray->base_addr = xmallocarray (alloc_size, sizeof
(rtype_name));
and
@@ -454,11 +445,7 @@ void
alloc_size = GFC_DESCRIPTOR_STRIDE(retarray,rank-1) *
extent[rank-1];
if (alloc_size == 0)
- {
- /* Make sure we have a zero-sized array. */
- GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1);
- return;
- }
+ return;
else
retarray->base_addr = xmallocarray (alloc_size, sizeof
(rtype_name));
}
do not set retarray->base_addr to non-NULL for alloc_size == 0.
Do you know if the first snippet can be safely rewritten to avoid
the (hopefully pointless) xmallocarray for alloc_size == 0?
This change to the testcase:
diff --git a/gcc/testsuite/gfortran.dg/bound_11.f90
b/gcc/testsuite/gfortran.dg/bound_11.f90
index 170eba4ddfd..2e96f843476 100644
--- a/gcc/testsuite/gfortran.dg/bound_11.f90
+++ b/gcc/testsuite/gfortran.dg/bound_11.f90
@@ -88,6 +88,7 @@ contains
m4 = .false.
i = 1
r = sum(a, dim=i)
+if (.not. allocated(r)) stop 210
if (any(shape(r) /= (/ 3, 0, 7 /))) stop 211
if (any(ubound(r) /= (/ 3, 0, 7 /))) stop 212
i = 2
@@ -104,6 +105,7 @@ contains
if (any(ubound(r) /= (/ 9, 3, 0 /))) stop 218
i = 1
r = sum(a, dim=i, mask=m1)
+if (.not. allocated(r)) stop 220
if (any(shape(r) /= (/ 3, 0, 7 /))) stop 221
if (any(ubound(r) /= (/ 3, 0, 7 /))) stop 222
i = 2
@@ -120,6 +122,7 @@ contains
if (any(ubound(r) /= (/ 9, 3, 0 /))) stop 228
i = 1
r = sum(a, dim=i, mask=m4)
+if (.not. allocated(r)) stop 230
if (any(shape(r) /= (/ 3, 0, 7 /))) stop 231
if (any(ubound(r) /= (/ 3, 0, 7 /))) stop 232
i = 2
gives me a FAIL with STOP 220 (or STOP 230 if the STOP 220 line is
commented); the first one with STOP 210 passes.
So it is the first snippet with the xmallocarray (which supports zero
values see memory.c) call that is the correct one.
Good catch, I will open a separate PR.
Mikael
Re: [PATCH 1/2] libgfortran: Remove early return if extent is zero [PR112371]
Le 06/11/2023 à 19:12, Harald Anlauf a écrit :
Hi Mikael,
Am 06.11.23 um 12:43 schrieb Mikael Morin:
Remove the early return present in function templates for
transformational
functions doing a (masked) reduction of an array along a dimension.
This early return, which triggered if the extent in the reduction
dimension
was zero, was wrong because even if the reduction operation
degenerates to
a constant value in that case, one has to loop anyway along the other
dimensions to initialize every element of the resulting array with that
constant value.
The offending piece of code was present in several places, and this
removes
them all. Namely, the impacted m4 files are ifunction.m4 for regular
functions and types, ifunction-s.m4 for character minloc and maxloc, and
ifunction-s2.m4 for character minval and maxval.
I wonder if the correct fix would be to replace (instead of deleting)
diff --git a/libgfortran/m4/ifunction.m4 b/libgfortran/m4/ifunction.m4
index c64217ec5db..480649cf691 100644
--- a/libgfortran/m4/ifunction.m4
+++ b/libgfortran/m4/ifunction.m4
@@ -232,8 +232,6 @@ m'name`'rtype_qual`_'atype_code` ('rtype` * const
restrict retarray,
}
len = GFC_DESCRIPTOR_EXTENT(array,dim);
- if (len <= 0)
- return;
mbase = mask->base_addr;
by the following:
if (len < 0)
len = 0;
See ifunction.m4, lines 56ff, which check if the result of
len = GFC_DESCRIPTOR_EXTENT(array,dim);
is negative. I haven't tried to create a testcase, though.
Similarly for the other templates.
Yes, you're right. I think I won't try to create a testcase and will
just pick your suggestion which seems safer.
[PATCH 1/2] libgfortran: Remove early return if extent is zero [PR112371]
Remove the early return present in function templates for transformational functions doing a (masked) reduction of an array along a dimension. This early return, which triggered if the extent in the reduction dimension was zero, was wrong because even if the reduction operation degenerates to a constant value in that case, one has to loop anyway along the other dimensions to initialize every element of the resulting array with that constant value. The offending piece of code was present in several places, and this removes them all. Namely, the impacted m4 files are ifunction.m4 for regular functions and types, ifunction-s.m4 for character minloc and maxloc, and ifunction-s2.m4 for character minval and maxval. PR fortran/112371 libgfortran/ChangeLog: * m4/ifunction.m4 (START_MASKED_ARRAY_FUNCTION): Remove early return if extent is zero. * m4/ifunction-s.m4 (START_MASKED_ARRAY_FUNCTION): Ditto. * m4/ifunction-s2.m4 (START_MASKED_ARRAY_FUNCTION): Ditto. * generated/iall_i1.c: Regenerate. * generated/iall_i16.c: Regenerate. * generated/iall_i2.c: Regenerate. * generated/iall_i4.c: Regenerate. * generated/iall_i8.c: Regenerate. * generated/iany_i1.c: Regenerate. * generated/iany_i16.c: Regenerate. * generated/iany_i2.c: Regenerate. * generated/iany_i4.c: Regenerate. * generated/iany_i8.c: Regenerate. * generated/iparity_i1.c: Regenerate. * generated/iparity_i16.c: Regenerate. * generated/iparity_i2.c: Regenerate. * generated/iparity_i4.c: Regenerate. * generated/iparity_i8.c: Regenerate. * generated/maxloc1_16_i1.c: Regenerate. * generated/maxloc1_16_i16.c: Regenerate. * generated/maxloc1_16_i2.c: Regenerate. * generated/maxloc1_16_i4.c: Regenerate. * generated/maxloc1_16_i8.c: Regenerate. * generated/maxloc1_16_r10.c: Regenerate. * generated/maxloc1_16_r16.c: Regenerate. * generated/maxloc1_16_r17.c: Regenerate. * generated/maxloc1_16_r4.c: Regenerate. * generated/maxloc1_16_r8.c: Regenerate. * generated/maxloc1_16_s1.c: Regenerate. * generated/maxloc1_16_s4.c: Regenerate. * generated/maxloc1_4_i1.c: Regenerate. * generated/maxloc1_4_i16.c: Regenerate. * generated/maxloc1_4_i2.c: Regenerate. * generated/maxloc1_4_i4.c: Regenerate. * generated/maxloc1_4_i8.c: Regenerate. * generated/maxloc1_4_r10.c: Regenerate. * generated/maxloc1_4_r16.c: Regenerate. * generated/maxloc1_4_r17.c: Regenerate. * generated/maxloc1_4_r4.c: Regenerate. * generated/maxloc1_4_r8.c: Regenerate. * generated/maxloc1_4_s1.c: Regenerate. * generated/maxloc1_4_s4.c: Regenerate. * generated/maxloc1_8_i1.c: Regenerate. * generated/maxloc1_8_i16.c: Regenerate. * generated/maxloc1_8_i2.c: Regenerate. * generated/maxloc1_8_i4.c: Regenerate. * generated/maxloc1_8_i8.c: Regenerate. * generated/maxloc1_8_r10.c: Regenerate. * generated/maxloc1_8_r16.c: Regenerate. * generated/maxloc1_8_r17.c: Regenerate. * generated/maxloc1_8_r4.c: Regenerate. * generated/maxloc1_8_r8.c: Regenerate. * generated/maxloc1_8_s1.c: Regenerate. * generated/maxloc1_8_s4.c: Regenerate. * generated/maxval1_s1.c: Regenerate. * generated/maxval1_s4.c: Regenerate. * generated/maxval_i1.c: Regenerate. * generated/maxval_i16.c: Regenerate. * generated/maxval_i2.c: Regenerate. * generated/maxval_i4.c: Regenerate. * generated/maxval_i8.c: Regenerate. * generated/maxval_r10.c: Regenerate. * generated/maxval_r16.c: Regenerate. * generated/maxval_r17.c: Regenerate. * generated/maxval_r4.c: Regenerate. * generated/maxval_r8.c: Regenerate. * generated/minloc1_16_i1.c: Regenerate. * generated/minloc1_16_i16.c: Regenerate. * generated/minloc1_16_i2.c: Regenerate. * generated/minloc1_16_i4.c: Regenerate. * generated/minloc1_16_i8.c: Regenerate. * generated/minloc1_16_r10.c: Regenerate. * generated/minloc1_16_r16.c: Regenerate. * generated/minloc1_16_r17.c: Regenerate. * generated/minloc1_16_r4.c: Regenerate. * generated/minloc1_16_r8.c: Regenerate. * generated/minloc1_16_s1.c: Regenerate. * generated/minloc1_16_s4.c: Regenerate. * generated/minloc1_4_i1.c: Regenerate. * generated/minloc1_4_i16.c: Regenerate. * generated/minloc1_4_i2.c: Regenerate. * generated/minloc1_4_i4.c: Regenerate. * generated/minloc1_4_i8.c: Regenerate. * generated/minloc1_4_r10.c: Regenerate. * generated/minloc1_4_r16.c: Regenerate. * generated/minloc1_4_r17.c: Regenerate. * generated/minloc1_4_r4.c: Regenerate.
[PATCH 0/2] libgfortran: empty array fixes [PR112371]
Hello, while preparing a testcase, I encountered a bug which I filed as PR112371. Investigating further, I found two different problems which I propose to fix with the followup patches. Those have been bootstraped and regression tested on x86_64-pc-linux-gnu. OK for master? Mikael Mikael Morin (2): libgfortran: Remove early return if extent is zero [PR112371] libgfortran: Remove empty array descriptor first dimension overwrite [PR112371] gcc/testsuite/gfortran.dg/bound_10.f90 | 207 + gcc/testsuite/gfortran.dg/bound_11.f90 | 588 + libgfortran/generated/all_l1.c | 6 +- libgfortran/generated/all_l16.c| 6 +- libgfortran/generated/all_l2.c | 6 +- libgfortran/generated/all_l4.c | 6 +- libgfortran/generated/all_l8.c | 6 +- libgfortran/generated/any_l1.c | 6 +- libgfortran/generated/any_l16.c| 6 +- libgfortran/generated/any_l2.c | 6 +- libgfortran/generated/any_l4.c | 6 +- libgfortran/generated/any_l8.c | 6 +- libgfortran/generated/count_16_l.c | 6 +- libgfortran/generated/count_1_l.c | 6 +- libgfortran/generated/count_2_l.c | 6 +- libgfortran/generated/count_4_l.c | 6 +- libgfortran/generated/count_8_l.c | 6 +- libgfortran/generated/findloc1_c10.c | 18 +- libgfortran/generated/findloc1_c16.c | 18 +- libgfortran/generated/findloc1_c17.c | 18 +- libgfortran/generated/findloc1_c4.c| 18 +- libgfortran/generated/findloc1_c8.c| 18 +- libgfortran/generated/findloc1_i1.c| 18 +- libgfortran/generated/findloc1_i16.c | 18 +- libgfortran/generated/findloc1_i2.c| 18 +- libgfortran/generated/findloc1_i4.c| 18 +- libgfortran/generated/findloc1_i8.c| 18 +- libgfortran/generated/findloc1_r10.c | 18 +- libgfortran/generated/findloc1_r16.c | 18 +- libgfortran/generated/findloc1_r17.c | 18 +- libgfortran/generated/findloc1_r4.c| 18 +- libgfortran/generated/findloc1_r8.c| 18 +- libgfortran/generated/findloc1_s1.c| 18 +- libgfortran/generated/findloc1_s4.c| 18 +- libgfortran/generated/iall_i1.c| 21 +- libgfortran/generated/iall_i16.c | 21 +- libgfortran/generated/iall_i2.c| 21 +- libgfortran/generated/iall_i4.c| 21 +- libgfortran/generated/iall_i8.c| 21 +- libgfortran/generated/iany_i1.c| 21 +- libgfortran/generated/iany_i16.c | 21 +- libgfortran/generated/iany_i2.c| 21 +- libgfortran/generated/iany_i4.c| 21 +- libgfortran/generated/iany_i8.c| 21 +- libgfortran/generated/iparity_i1.c | 21 +- libgfortran/generated/iparity_i16.c| 21 +- libgfortran/generated/iparity_i2.c | 21 +- libgfortran/generated/iparity_i4.c | 21 +- libgfortran/generated/iparity_i8.c | 21 +- libgfortran/generated/maxloc1_16_i1.c | 21 +- libgfortran/generated/maxloc1_16_i16.c | 21 +- libgfortran/generated/maxloc1_16_i2.c | 21 +- libgfortran/generated/maxloc1_16_i4.c | 21 +- libgfortran/generated/maxloc1_16_i8.c | 21 +- libgfortran/generated/maxloc1_16_r10.c | 21 +- libgfortran/generated/maxloc1_16_r16.c | 21 +- libgfortran/generated/maxloc1_16_r17.c | 21 +- libgfortran/generated/maxloc1_16_r4.c | 21 +- libgfortran/generated/maxloc1_16_r8.c | 21 +- libgfortran/generated/maxloc1_16_s1.c | 21 +- libgfortran/generated/maxloc1_16_s4.c | 21 +- libgfortran/generated/maxloc1_4_i1.c | 21 +- libgfortran/generated/maxloc1_4_i16.c | 21 +- libgfortran/generated/maxloc1_4_i2.c | 21 +- libgfortran/generated/maxloc1_4_i4.c | 21 +- libgfortran/generated/maxloc1_4_i8.c | 21 +- libgfortran/generated/maxloc1_4_r10.c | 21 +- libgfortran/generated/maxloc1_4_r16.c | 21 +- libgfortran/generated/maxloc1_4_r17.c | 21 +- libgfortran/generated/maxloc1_4_r4.c | 21 +- libgfortran/generated/maxloc1_4_r8.c | 21 +- libgfortran/generated/maxloc1_4_s1.c | 21 +- libgfortran/generated/maxloc1_4_s4.c | 21 +- libgfortran/generated/maxloc1_8_i1.c | 21 +- libgfortran/generated/maxloc1_8_i16.c | 21 +- libgfortran/generated/maxloc1_8_i2.c | 21 +- libgfortran/generated/maxloc1_8_i4.c | 21 +- libgfortran/generated/maxloc1_8_i8.c | 21 +- libgfortran/generated/maxloc1_8_r10.c | 21 +- libgfortran/generated/maxloc1_8_r16.c | 21 +- libgfortran/generated/maxloc1_8_r17.c | 21 +- libgfortran/generated/maxloc1_8_r4.c | 21 +- libgfortran/generated/maxloc1_8_r8.c | 21 +- libgfortran/generated/maxloc1_8_s1.c | 21 +- libgfortran/generated/maxloc1_8_s4.c | 21 +- libgfortran/generated/maxval1_s1.c | 21 +- libgfortran/generated/maxval1_s4.c | 21 +- libgfortran/generated/maxval_i1.c | 21 +- libgfortran/generated/maxval_i16.c | 21 +- libgfortran/generated/maxval_i2.c | 21 +- libgfortran/generated/maxval_i4.c | 21 +- libgfortran/generated/maxval_i8.c | 21
Re: [PATCH] Improve tree_expr_nonnegative_p by using the ranger [PR111959]
Le 26/10/2023 à 11:29, Richard Biener a écrit :
On Wed, Oct 25, 2023 at 5:51 AM Andrew Pinski wrote:
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 40767736389..2a2a90230f5 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -15047,15 +15047,33 @@ tree_single_nonnegative_warnv_p (tree t, bool
*strict_overflow_p, int depth)
return RECURSE (TREE_OPERAND (t, 1)) && RECURSE (TREE_OPERAND (t, 2));
case SSA_NAME:
- /* Limit the depth of recursion to avoid quadratic behavior.
-This is expected to catch almost all occurrences in practice.
-If this code misses important cases that unbounded recursion
-would not, passes that need this information could be revised
-to provide it through dataflow propagation. */
- return (!name_registered_for_update_p (t)
- && depth < param_max_ssa_name_query_depth
- && gimple_stmt_nonnegative_warnv_p (SSA_NAME_DEF_STMT (t),
- strict_overflow_p, depth));
+ {
+ /* For integral types, querry the global range if possible. */
query
+ if (INTEGRAL_TYPE_P (TREE_TYPE (t)))
+ {
+ value_range vr;
+ if (get_global_range_query ()->range_of_expr (vr, t)
+ && !vr.varying_p () && !vr.undefined_p ())
+ {
+ /* If the range is nonnegative, return true. */
+ if (vr.nonnegative_p ())
+ return true;
+
+ /* If the range is non-positive, then return false. */
+ if (vr.nonpositive_p ())
+ return false;
That's testing for <= 0, nonnegative for >= 0. This means when
vr.nonpositive_p () the value could still be zero (and nonnegative),
possibly be figured out by the recursion below.
Since we don't have negative_p () do we want to test
nonpositive_p () && nonzero_p () instead?
Maybe !contains_zero_p () instead of nonzero_p () ?
nonzero_p seems to check that the range is exactly the "all but zero"
range as visible in the implementation:
inline bool
irange::nonzero_p () const
{
if (undefined_p ())
return false;
wide_int zero = wi::zero (TYPE_PRECISION (type ()));
return *this == int_range<2> (type (), zero, zero, VR_ANTI_RANGE);
}
Re: [PATCH] vec.h, v2: Make some ops work with non-trivially copy constructible and/or destructible types
Hello,
Le 27/09/2023 à 12:46, Jakub Jelinek a écrit :
--- gcc/vec.h.jj2023-09-27 10:38:50.635845540 +0200
+++ gcc/vec.h 2023-09-27 12:11:56.665586490 +0200
@@ -1028,13 +1050,17 @@ template
inline void
vec::truncate (unsigned size)
{
- gcc_checking_assert (length () >= size);
+ unsigned l = length ();
+ gcc_checking_assert (l >= size);
+ if (!std::is_trivially_destructible ::value)
+vec_destruct (address () + l, l - size);
Shouldn't this line be:
vec_destruct (address () + *size*, l - size);
instead?
m_vecpfx.m_num = size;
}
[PATCH] fortran: Remove reference count update [PR108957]
Hello, Harald reminded me recently that there was a working patch attached to the PR. I added a documentation comment with the hope that it may help avoid making the same mistake in the future. Regression tested on x86_64-pc-linux-gnu. OK for master? -- >8 -- Remove one reference count incrementation following the assignment of a symbol pointer to a local variable. Most symbol pointers are "weak" pointer and don't need any reference count update when they are assigned, and it is especially the case of local variables. This fixes a memory leak with the testcase from the PR (not included). PR fortran/108957 gcc/fortran/ChangeLog: * gfortran.h (gfc_symbol): Add comment documenting reference counting. * parse.cc (parse_interface): Remove reference count incrementation. --- gcc/fortran/gfortran.h | 20 gcc/fortran/parse.cc | 3 --- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index f4a1c106cea..6caf7765ac6 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -1944,7 +1944,27 @@ typedef struct gfc_symbol according to the Fortran standard. */ unsigned pass_as_value:1; + /* Reference counter, used for memory management. + + Some symbols may be present in more than one namespace, for example + function and subroutine symbols are present both in the outer namespace and + the procedure body namespace. Freeing symbols with the namespaces they are + in would result in double free for those symbols. This field counts + references and is used to delay the memory release until the last reference + to the symbol is removed. + + Not every symbol pointer is accounted for reference counting. Fields + gfc_symtree::n::sym are, and gfc_finalizer::proc_sym as well. But most of + them (dummy arguments, generic list elements, etc) are "weak" pointers; + the reference count isn't updated when they are assigned, and they are + ignored when the surrounding structure memory is released. This is not a + problem because there is always a namespace as surrounding context and + symbols have a name they can be referred with in that context, so the + namespace keeps the symbol from being freed, keeping the pointer valid. + When the namespace ceases to exist, and the symbols with it, the other + structures referencing symbols cease to exist as well. */ int refs; + struct gfc_namespace *ns;/* namespace containing this symbol */ tree backend_decl; diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc index 8f09ddf753c..58386805ffe 100644 --- a/gcc/fortran/parse.cc +++ b/gcc/fortran/parse.cc @@ -4064,9 +4064,6 @@ loop: accept_statement (st); prog_unit = gfc_new_block; prog_unit->formal_ns = gfc_current_ns; - if (prog_unit == prog_unit->formal_ns->proc_name - && prog_unit->ns != prog_unit->formal_ns) -prog_unit->refs++; decl: /* Read data declaration statements. */ -- 2.40.1
[PATCH] fortran: Undo new symbols in all namespaces [PR110996]
Hello,
this fixes a memory management issue in the fortran frontend.
I have included the (reduced) testcase from the PR, even if it wasn't failing
here on x86_64 with the test harness.
Tested on x86_64-pc-linux-gnu and manually checked the testcase with
valgrind.
OK for master?
-- >8 --
Remove new symbols from all namespaces they may have been added to in case a
statement mismatches in the end and all the symbols referenced in it have to
be reverted.
This fixes memory errors and random internal compiler errors caused by
a new symbol left with dangling pointers but not properly removed from the
namespace at statement rejection.
Before this change, new symbols were removed from their own namespace
(their ns field) only. This change additionally removes them from the
namespaces pointed to by respectively the gfc_current_ns global variable,
and the symbols' formal_ns field.
PR fortran/110996
gcc/fortran/ChangeLog:
* gfortran.h (gfc_release_symbol): Set return type to bool.
* symbol.cc (gfc_release_symbol): Ditto. Return whether symbol was
freed.
(delete_symbol_from_ns): New, outline code from...
(gfc_restore_last_undo_checkpoint): ... here. Delete new symbols
from two more namespaces.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr110996.f90: New test.
---
gcc/fortran/gfortran.h | 2 +-
gcc/fortran/symbol.cc | 57 --
gcc/testsuite/gfortran.dg/pr110996.f90 | 16
3 files changed, 62 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/pr110996.f90
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 371f8743312..f4a1c106cea 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3514,7 +3514,7 @@ gfc_symtree *gfc_get_unique_symtree (gfc_namespace *);
gfc_user_op *gfc_get_uop (const char *);
gfc_user_op *gfc_find_uop (const char *, gfc_namespace *);
void gfc_free_symbol (gfc_symbol *&);
-void gfc_release_symbol (gfc_symbol *&);
+bool gfc_release_symbol (gfc_symbol *&);
gfc_symbol *gfc_new_symbol (const char *, gfc_namespace *);
gfc_symtree* gfc_find_symtree_in_proc (const char *, gfc_namespace *);
int gfc_find_symbol (const char *, gfc_namespace *, int, gfc_symbol **);
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 2cba2ea0bed..a6078bc608a 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3105,13 +3105,14 @@ gfc_free_symbol (gfc_symbol *)
}
-/* Decrease the reference counter and free memory when we reach zero. */
+/* Decrease the reference counter and free memory when we reach zero.
+ Returns true if the symbol has been freed, false otherwise. */
-void
+bool
gfc_release_symbol (gfc_symbol *)
{
if (sym == NULL)
-return;
+return false;
if (sym->formal_ns != NULL && sym->refs == 2 && sym->formal_ns != sym->ns
&& (!sym->attr.entry || !sym->module))
@@ -3125,10 +3126,11 @@ gfc_release_symbol (gfc_symbol *)
sym->refs--;
if (sym->refs > 0)
-return;
+return false;
gcc_assert (sym->refs == 0);
gfc_free_symbol (sym);
+ return true;
}
@@ -3649,6 +3651,29 @@ gfc_drop_last_undo_checkpoint (void)
}
+/* Remove the reference to the symbol SYM in the symbol tree held by NS
+ and free SYM if the last reference to it has been removed.
+ Returns whether the symbol has been freed. */
+
+static bool
+delete_symbol_from_ns (gfc_symbol *sym, gfc_namespace *ns)
+{
+ if (ns == nullptr)
+return false;
+
+ /* The derived type is saved in the symtree with the first
+ letter capitalized; the all lower-case version to the
+ derived type contains its associated generic function. */
+ const char *sym_name = gfc_fl_struct (sym->attr.flavor)
+? gfc_dt_upper_string (sym->name)
+: sym->name;
+
+ gfc_delete_symtree (>sym_root, sym_name);
+
+ return gfc_release_symbol (sym);
+}
+
+
/* Undoes all the changes made to symbols since the previous checkpoint.
This subroutine is made simpler due to the fact that attributes are
never removed once added. */
@@ -3703,15 +3728,23 @@ gfc_restore_last_undo_checkpoint (void)
}
if (p->gfc_new)
{
- /* The derived type is saved in the symtree with the first
-letter capitalized; the all lower-case version to the
-derived type contains its associated generic function. */
- if (gfc_fl_struct (p->attr.flavor))
- gfc_delete_symtree (>ns->sym_root,gfc_dt_upper_string (p->name));
- else
- gfc_delete_symtree (>ns->sym_root, p->name);
+ bool freed = delete_symbol_from_ns (p, p->ns);
- gfc_release_symbol (p);
+ /* If the symbol is a procedure (function or subroutine), remove
+it from the procedure body namespace as well as from the outer
+namespace. */
+ if (!freed
+ && p->formal_ns !=
Re: [PATCH] fortran: Remove redundant tree walk to delete element
Le 08/09/2023 à 23:22, Harald Anlauf via Fortran a écrit :
Am 08.09.23 um 12:04 schrieb Mikael Morin via Gcc-patches:
Hello,
this avoids some redundant work in the symbol deletion code, which is
used a lot by the parser to cancel statements that fail to match in the
end.
I haven't tried to measure the performance effect, if any, on a
pathological example;
just passed the fortran testsuite on x86_64-pc-linux-gnu.
OK for master?
This is OK.
Thanks.
I had forgotten function comments.
This is what I have pushed.
From 1ea7130315a14ba4f66c2de76d034b33181812c5 Mon Sep 17 00:00:00 2001
From: Mikael Morin
Date: Sat, 9 Sep 2023 11:45:11 +0200
Subject: [PATCH] fortran: Remove redundant tree walk to delete element
Remove preliminary walk of the symbol tree when we are about to remove an
element. This preliminary walk was necessary because the deletion function
updated the tree without reporting back to the caller the element it had
removed. But knowing that element is necessary to free its memory, so one
had to first get that element before it was removed from the tree.
This change updates the main deletion function delete_treap and its public
wrapper gfc_delete_bbt so that the removed element can be known by the
caller. This makes the preliminary walk in gfc_delete_symtree redundant,
permitting its removal.
gcc/fortran/ChangeLog:
* bbt.cc (delete_treap): Add argument REMOVED, set it to the removed
element from the tree. Change NULL to nullptr.
(gfc_delete_bbt): Return the removed element from the tree.
* gfortran.h (gfc_delete_symtree): Remove prototype.
(gfc_delete_bbt): Set return type to pointer.
* symbol.cc (gfc_delete_symtree): Make static. Get the element to be
freed from the result of gfc_delete_bbt. Remove the preliminary walk to
get it.
---
gcc/fortran/bbt.cc | 41 +
gcc/fortran/gfortran.h | 3 +--
gcc/fortran/symbol.cc | 6 ++
3 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/gcc/fortran/bbt.cc b/gcc/fortran/bbt.cc
index 851e5e92c7b..7f1f4067fbd 100644
--- a/gcc/fortran/bbt.cc
+++ b/gcc/fortran/bbt.cc
@@ -162,37 +162,54 @@ delete_root (gfc_bbt *t)
}
-/* Delete an element from a tree. The 'old' value does not
- necessarily have to point to the element to be deleted, it must
- just point to a treap structure with the key to be deleted.
- Returns the new root node of the tree. */
+/* Delete an element from a tree, returning the new root node of the tree.
+ The OLD value does not necessarily have to point to the element to be
+ deleted, it must just point to a treap structure with the key to be deleted.
+ The REMOVED argument, if non-null, is set to the removed element from the
+ tree upon return. */
static gfc_bbt *
-delete_treap (gfc_bbt *old, gfc_bbt *t, compare_fn compare)
+delete_treap (gfc_bbt *old, gfc_bbt *t, compare_fn compare, gfc_bbt **removed)
{
int c;
- if (t == NULL)
-return NULL;
+ if (t == nullptr)
+{
+ if (removed)
+ *removed = nullptr;
+ return nullptr;
+}
c = (*compare) (old, t);
if (c < 0)
-t->left = delete_treap (old, t->left, compare);
+t->left = delete_treap (old, t->left, compare, removed);
if (c > 0)
-t->right = delete_treap (old, t->right, compare);
+t->right = delete_treap (old, t->right, compare, removed);
if (c == 0)
-t = delete_root (t);
+{
+ if (removed)
+ *removed = t;
+ t = delete_root (t);
+}
return t;
}
-void
+/* Delete the element from the tree at *ROOT that matches the OLD element
+ according to the COMPARE_FN function. This updates the *ROOT pointer to
+ point to the new tree root (if different from the original) and returns the
+ deleted element. */
+
+void *
gfc_delete_bbt (void *root, void *old, compare_fn compare)
{
gfc_bbt **t;
+ gfc_bbt *removed;
t = (gfc_bbt **) root;
- *t = delete_treap ((gfc_bbt *) old, *t, compare);
+ *t = delete_treap ((gfc_bbt *) old, *t, compare, );
+
+ return (void *) removed;
}
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index b37c6bb9ad4..371f8743312 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3510,7 +3510,6 @@ bool gfc_reference_st_label (gfc_st_label *, gfc_sl_type);
gfc_namespace *gfc_get_namespace (gfc_namespace *, int);
gfc_symtree *gfc_new_symtree (gfc_symtree **, const char *);
gfc_symtree *gfc_find_symtree (gfc_symtree *, const char *);
-void gfc_delete_symtree (gfc_symtree **, const char *);
gfc_symtree *gfc_get_unique_symtree (gfc_namespace *);
gfc_user_op *gfc_get_uop (const char *);
gfc_user_op *gfc_find_uop (const char *, gfc_namespace *);
@@ -3911,7 +3910,7 @@ bool gfc_inline_intrinsic_function_p (gfc_expr *);
/* bbt.cc */
typedef int (*compare_fn) (void *, void *);
void gfc_insert_bbt (void *, void *, compare_fn);
-void gfc_delete_bbt (void *, void *, compare_fn);
+void * gfc_delete_bbt (void *, void *, compare
[PATCH] fortran: Remove redundant tree walk to delete element
Hello,
this avoids some redundant work in the symbol deletion code, which is
used a lot by the parser to cancel statements that fail to match in the
end.
I haven't tried to measure the performance effect, if any, on a pathological
example;
just passed the fortran testsuite on x86_64-pc-linux-gnu.
OK for master?
-- >8 --
Remove preliminary walk of the symbol tree when we are about to remove an
element. This preliminary walk was necessary because the deletion function
updated the tree without reporting back to the caller the element it had
removed. But knowing that element is necessary to free its memory, so one
had to first get that element before it was removed from the tree.
This change updates the main deletion function delete_treap and its public
wrapper gfc_delete_bbt so that the removed element can be known by the
caller. This makes the preliminary walk in gfc_delete_symtree redundant,
permitting its removal.
gcc/fortran/ChangeLog:
* bbt.cc (delete_treap): Add argument REMOVED, set it to the removed
element from the tree. Change NULL to nullptr.
(gfc_delete_bbt): Return the removed element from the tree.
* gfortran.h (gfc_delete_symtree): Remove prototype.
(gfc_delete_bbt): Set return type to pointer.
* symbol.cc (gfc_delete_symtree): Make static. Get the element to be
freed from the result of gfc_delete_bbt. Remove the preliminary walk to
get it.
---
gcc/fortran/bbt.cc | 27 +++
gcc/fortran/gfortran.h | 3 +--
gcc/fortran/symbol.cc | 6 ++
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/gcc/fortran/bbt.cc b/gcc/fortran/bbt.cc
index 851e5e92c7b..2a032083c5c 100644
--- a/gcc/fortran/bbt.cc
+++ b/gcc/fortran/bbt.cc
@@ -168,31 +168,42 @@ delete_root (gfc_bbt *t)
Returns the new root node of the tree. */
static gfc_bbt *
-delete_treap (gfc_bbt *old, gfc_bbt *t, compare_fn compare)
+delete_treap (gfc_bbt *old, gfc_bbt *t, compare_fn compare, gfc_bbt **removed)
{
int c;
- if (t == NULL)
-return NULL;
+ if (t == nullptr)
+{
+ if (removed)
+ *removed = nullptr;
+ return nullptr;
+}
c = (*compare) (old, t);
if (c < 0)
-t->left = delete_treap (old, t->left, compare);
+t->left = delete_treap (old, t->left, compare, removed);
if (c > 0)
-t->right = delete_treap (old, t->right, compare);
+t->right = delete_treap (old, t->right, compare, removed);
if (c == 0)
-t = delete_root (t);
+{
+ if (removed)
+ *removed = t;
+ t = delete_root (t);
+}
return t;
}
-void
+void *
gfc_delete_bbt (void *root, void *old, compare_fn compare)
{
gfc_bbt **t;
+ gfc_bbt *removed;
t = (gfc_bbt **) root;
- *t = delete_treap ((gfc_bbt *) old, *t, compare);
+ *t = delete_treap ((gfc_bbt *) old, *t, compare, );
+
+ return (void *) removed;
}
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index b37c6bb9ad4..371f8743312 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3510,7 +3510,6 @@ bool gfc_reference_st_label (gfc_st_label *, gfc_sl_type);
gfc_namespace *gfc_get_namespace (gfc_namespace *, int);
gfc_symtree *gfc_new_symtree (gfc_symtree **, const char *);
gfc_symtree *gfc_find_symtree (gfc_symtree *, const char *);
-void gfc_delete_symtree (gfc_symtree **, const char *);
gfc_symtree *gfc_get_unique_symtree (gfc_namespace *);
gfc_user_op *gfc_get_uop (const char *);
gfc_user_op *gfc_find_uop (const char *, gfc_namespace *);
@@ -3911,7 +3910,7 @@ bool gfc_inline_intrinsic_function_p (gfc_expr *);
/* bbt.cc */
typedef int (*compare_fn) (void *, void *);
void gfc_insert_bbt (void *, void *, compare_fn);
-void gfc_delete_bbt (void *, void *, compare_fn);
+void * gfc_delete_bbt (void *, void *, compare_fn);
/* dump-parse-tree.cc */
void gfc_dump_parse_tree (gfc_namespace *, FILE *);
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index aa3cdc98c86..2cba2ea0bed 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -2948,7 +2948,7 @@ gfc_new_symtree (gfc_symtree **root, const char *name)
/* Delete a symbol from the tree. Does not free the symbol itself! */
-void
+static void
gfc_delete_symtree (gfc_symtree **root, const char *name)
{
gfc_symtree st, *st0;
@@ -2963,10 +2963,8 @@ gfc_delete_symtree (gfc_symtree **root, const char *name)
else
p = name;
- st0 = gfc_find_symtree (*root, p);
-
st.name = gfc_get_string ("%s", p);
- gfc_delete_bbt (root, , compare_symtree);
+ st0 = (gfc_symtree *) gfc_delete_bbt (root, , compare_symtree);
free (st0);
}
--
2.40.1
Re: [PATCH] Fortran: runtime bounds-checking in presence of array constructors [PR31059]
Le 01/09/2023 à 22:48, Harald Anlauf a écrit : Hi Mikael, On 9/1/23 10:41, Mikael Morin via Gcc-patches wrote: May I suggest to handle functions the same way? I'll have a look at them, but will need to gather a few suitable testcases first. I have just opened PR111339 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111339) to track the case of functions separately.
[PATCH] diagnostics: Delete config pointer before overwriting it.
Hello, this is a fix for a small memory leak in the fortran frontend. Tested on x86_64-pc-linux-gnu, nothing stands out besides the apparently well-known guality instability. OK for master ? -- >8 -- Delete m_client_data_hooks before it is reassigned in tree_diagnostics_defaults. This fixes a small memory leak in the fortran frontend, which restores the diagnostics configurations to their default values with a call to tree_diagnostics_defaults at the end of the main parse hook. gcc/ChangeLog: * tree-diagnostic.cc (tree_diagnostics_defaults): Delete allocated pointer before overwriting it. --- gcc/tree-diagnostic.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/tree-diagnostic.cc b/gcc/tree-diagnostic.cc index 731e3559cd8..d2f6637b6d9 100644 --- a/gcc/tree-diagnostic.cc +++ b/gcc/tree-diagnostic.cc @@ -377,5 +377,6 @@ tree_diagnostics_defaults (diagnostic_context *context) context->print_path = default_tree_diagnostic_path_printer; context->make_json_for_path = default_tree_make_json_for_path; context->set_locations_cb = set_inlining_locations; + delete context->m_client_data_hooks; context->m_client_data_hooks = make_compiler_data_hooks (); } -- 2.40.1
Re: [PATCH] Fortran: runtime bounds-checking in presence of array constructors [PR31059]
Le 31/08/2023 à 22:42, Harald Anlauf via Fortran a écrit : Dear all, gfortran's array bounds-checking code does a mostly reasonable job for array sections in expressions and assignments, but forgot the case that (rank-1) expressions can involve array constructors, which have a shape ;-) The attached patch walks over the loops generated by the scalarizer, checks for the presence of a constructor, and takes the first shape found as reference. (If several constructors are present, discrepancies in their shape seems to be already detected at compile time). For more details on what will be caught now see testcase. Regtested on x86_64-pc-linux-gnu. OK for mainline? This is OK. May I suggest to handle functions the same way? Thanks. Thanks, Harald
Re: [PATCH] fortran: Restore interface to its previous state on error [PR48776]
Le 28/08/2023 à 21:17, Harald Anlauf via Fortran a écrit :
Hi Mikael,
On 8/27/23 21:22, Mikael Morin via Gcc-patches wrote:
Hello,
this fixes an old error-recovery bug.
Tested on x86_64-pc-linux-gnu.
OK for master?
I have only a minor comment:
+/* Free the leading members of the gfc_interface linked list given in
INTR
+ up to the END element (exclusive: the END element is not freed).
+ If END is not nullptr, it is assumed that END is in the linked
list starting
+ with INTR. */
+
+static void
+free_interface_elements_until (gfc_interface *intr, gfc_interface *end)
+{
+ gfc_interface *next;
+
+ for (; intr != end; intr = next)
Would it make sense to add a protection for intr == NULL, i.e.:
+ for (; intr && intr != end; intr = next)
Just to prevent a NULL pointer dereference in case there
is a corruption of the chain or something else went wrong.
This would happen in the case END is not a member of the INTR linked
list. In that case, the most forgiving would be not freeing any memory
and just returning. But it would require walking the list a second time
to determine before proceeding if END is present, and let's not do work
that is expected to be useless.
I will just do the change as you suggest it.
Otherwise it looks good to me.
It appears that your patch similarly fixes PR107923. :-)
Good news. :-)
I will double check that none of the testcases there remain unfixed and
close as duplicate.
I don't know how you manage to make your way through the hundreds of
open PRs by the way.
Thanks for the review.
Thanks for the patch!
Harald
