Re: [Patch, fortran] PR84868 - [11/12/13/14/15 Regression] ICE in gfc_conv_descriptor_offset, at fortran/trans-array.c:208
Hi Harald, Thank you for the review and for the testing to destruction. Both issues are fixed in the attached patch. Note the new function 'h', which both tests that the namespace confusion is fixed and that the elemental-ness of LEN_TRIM is respected. The patch continues to regtest OK. If I don't receive anymore comments/corrections, I will commit tomorrow morning. Regards Paul On Sun, 14 Jul 2024 at 19:50, Harald Anlauf wrote: > Hi Paul, > > at first sight the patch seems to be the right approach, but > it breaks for the following two variations: > > (1) LEN_TRIM is elemental, but the following is erroneously rejected: > >function g(n) result(z) > integer, intent(in) :: n > character, parameter :: d(3,3) = 'x' > character(len_trim(d(n,n))) :: z > z = d(n,n) >end > > This is fixed here by commenting/removing the line > >expr->rank = 1; > > as the result shall have the same shape as the argument. > Can you check? > > (2) The handling of namespaces is problematic: using the same name > for a parameter within procedures in the same scope generates another > ICE. The following testcase demonstrates this: > > module m >implicit none >integer :: c > contains >function f(n) result(z) > integer, intent(in) :: n > character, parameter :: c(3) = ['x', 'y', 'z'] > character(len_trim(c(n))) :: z > z = c(n) >end >function h(n) result(z) > integer, intent(in) :: n > character, parameter :: c(3,3) = 'x' > character(len_trim(c(n,n))) :: z > z = c(n,n) >end > end > program p >use m >implicit none >print *, f(2) >print *, h(1) > end > > I get: > > pr84868-z0.f90:22:15: > > 22 | print *, h(1) >| 1 > internal compiler error: in gfc_conv_descriptor_stride_get, at > fortran/trans-array.cc:483 > 0x243e156 internal_error(char const*, ...) > ../../gcc-trunk/gcc/diagnostic-global-context.cc:491 > 0x96dd70 fancy_abort(char const*, int, char const*) > ../../gcc-trunk/gcc/diagnostic.cc:1725 > 0x749d68 gfc_conv_descriptor_stride_get(tree_node*, tree_node*) > ../../gcc-trunk/gcc/fortran/trans-array.cc:483 > [rest of traceback elided] > > Renaming the parameter array in h solves the problem. > > Am 13.07.24 um 17:57 schrieb Paul Richard Thomas: > > Hi All, > > > > Harald has pointed out that I attached the ChangeLog twice and the patch > > not at all :-( > > > > Please find the patch duly attached. > > > > Paul > > > > > > On Sat, 13 Jul 2024 at 10:58, Paul Richard Thomas < > > paul.richard.tho...@gmail.com> wrote: > > > >> Hi All, > >> > >> After messing around with argument mapping, where I found and fixed > >> another bug, I realised that the problem lay with simplification of > >> len_trim with an argument that is the element of a parameter array. The > fix > >> was then a straightforward lift of existing code in expr.cc. The mapping > >> bug is also fixed by supplying the se string length when building > character > >> typespecs. > >> > >> Regtests just fine. OK for mainline? I believe that this is safe for > >> backporting to 14-branch before the 14.2 release - thoughts? > > If you manage to correct/fix the above issues, I am fine with > backporting, as this appears a very reasonable fix. > > Thanks, > Harald > > >> Regards > >> > >> Paul > >> > > > >
Re: [Patch, fortran] PR84868 - [11/12/13/14/15 Regression] ICE in gfc_conv_descriptor_offset, at fortran/trans-array.c:208
Hi Paul, I am missing an attachment or is the change to small, that I've overlooked it :-) - Andre On Mon, 15 Jul 2024 09:21:54 +0100 Paul Richard Thomas wrote: > Hi Harald, > > Thank you for the review and for the testing to destruction. Both issues > are fixed in the attached patch. Note the new function 'h', which both > tests that the namespace confusion is fixed and that the elemental-ness of > LEN_TRIM is respected. > > The patch continues to regtest OK. If I don't receive anymore > comments/corrections, I will commit tomorrow morning. > > Regards > > Paul > > > On Sun, 14 Jul 2024 at 19:50, Harald Anlauf wrote: > > > Hi Paul, > > > > at first sight the patch seems to be the right approach, but > > it breaks for the following two variations: > > > > (1) LEN_TRIM is elemental, but the following is erroneously rejected: > > > >function g(n) result(z) > > integer, intent(in) :: n > > character, parameter :: d(3,3) = 'x' > > character(len_trim(d(n,n))) :: z > > z = d(n,n) > >end > > > > This is fixed here by commenting/removing the line > > > >expr->rank = 1; > > > > as the result shall have the same shape as the argument. > > Can you check? > > > > (2) The handling of namespaces is problematic: using the same name > > for a parameter within procedures in the same scope generates another > > ICE. The following testcase demonstrates this: > > > > module m > >implicit none > >integer :: c > > contains > >function f(n) result(z) > > integer, intent(in) :: n > > character, parameter :: c(3) = ['x', 'y', 'z'] > > character(len_trim(c(n))) :: z > > z = c(n) > >end > >function h(n) result(z) > > integer, intent(in) :: n > > character, parameter :: c(3,3) = 'x' > > character(len_trim(c(n,n))) :: z > > z = c(n,n) > >end > > end > > program p > >use m > >implicit none > >print *, f(2) > >print *, h(1) > > end > > > > I get: > > > > pr84868-z0.f90:22:15: > > > > 22 | print *, h(1) > >| 1 > > internal compiler error: in gfc_conv_descriptor_stride_get, at > > fortran/trans-array.cc:483 > > 0x243e156 internal_error(char const*, ...) > > ../../gcc-trunk/gcc/diagnostic-global-context.cc:491 > > 0x96dd70 fancy_abort(char const*, int, char const*) > > ../../gcc-trunk/gcc/diagnostic.cc:1725 > > 0x749d68 gfc_conv_descriptor_stride_get(tree_node*, tree_node*) > > ../../gcc-trunk/gcc/fortran/trans-array.cc:483 > > [rest of traceback elided] > > > > Renaming the parameter array in h solves the problem. > > > > Am 13.07.24 um 17:57 schrieb Paul Richard Thomas: > > > Hi All, > > > > > > Harald has pointed out that I attached the ChangeLog twice and the patch > > > not at all :-( > > > > > > Please find the patch duly attached. > > > > > > Paul > > > > > > > > > On Sat, 13 Jul 2024 at 10:58, Paul Richard Thomas < > > > paul.richard.tho...@gmail.com> wrote: > > > > > >> Hi All, > > >> > > >> After messing around with argument mapping, where I found and fixed > > >> another bug, I realised that the problem lay with simplification of > > >> len_trim with an argument that is the element of a parameter array. The > > fix > > >> was then a straightforward lift of existing code in expr.cc. The mapping > > >> bug is also fixed by supplying the se string length when building > > character > > >> typespecs. > > >> > > >> Regtests just fine. OK for mainline? I believe that this is safe for > > >> backporting to 14-branch before the 14.2 release - thoughts? > > > > If you manage to correct/fix the above issues, I am fine with > > backporting, as this appears a very reasonable fix. > > > > Thanks, > > Harald > > > > >> Regards > > >> > > >> Paul > > >> > > > > > > > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Patch, fortran] PR84868 - [11/12/13/14/15 Regression] ICE in gfc_conv_descriptor_offset, at fortran/trans-array.c:208
I've done it again! Patch duly added.
Paul
On Mon, 15 Jul 2024 at 09:21, Paul Richard Thomas <
paul.richard.tho...@gmail.com> wrote:
> Hi Harald,
>
> Thank you for the review and for the testing to destruction. Both issues
> are fixed in the attached patch. Note the new function 'h', which both
> tests that the namespace confusion is fixed and that the elemental-ness of
> LEN_TRIM is respected.
>
> The patch continues to regtest OK. If I don't receive anymore
> comments/corrections, I will commit tomorrow morning.
>
> Regards
>
> Paul
>
>
> On Sun, 14 Jul 2024 at 19:50, Harald Anlauf wrote:
>
>> Hi Paul,
>>
>> at first sight the patch seems to be the right approach, but
>> it breaks for the following two variations:
>>
>> (1) LEN_TRIM is elemental, but the following is erroneously rejected:
>>
>>function g(n) result(z)
>> integer, intent(in) :: n
>> character, parameter :: d(3,3) = 'x'
>> character(len_trim(d(n,n))) :: z
>> z = d(n,n)
>>end
>>
>> This is fixed here by commenting/removing the line
>>
>>expr->rank = 1;
>>
>> as the result shall have the same shape as the argument.
>> Can you check?
>>
>> (2) The handling of namespaces is problematic: using the same name
>> for a parameter within procedures in the same scope generates another
>> ICE. The following testcase demonstrates this:
>>
>> module m
>>implicit none
>>integer :: c
>> contains
>>function f(n) result(z)
>> integer, intent(in) :: n
>> character, parameter :: c(3) = ['x', 'y', 'z']
>> character(len_trim(c(n))) :: z
>> z = c(n)
>>end
>>function h(n) result(z)
>> integer, intent(in) :: n
>> character, parameter :: c(3,3) = 'x'
>> character(len_trim(c(n,n))) :: z
>> z = c(n,n)
>>end
>> end
>> program p
>>use m
>>implicit none
>>print *, f(2)
>>print *, h(1)
>> end
>>
>> I get:
>>
>> pr84868-z0.f90:22:15:
>>
>> 22 | print *, h(1)
>>| 1
>> internal compiler error: in gfc_conv_descriptor_stride_get, at
>> fortran/trans-array.cc:483
>> 0x243e156 internal_error(char const*, ...)
>> ../../gcc-trunk/gcc/diagnostic-global-context.cc:491
>> 0x96dd70 fancy_abort(char const*, int, char const*)
>> ../../gcc-trunk/gcc/diagnostic.cc:1725
>> 0x749d68 gfc_conv_descriptor_stride_get(tree_node*, tree_node*)
>> ../../gcc-trunk/gcc/fortran/trans-array.cc:483
>> [rest of traceback elided]
>>
>> Renaming the parameter array in h solves the problem.
>>
>> Am 13.07.24 um 17:57 schrieb Paul Richard Thomas:
>> > Hi All,
>> >
>> > Harald has pointed out that I attached the ChangeLog twice and the patch
>> > not at all :-(
>> >
>> > Please find the patch duly attached.
>> >
>> > Paul
>> >
>> >
>> > On Sat, 13 Jul 2024 at 10:58, Paul Richard Thomas <
>> > paul.richard.tho...@gmail.com> wrote:
>> >
>> >> Hi All,
>> >>
>> >> After messing around with argument mapping, where I found and fixed
>> >> another bug, I realised that the problem lay with simplification of
>> >> len_trim with an argument that is the element of a parameter array.
>> The fix
>> >> was then a straightforward lift of existing code in expr.cc. The
>> mapping
>> >> bug is also fixed by supplying the se string length when building
>> character
>> >> typespecs.
>> >>
>> >> Regtests just fine. OK for mainline? I believe that this is safe for
>> >> backporting to 14-branch before the 14.2 release - thoughts?
>>
>> If you manage to correct/fix the above issues, I am fine with
>> backporting, as this appears a very reasonable fix.
>>
>> Thanks,
>> Harald
>>
>> >> Regards
>> >>
>> >> Paul
>> >>
>> >
>>
>>
diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
index 7a5d31c01a6..931a9a8f5ed 100644
--- a/gcc/fortran/simplify.cc
+++ b/gcc/fortran/simplify.cc
@@ -4637,6 +4637,75 @@ gfc_simplify_len_trim (gfc_expr *e, gfc_expr *kind)
if (k == -1)
return &gfc_bad_expr;
+ if (e->expr_type == EXPR_VARIABLE
+ && e->ts.type == BT_CHARACTER
+ && e->symtree->n.sym->attr.flavor == FL_PARAMETER
+ && e->ref && e->ref->type == REF_ARRAY
+ && e->ref->u.ar.dimen_type[0] == DIMEN_ELEMENT
+ && e->symtree->n.sym->value)
+{
+ char name[2*GFC_MAX_SYMBOL_LEN + 10];
+ gfc_namespace *ns = e->symtree->n.sym->ns;
+ gfc_symtree *st;
+ gfc_expr *expr;
+ gfc_expr *p;
+ gfc_constructor *c;
+ int cnt = 0;
+
+ sprintf (name, "_len_trim_%s_%s", e->symtree->n.sym->name, ns->proc_name->name);
+ st = gfc_find_symtree (ns->sym_root, name);
+ if (st)
+ goto already_built;
+
+ /* Recursively call this fcn to simplify the constructor elements. */
+ expr = gfc_copy_expr (e->symtree->n.sym->value);
+ expr->ts.type = BT_INTEGER;
+ expr->ts.kind = k;
+ expr->ts.u.cl = NULL;
+ c = gfc_constructor_first (expr->value.constructor);
+ for (; c; c = gfc_constructor_next (c))
+ {
+ if (c->iterator)
+ continue;
+
+ if (c->expr && c->expr->ts.type =
Re: [Patch, fortran] PR84868 - [11/12/13/14/15 Regression] ICE in gfc_conv_descriptor_offset, at fortran/trans-array.c:208
Hi Paul, I tried to "break" your patch, but I failed. (I tried having same function in both modules with identical signature; but I do not get a symbol collision). So to me the patch looks fine now. But you may want to give Harald some time for a look. Thanks for the patch, Andre On Mon, 15 Jul 2024 10:41:45 +0100 Paul Richard Thomas wrote: > I've done it again! Patch duly added. > > Paul > > On Mon, 15 Jul 2024 at 09:21, Paul Richard Thomas < > paul.richard.tho...@gmail.com> wrote: > > > Hi Harald, > > > > Thank you for the review and for the testing to destruction. Both issues > > are fixed in the attached patch. Note the new function 'h', which both > > tests that the namespace confusion is fixed and that the elemental-ness of > > LEN_TRIM is respected. > > > > The patch continues to regtest OK. If I don't receive anymore > > comments/corrections, I will commit tomorrow morning. > > > > Regards > > > > Paul > > > > > > On Sun, 14 Jul 2024 at 19:50, Harald Anlauf wrote: > > > >> Hi Paul, > >> > >> at first sight the patch seems to be the right approach, but > >> it breaks for the following two variations: > >> > >> (1) LEN_TRIM is elemental, but the following is erroneously rejected: > >> > >>function g(n) result(z) > >> integer, intent(in) :: n > >> character, parameter :: d(3,3) = 'x' > >> character(len_trim(d(n,n))) :: z > >> z = d(n,n) > >>end > >> > >> This is fixed here by commenting/removing the line > >> > >>expr->rank = 1; > >> > >> as the result shall have the same shape as the argument. > >> Can you check? > >> > >> (2) The handling of namespaces is problematic: using the same name > >> for a parameter within procedures in the same scope generates another > >> ICE. The following testcase demonstrates this: > >> > >> module m > >>implicit none > >>integer :: c > >> contains > >>function f(n) result(z) > >> integer, intent(in) :: n > >> character, parameter :: c(3) = ['x', 'y', 'z'] > >> character(len_trim(c(n))) :: z > >> z = c(n) > >>end > >>function h(n) result(z) > >> integer, intent(in) :: n > >> character, parameter :: c(3,3) = 'x' > >> character(len_trim(c(n,n))) :: z > >> z = c(n,n) > >>end > >> end > >> program p > >>use m > >>implicit none > >>print *, f(2) > >>print *, h(1) > >> end > >> > >> I get: > >> > >> pr84868-z0.f90:22:15: > >> > >> 22 | print *, h(1) > >>| 1 > >> internal compiler error: in gfc_conv_descriptor_stride_get, at > >> fortran/trans-array.cc:483 > >> 0x243e156 internal_error(char const*, ...) > >> ../../gcc-trunk/gcc/diagnostic-global-context.cc:491 > >> 0x96dd70 fancy_abort(char const*, int, char const*) > >> ../../gcc-trunk/gcc/diagnostic.cc:1725 > >> 0x749d68 gfc_conv_descriptor_stride_get(tree_node*, tree_node*) > >> ../../gcc-trunk/gcc/fortran/trans-array.cc:483 > >> [rest of traceback elided] > >> > >> Renaming the parameter array in h solves the problem. > >> > >> Am 13.07.24 um 17:57 schrieb Paul Richard Thomas: > >> > Hi All, > >> > > >> > Harald has pointed out that I attached the ChangeLog twice and the patch > >> > not at all :-( > >> > > >> > Please find the patch duly attached. > >> > > >> > Paul > >> > > >> > > >> > On Sat, 13 Jul 2024 at 10:58, Paul Richard Thomas < > >> > paul.richard.tho...@gmail.com> wrote: > >> > > >> >> Hi All, > >> >> > >> >> After messing around with argument mapping, where I found and fixed > >> >> another bug, I realised that the problem lay with simplification of > >> >> len_trim with an argument that is the element of a parameter array. > >> The fix > >> >> was then a straightforward lift of existing code in expr.cc. The > >> mapping > >> >> bug is also fixed by supplying the se string length when building > >> character > >> >> typespecs. > >> >> > >> >> Regtests just fine. OK for mainline? I believe that this is safe for > >> >> backporting to 14-branch before the 14.2 release - thoughts? > >> > >> If you manage to correct/fix the above issues, I am fine with > >> backporting, as this appears a very reasonable fix. > >> > >> Thanks, > >> Harald > >> > >> >> Regards > >> >> > >> >> Paul > >> >> > >> > > >> > >> -- Andre Vehreschild * Email: vehre ad gmx dot de
RE: Lower zeroing array assignment to memset for allocatable arrays
> -Original Message-
> From: Harald Anlauf
> Sent: Saturday, July 13, 2024 1:15 AM
> To: Prathamesh Kulkarni ; gcc-
> patc...@gcc.gnu.org; fortran@gcc.gnu.org
> Subject: Re: Lower zeroing array assignment to memset for allocatable
> arrays
>
> External email: Use caution opening links or attachments
>
>
> Hi Prathamesh,
Hi Harald,
>
> Am 12.07.24 um 15:31 schrieb Prathamesh Kulkarni:
> > It seems that component references are not currently handled even
> for static size arrays ?
> > For eg:
> > subroutine test_dt (dt, y)
> > implicit none
> > real :: y (10, 20, 30)
> > type t
> >real :: x(10, 20, 30)
> > end type t
> > type(t) :: dt
> > y = 0
> > dt% x = 0
> > end subroutine
> >
> > With trunk, it generates memset for 'y' but not for dt%x.
> > That happens because copyable_array_p returns false for dt%x,
> because
> > expr->ref->next is non NULL:
> >
> >/* First check it's an array. */
> >if (expr->rank < 1 || !expr->ref || expr->ref->next)
> > return false;
> >
> > and gfc_full_array_ref_p(expr) bails out if expr->ref->type !=
> REF_ARRAY.
>
> Indeed that check (as is) prevents the use of component refs.
> (I just tried to modify the this part to cycle thru the refs, but then
> I get regressions in the testsuite for some of the coarray tests.
> Furthermore, gfc_trans_zero_assign would need further changes to
> handle even the constant shapes from above.)
>
> > Looking thru git history, it seems both the checks were added in
> 18eaa2c0cd20 to fix PR33370.
> > (Even after removing these checks, the previous patch bails out from
> > gfc_trans_zero_assign because GFC_DESCRIPTOR_TYPE_P (type) returns
> > false for component ref and ends up returning NULL_TREE) I am
> working on extending the patch to handle component refs for statically
> sized as well as allocatable arrays.
> >
> > Since it looks like a bigger change and an extension to current
> > functionality, will it be OK to commit the previous patch as-is (if
> it looks correct) and address component refs in follow up one ?
>
> I agree that it is reasonable to defer the handling of arrays as
> components of derived types, and recommend to do the following:
>
> - replace "&& gfc_is_simply_contiguous (expr, true, false))" in your
>last patch by "&& gfc_is_simply_contiguous (expr, false, false))",
>as that would also allow to treat
>
>z(:,::1,:) = 0
>
>as contiguous if z is allocatable or a contiguous pointer.
>
> - open a PR in bugzilla to track the missed-optimization for
>the cases we discussed here, and link the discussion in the ML.
Done: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115935
>
> Your patch then will be OK for mainline.
Thanks, does the attached version look OK ?
Bootstrapped+tested on aarch64-linux-gnu, x86_64-linux-gnu.
Thanks,
Prathamesh
>
> Thanks,
> Harald
>
> > Thanks,
> > Prathamesh
> >>
> >> Thanks,
> >> Harald
> >>
> >>> Bootstrapped+tested on aarch64-linux-gnu.
> >>> Does the attached patch look OK ?
> >>>
> >>> Signed-off-by: Prathamesh Kulkarni
> >>>
> >>> Thanks,
> >>> Prathamesh
>
> Thanks,
> Harald
>
>
> > Signed-off-by: Prathamesh Kulkarni
> >
> > Thanks,
> > Prathamesh
> >>>
> >
Lower zeroing array assignment to memset for allocatable arrays.
gcc/fortran/ChangeLog:
* trans-expr.cc (gfc_trans_zero_assign): Handle allocatable arrays.
gcc/testsuite/ChangeLog:
* gfortran.dg/array_memset_3.f90: New test.
Signed-off-by: Prathamesh Kulkarni
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 477c2720187..a85b41bf815 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -11515,18 +11515,24 @@ gfc_trans_zero_assign (gfc_expr * expr)
type = TREE_TYPE (dest);
if (POINTER_TYPE_P (type))
type = TREE_TYPE (type);
- if (!GFC_ARRAY_TYPE_P (type))
-return NULL_TREE;
-
- /* Determine the length of the array. */
- len = GFC_TYPE_ARRAY_SIZE (type);
- if (!len || TREE_CODE (len) != INTEGER_CST)
+ if (GFC_ARRAY_TYPE_P (type))
+{
+ /* Determine the length of the array. */
+ len = GFC_TYPE_ARRAY_SIZE (type);
+ if (!len || TREE_CODE (len) != INTEGER_CST)
+ return NULL_TREE;
+}
+ else if (GFC_DESCRIPTOR_TYPE_P (type)
+ && gfc_is_simply_contiguous (expr, false, false))
+{
+ if (POINTER_TYPE_P (TREE_TYPE (dest)))
+ dest = build_fold_indirect_ref_loc (input_location, dest);
+ len = gfc_conv_descriptor_size (dest, GFC_TYPE_ARRAY_RANK (type));
+ dest = gfc_conv_descriptor_data_get (dest);
+}
+ else
return NULL_TREE;
- tmp = TYPE_SIZE_UNIT (gfc_get_element_type (type));
- len = fold_build2_loc (input_location, MULT_EXPR, gfc_array_index_type, len,
-fold_convert (gfc_array_index_type, tmp));
-
/* If we are zeroing a local array avoid taking its address by emitting
a = {} instead. */
if (!POINTER_TYPE_P (TREE_TYPE (dest)))
@
[Patch, fortran] PR115070 (and PR115348) - [13/14/15 Regression] ICE using IEEE_ARITHMETIC in a derived type method with class, intent(out)
Hi All,
I am not sure that I understand why this bug occurs. The regression was
introduced by my patch that had gfc_trans_class_init_assign return
NULL_TREE, when all the components of the default initializer are NULL.
Note that this only afflicts scalar dummy arguments.
With pr115070:
void my_sub (struct __class_my_mod_My_type_t & restrict obs)
c_char fpstate.5[33];// This disappears, when NULL is returned.
try
{
_gfortran_ieee_procedure_entry ((void *) &fpstate.5);
With pr115348:
void myroutine (struct __class_mymodule_Mytype_t & restrict self)
{
static logical(kind=4) is_recursive.0 = 0; // This disappears when NULL
is returned
try
{
if (is_recursive.0)
The fix is equally magical in that finishing build_empty_stmt seems to
provide the backend with everything that it needs to retain these
declarations. See the attached patch. If somebody can explain what causes
the problem and why the patch fixes it, I would be very pleased. As far as
I can tell, the tail end of trans_code should have been sufficient to
handle the return of NULL_TREE.
Anyway, work it does and regtests OK. OK for mainline and backporting?
Regards
Paul
Change.Logs
Description: Binary data
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 477c2720187..d84ab46897f 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -1789,10 +1789,12 @@ gfc_trans_class_init_assign (gfc_code *code)
{
stmtblock_t block;
tree tmp;
+ bool cmp_flag = true;
gfc_se dst,src,memsz;
gfc_expr *lhs, *rhs, *sz;
gfc_component *cmp;
gfc_symbol *sym;
+ gfc_ref *ref;
gfc_start_block (&block);
@@ -1810,24 +1812,25 @@ gfc_trans_class_init_assign (gfc_code *code)
rhs->rank = 0;
/* Check def_init for initializers. If this is an INTENT(OUT) dummy with all
- default initializer components NULL, return NULL_TREE and use the passed
- value as required by F2018(8.5.10). */
+ default initializer components NULL, use the passed value even though
+ F2018(8.5.10) asserts that it should considered to be undefined. This is
+ needed for consistency with other brands. */
sym = code->expr1->expr_type == EXPR_VARIABLE ? code->expr1->symtree->n.sym
: NULL;
if (code->op != EXEC_ALLOCATE
&& sym && sym->attr.dummy
&& sym->attr.intent == INTENT_OUT)
{
- if (!lhs->ref && lhs->symtree->n.sym->attr.dummy)
+ ref = rhs->ref;
+ while (ref && ref->next)
+ ref = ref->next;
+ cmp = ref->u.c.component->ts.u.derived->components;
+ for (; cmp; cmp = cmp->next)
{
- cmp = rhs->ref->next->u.c.component->ts.u.derived->components;
- for (; cmp; cmp = cmp->next)
- {
- if (cmp->initializer)
- break;
- else if (!cmp->next)
- return NULL_TREE;
- }
+ if (cmp->initializer)
+ break;
+ else if (!cmp->next)
+ cmp_flag = false;
}
}
@@ -1841,7 +1844,7 @@ gfc_trans_class_init_assign (gfc_code *code)
gfc_add_full_array_ref (lhs, tmparr);
tmp = gfc_trans_class_array_init_assign (rhs, lhs, code->expr1);
}
- else
+ else if (cmp_flag)
{
/* Scalar initialization needs the _data component. */
gfc_add_data_component (lhs);
@@ -1871,6 +1874,8 @@ gfc_trans_class_init_assign (gfc_code *code)
tmp, build_empty_stmt (input_location));
}
}
+ else
+tmp = build_empty_stmt (input_location);
if (code->expr1->symtree->n.sym->attr.dummy
&& (code->expr1->symtree->n.sym->attr.optional
diff --git a/gcc/testsuite/gfortran.dg/pr115070.f90 b/gcc/testsuite/gfortran.dg/pr115070.f90
new file mode 100644
index 000..9378f770e2c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr115070.f90
@@ -0,0 +1,28 @@
+! { dg-do compile }
+!
+! Test the fix for PR115070
+!
+! Contributed by Sebastien Bardeau
+!
+module my_mod
+ type my_type
+integer :: a
+ contains
+final :: myfinal
+ end type my_type
+contains
+ subroutine my_sub(obs)
+use ieee_arithmetic
+class(my_type), intent(out) :: obs
+ end subroutine my_sub
+ subroutine myfinal (arg)
+type (my_type) :: arg
+print *, arg%a
+ end
+end module my_mod
+
+ use my_mod
+ type (my_type) :: z
+ z%a = 42
+ call my_sub (z)
+end
diff --git a/gcc/testsuite/gfortran.dg/pr115348.f90 b/gcc/testsuite/gfortran.dg/pr115348.f90
new file mode 100644
index 000..bc644b2f1c0
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr115348.f90
@@ -0,0 +1,35 @@
+! { dg-do compile }
+! { dg-options "-fcheck=recursion" }
+!
+! Test the fix for pr115348.
+!
+! Contributed by Maxime van den Bossche
+!
+module mymodule
+implicit none
+
+type mytype
+integer :: mynumber
+contains
+procedure :: myroutine
+end type mytype
+
+contains
+
+subroutine myroutine(self)
+class(mytype), intent(out) :: self
+
+self%mynumber = 1
+end subroutine myroutine
+end module mymodule
+
+
+program myprogram
+use mymodule, only: mytype
+i
Re: [Patch, fortran] PR84868 - [11/12/13/14/15 Regression] ICE in gfc_conv_descriptor_offset, at fortran/trans-array.c:208
Hi Paul, Andre, at the risk of getting stoned to death, here's an example: module m implicit none integer :: c contains function f(n) result(z) integer, intent(in) :: n character, parameter :: c(3) = ['x', 'y', 'z'] character(len_trim(c(n))) :: z z = c(n) end function h(n) result(z) integer, intent(in) :: n character, parameter :: c(3,3) = 'x' character(len_trim(c(n,n))) :: z z = c(n,n) contains function k(n) result(z) integer, intent(in) :: n character, parameter :: c(3) = ['*', '+', '-'] character(len_trim(c(n))) :: z z = c(n) end end end program p use m implicit none print *, f(2) print *, h(1) end % gfc-15 pr84868-z0.f90 -fdump-fortran-original Namespace: A-Z: (UNKNOWN 0) procedure name = m symtree: '@0' || symbol: '_len_trim_c_h' from namespace 'h' symtree: '@1' || symbol: '_len_trim_c_k' from namespace 'k' symtree: '@2' || symbol: '_len_trim_c_f' from namespace 'f' symtree: 'c' || symbol: 'c' [...] f951: internal compiler error: in gfc_create_module_variable, at fortran/trans-decl.cc:5515 0x2438776 internal_error(char const*, ...) ../../gcc-trunk/gcc/diagnostic-global-context.cc:491 0x96df24 fancy_abort(char const*, int, char const*) ../../gcc-trunk/gcc/diagnostic.cc:1725 0x752d8a gfc_create_module_variable ../../gcc-trunk/gcc/fortran/trans-decl.cc:5515 0x752d8a gfc_create_module_variable ../../gcc-trunk/gcc/fortran/trans-decl.cc:5428 [...] So you are very close! My example is likely very artificial, but nevertheless legal. We hit the following assert: gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE || (sym->ns->parent->proc_name->attr.flavor == FL_MODULE && sym->fn_result_spec)); For '_len_trim_c_k': Breakpoint 1, gfc_create_module_variable (sym=0x32af2f0) at ../../gcc-trunk/gcc/fortran/trans-decl.cc:5515 5515 gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE (gdb) p sym->ns->proc_name->attr.flavor $9 = FL_PROCEDURE (gdb) p sym->ns->parent->proc_name->attr.flavor $10 = FL_PROCEDURE This is not good. Can we prevent the export of this artificial symbol? Otherwise the patch is fine. Thanks, Harald Am 15.07.24 um 12:23 schrieb Andre Vehreschild: Hi Paul, I tried to "break" your patch, but I failed. (I tried having same function in both modules with identical signature; but I do not get a symbol collision). So to me the patch looks fine now. But you may want to give Harald some time for a look. Thanks for the patch, Andre On Mon, 15 Jul 2024 10:41:45 +0100 Paul Richard Thomas wrote: I've done it again! Patch duly added. Paul On Mon, 15 Jul 2024 at 09:21, Paul Richard Thomas < paul.richard.tho...@gmail.com> wrote: Hi Harald, Thank you for the review and for the testing to destruction. Both issues are fixed in the attached patch. Note the new function 'h', which both tests that the namespace confusion is fixed and that the elemental-ness of LEN_TRIM is respected. The patch continues to regtest OK. If I don't receive anymore comments/corrections, I will commit tomorrow morning. Regards Paul On Sun, 14 Jul 2024 at 19:50, Harald Anlauf wrote: Hi Paul, at first sight the patch seems to be the right approach, but it breaks for the following two variations: (1) LEN_TRIM is elemental, but the following is erroneously rejected: function g(n) result(z) integer, intent(in) :: n character, parameter :: d(3,3) = 'x' character(len_trim(d(n,n))) :: z z = d(n,n) end This is fixed here by commenting/removing the line expr->rank = 1; as the result shall have the same shape as the argument. Can you check? (2) The handling of namespaces is problematic: using the same name for a parameter within procedures in the same scope generates another ICE. The following testcase demonstrates this: module m implicit none integer :: c contains function f(n) result(z) integer, intent(in) :: n character, parameter :: c(3) = ['x', 'y', 'z'] character(len_trim(c(n))) :: z z = c(n) end function h(n) result(z) integer, intent(in) :: n character, parameter :: c(3,3) = 'x' character(len_trim(c(n,n))) :: z z = c(n,n) end end program p use m implicit none print *, f(2) print *, h(1) end I get: pr84868-z0.f90:22:15: 22 | print *, h(1) | 1 internal compiler error: in gfc_conv_descriptor_stride_get, at fortran/trans-array.cc:483 0x243e156 internal_error(char const*, ...) ../../gcc-trunk/gcc/diagnostic-global-context.cc:491 0x96dd70 fancy_abort(char const*, int, char const*) ../../gcc-trunk/gcc/diagnostic.cc:1725 0x749d68 gfc_conv_descriptor_stride_get(tree_node*, tree_node*) ../../gcc-trunk/gcc/fortran/trans-array.cc:483 [rest of traceback elided] Renaming the parameter array in h
Re: [Patch, fortran] PR84868 - [11/12/13/14/15 Regression] ICE in gfc_conv_descriptor_offset, at fortran/trans-array.c:208
Replying to myself:
Am 15.07.24 um 19:35 schrieb Harald Anlauf:
For '_len_trim_c_k':
Breakpoint 1, gfc_create_module_variable (sym=0x32af2f0)
at ../../gcc-trunk/gcc/fortran/trans-decl.cc:5515
5515 gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE
(gdb) p sym->ns->proc_name->attr.flavor
$9 = FL_PROCEDURE
(gdb) p sym->ns->parent->proc_name->attr.flavor
$10 = FL_PROCEDURE
This is not good.
Can we prevent the export of this artificial symbol?
Like this here (to give an idea, but otherwise untested):
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index 54ab60b4935..cc6ac7f192e 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -5455,6 +5468,13 @@ gfc_create_module_variable (gfc_symbol * sym)
&& !(sym->attr.flavor == FL_PROCEDURE && sym->attr.proc_pointer))
return;
+ /* Do not output artificially created parameters. */
+ if (sym->attr.flavor == FL_PARAMETER
+ && sym->name[0] == '_'
+ && sym->ns->proc_name->attr.flavor == FL_PROCEDURE
+ && sym->ns->parent->proc_name->attr.flavor == FL_PROCEDURE)
+return;
+
if ((sym->attr.in_common || sym->attr.in_equivalence) &&
sym->backend_decl)
{
decl = sym->backend_decl;
Maybe one might mark the symbol already at creation time and
detect this mark here, too.
Harald
Re: Lower zeroing array assignment to memset for allocatable arrays
Hi Prathamesh! Am 15.07.24 um 15:07 schrieb Prathamesh Kulkarni: -Original Message- From: Harald Anlauf I agree that it is reasonable to defer the handling of arrays as components of derived types, and recommend to do the following: - replace "&& gfc_is_simply_contiguous (expr, true, false))" in your last patch by "&& gfc_is_simply_contiguous (expr, false, false))", as that would also allow to treat z(:,::1,:) = 0 as contiguous if z is allocatable or a contiguous pointer. - open a PR in bugzilla to track the missed-optimization for the cases we discussed here, and link the discussion in the ML. Done: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115935 Your patch then will be OK for mainline. Thanks, does the attached version look OK ? Bootstrapped+tested on aarch64-linux-gnu, x86_64-linux-gnu. This is now OK. Thanks for the patch! Harald Thanks, Prathamesh
Re: [Patch, fortran] PR84868 - [11/12/13/14/15 Regression] ICE in gfc_conv_descriptor_offset, at fortran/trans-array.c:208
Am 15.07.24 um 20:27 schrieb Harald Anlauf:
Replying to myself:
Am 15.07.24 um 19:35 schrieb Harald Anlauf:
For '_len_trim_c_k':
Breakpoint 1, gfc_create_module_variable (sym=0x32af2f0)
at ../../gcc-trunk/gcc/fortran/trans-decl.cc:5515
5515 gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE
(gdb) p sym->ns->proc_name->attr.flavor
$9 = FL_PROCEDURE
(gdb) p sym->ns->parent->proc_name->attr.flavor
$10 = FL_PROCEDURE
This is not good.
Can we prevent the export of this artificial symbol?
Like this here (to give an idea, but otherwise untested):
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index 54ab60b4935..cc6ac7f192e 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -5455,6 +5468,13 @@ gfc_create_module_variable (gfc_symbol * sym)
&& !(sym->attr.flavor == FL_PROCEDURE && sym->attr.proc_pointer))
return;
+ /* Do not output artificially created parameters. */
+ if (sym->attr.flavor == FL_PARAMETER
+ && sym->name[0] == '_'
+ && sym->ns->proc_name->attr.flavor == FL_PROCEDURE
+ && sym->ns->parent->proc_name->attr.flavor == FL_PROCEDURE)
+ return;
+
if ((sym->attr.in_common || sym->attr.in_equivalence) &&
sym->backend_decl)
{
decl = sym->backend_decl;
Maybe one might mark the symbol already at creation time and
detect this mark here, too.
JFTR: this regtests cleanly here.
While looking over the last version of the patch again,
the following should be corrected:
@@ -4637,6 +4637,75 @@ gfc_simplify_len_trim (gfc_expr *e, gfc_expr *kind)
if (k == -1)
return &gfc_bad_expr;
+ if (e->expr_type == EXPR_VARIABLE
+ && e->ts.type == BT_CHARACTER
+ && e->symtree->n.sym->attr.flavor == FL_PARAMETER
+ && e->ref && e->ref->type == REF_ARRAY
+ && e->ref->u.ar.dimen_type[0] == DIMEN_ELEMENT
+ && e->symtree->n.sym->value)
+{
+ char name[2*GFC_MAX_SYMBOL_LEN + 10];
^^ this has to be 12 now
+ gfc_namespace *ns = e->symtree->n.sym->ns;
+ gfc_symtree *st;
+ gfc_expr *expr;
+ gfc_expr *p;
+ gfc_constructor *c;
+ int cnt = 0;
+
+ sprintf (name, "_len_trim_%s_%s", e->symtree->n.sym->name,
ns->proc_name->name);
Cheers,
Harald
