* [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] @ 2024-02-15 17:50 Peter Hill 2024-02-16 21:19 ` Harald Anlauf 0 siblings, 1 reply; 9+ messages in thread From: Peter Hill @ 2024-02-15 17:50 UTC (permalink / raw) To: gcc-patches; +Cc: fortran Dear all, The attached patch fixes PR105658 by forcing an array temporary to be created. This is required when passing an array component, but this didn't happen if the dummy argument was an unlimited polymorphic type. The problem bit of code is in `gfc_conv_expr_descriptor`, near L7828: subref_array_target = (is_subref_array (expr) && (se->direct_byref || expr->ts.type == BT_CHARACTER)); need_tmp = (gfc_ref_needs_temporary_p (expr->ref) && !subref_array_target); where `need_tmp` is being evaluated to 0. The logic here isn't clear to me, and this function is used in several places, which is why I went with setting `parmse.force_tmp = 1` in `gfc_conv_procedure_call` and using the same conditional as the later branch for the non-polymorphic case (near the call to `gfc_conv_subref_array_arg`) If this patch is ok, please could someone commit it for me? This is my first patch for GCC, so apologies in advance if the commit message is missing something. Tested on x86_64-pc-linux-gnu. The bug is present in gfortran back to 4.9, so should it also be backported? Cheers, Peter PR fortran/105658 gcc/fortran/ChangeLog * trans-expr.cc (gfc_conv_procedure_call): When passing an array component reference of intrinsic type to a procedure with an unlimited polymorphic dummy argument, a temporary should be created. gcc/testsuite/ChangeLog * gfortran.dg/PR105658.f90: New test. --- gcc/fortran/trans-expr.cc | 8 ++++++++ gcc/testsuite/gfortran.dg/PR105658.f90 | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/PR105658.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index a0593b76f18..7fd3047c4e9 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6439,6 +6439,14 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, CLASS object for the unlimited polymorphic formal. */ gfc_find_vtab (&e->ts); gfc_init_se (&parmse, se); + /* The actual argument is a component reference to an array + of derived types, so we need to force creation of a + temporary */ + if (e->expr_type == EXPR_VARIABLE + && is_subref_array (e) + && !(fsym && fsym->attr.pointer)) + parmse.force_tmp = 1; + gfc_conv_intrinsic_to_class (&parmse, e, fsym->ts); } diff --git a/gcc/testsuite/gfortran.dg/PR105658.f90 b/gcc/testsuite/gfortran.dg/PR105658.f90 new file mode 100644 index 00000000000..407ee25f77c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/PR105658.f90 @@ -0,0 +1,25 @@ +! { dg-do compile } +! { dg-options "-Warray-temporaries" } +! Test fix for incorrectly passing array component to unlimited polymorphic procedure + +module test_PR105658_mod + implicit none + type :: foo + integer :: member1 + integer :: member2 + end type foo +contains + subroutine print_poly(array) + class(*), dimension(:), intent(in) :: array + select type(array) + type is (integer) + print*, array + end select + end subroutine print_poly + + subroutine do_print(thing) + type(foo), dimension(3), intent(in) :: thing + call print_poly(thing%member1) ! { dg-warning "array temporary" } + end subroutine do_print + +end module test_PR105658_mod -- 2.43.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] 2024-02-15 17:50 [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] Peter Hill @ 2024-02-16 21:19 ` Harald Anlauf 2024-02-16 21:19 ` Harald Anlauf 2024-02-19 15:19 ` Peter Hill 0 siblings, 2 replies; 9+ messages in thread From: Harald Anlauf @ 2024-02-16 21:19 UTC (permalink / raw) To: Peter Hill, gcc-patches; +Cc: fortran Hi Peter, thanks for your contribution to gfortran! You've found indeed a solution for a potentially annoying bug. Am 15.02.24 um 18:50 schrieb Peter Hill: > Dear all, > > The attached patch fixes PR105658 by forcing an array temporary to be > created. This is required when passing an array component, but this > didn't happen if the dummy argument was an unlimited polymorphic type. > > The problem bit of code is in `gfc_conv_expr_descriptor`, near L7828: > > subref_array_target = (is_subref_array (expr) > && (se->direct_byref > || expr->ts.type == BT_CHARACTER)); > need_tmp = (gfc_ref_needs_temporary_p (expr->ref) > && !subref_array_target); > > where `need_tmp` is being evaluated to 0. The logic here isn't clear > to me, and this function is used in several places, which is why I > went with setting `parmse.force_tmp = 1` in `gfc_conv_procedure_call` > and using the same conditional as the later branch for the > non-polymorphic case (near the call to `gfc_conv_subref_array_arg`) > > If this patch is ok, please could someone commit it for me? This is my > first patch for GCC, so apologies in advance if the commit message is > missing something. Your patch mostly does the right thing. Note that when fsym is an unlimited polymorphic, some of its attributes are buried deep within its internal representation. I would also prefer to move the code to gfc_conv_intrinsic_to_class where it seems to fit better, like: diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index a0593b76f18..db906caa52e 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -1019,6 +1019,14 @@ gfc_conv_intrinsic_to_class (gfc_se *parmse, gfc_expr *e, tmp = gfc_typenode_for_spec (&class_ts); var = gfc_create_var (tmp, "class"); + /* Force a temporary for component or substring references. */ + if (unlimited_poly + && class_ts.u.derived->components->attr.dimension + && !class_ts.u.derived->components->attr.class_pointer + && !class_ts.u.derived->components->attr.allocatable + && is_subref_array (e)) + parmse->force_tmp = 1; + /* Set the vptr. */ ctree = gfc_class_vptr_get (var); (I am not entirely sure whether we need to exclude pointer and allocatable attributes here explicitly, given the constraints in F2023:15.5.2.6, but other may have an opinion, too. The above should be safe anyway.) > Tested on x86_64-pc-linux-gnu. > > The bug is present in gfortran back to 4.9, so should it also be backported? I think we'll target 14-mainline and might consider a backport to 13-branch. > Cheers, > Peter > > PR fortran/105658 > > gcc/fortran/ChangeLog > > * trans-expr.cc (gfc_conv_procedure_call): When passing an > array component reference of intrinsic type to a procedure > with an unlimited polymorphic dummy argument, a temporary > should be created. > > gcc/testsuite/ChangeLog > > * gfortran.dg/PR105658.f90: New test. > --- > gcc/fortran/trans-expr.cc | 8 ++++++++ > gcc/testsuite/gfortran.dg/PR105658.f90 | 25 +++++++++++++++++++++++++ > 2 files changed, 33 insertions(+) > create mode 100644 gcc/testsuite/gfortran.dg/PR105658.f90 > > diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc > index a0593b76f18..7fd3047c4e9 100644 > --- a/gcc/fortran/trans-expr.cc > +++ b/gcc/fortran/trans-expr.cc > @@ -6439,6 +6439,14 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, > CLASS object for the unlimited polymorphic formal. */ > gfc_find_vtab (&e->ts); > gfc_init_se (&parmse, se); > + /* The actual argument is a component reference to an array > + of derived types, so we need to force creation of a > + temporary */ > + if (e->expr_type == EXPR_VARIABLE > + && is_subref_array (e) > + && !(fsym && fsym->attr.pointer)) > + parmse.force_tmp = 1; > + > gfc_conv_intrinsic_to_class (&parmse, e, fsym->ts); > > } > diff --git a/gcc/testsuite/gfortran.dg/PR105658.f90 > b/gcc/testsuite/gfortran.dg/PR105658.f90 > new file mode 100644 > index 00000000000..407ee25f77c > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/PR105658.f90 > @@ -0,0 +1,25 @@ > +! { dg-do compile } > +! { dg-options "-Warray-temporaries" } > +! Test fix for incorrectly passing array component to unlimited > polymorphic procedure > + > +module test_PR105658_mod > + implicit none > + type :: foo > + integer :: member1 > + integer :: member2 > + end type foo > +contains > + subroutine print_poly(array) > + class(*), dimension(:), intent(in) :: array > + select type(array) > + type is (integer) > + print*, array > + end select > + end subroutine print_poly > + > + subroutine do_print(thing) > + type(foo), dimension(3), intent(in) :: thing > + call print_poly(thing%member1) ! { dg-warning "array temporary" } > + end subroutine do_print > + > +end module test_PR105658_mod One could extend this testcase to cover substrings as well: module test_PR105658_mod implicit none type :: foo integer :: member1 integer :: member2 end type foo contains subroutine print_poly(array) class(*), dimension(:), intent(in) :: array select type(array) type is (integer) print*, array type is (character(*)) print *, array end select end subroutine print_poly subroutine do_print(thing) type(foo), dimension(3), intent(in) :: thing type(foo), parameter :: y(3) = [foo(1,2),foo(3,4),foo(5,6)] integer :: i, j, uu(5,6) call print_poly(thing%member1) ! { dg-warning "array temporary" } call print_poly(y%member2) ! { dg-warning "array temporary" } call print_poly(y(1::2)%member2) ! { dg-warning "array temporary" } ! The following array sections work without temporaries uu = reshape([(((10*i+j),i=1,5),j=1,6)],[5,6]) print *, uu(2,2::2) call print_poly (uu(2,2::2)) ! no temp needed! print *, uu(1::2,6) call print_poly (uu(1::2,6)) ! no temp needed! end subroutine do_print subroutine do_print2(thing2) class(foo), dimension(:), intent(in) :: thing2 call print_poly (thing2% member2) ! { dg-warning "array temporary" } end subroutine do_print2 subroutine do_print3 () character(3) :: c(3) = ["abc","def","ghi"] call print_poly (c(1::2)) ! no temp needed! call print_poly (c(1::2)(2:3)) ! { dg-warning "array temporary" } end subroutine do_print3 end module test_PR105658_mod If you like, you can repackage the patch and sign it (see https://gcc.gnu.org/dco.html), and one of us will then commit it for you. Thanks! Harald ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] 2024-02-16 21:19 ` Harald Anlauf @ 2024-02-16 21:19 ` Harald Anlauf 2024-02-19 15:19 ` Peter Hill 1 sibling, 0 replies; 9+ messages in thread From: Harald Anlauf @ 2024-02-16 21:19 UTC (permalink / raw) To: gcc-patches; +Cc: fortran Hi Peter, thanks for your contribution to gfortran! You've found indeed a solution for a potentially annoying bug. Am 15.02.24 um 18:50 schrieb Peter Hill: > Dear all, > > The attached patch fixes PR105658 by forcing an array temporary to be > created. This is required when passing an array component, but this > didn't happen if the dummy argument was an unlimited polymorphic type. > > The problem bit of code is in `gfc_conv_expr_descriptor`, near L7828: > > subref_array_target = (is_subref_array (expr) > && (se->direct_byref > || expr->ts.type == BT_CHARACTER)); > need_tmp = (gfc_ref_needs_temporary_p (expr->ref) > && !subref_array_target); > > where `need_tmp` is being evaluated to 0. The logic here isn't clear > to me, and this function is used in several places, which is why I > went with setting `parmse.force_tmp = 1` in `gfc_conv_procedure_call` > and using the same conditional as the later branch for the > non-polymorphic case (near the call to `gfc_conv_subref_array_arg`) > > If this patch is ok, please could someone commit it for me? This is my > first patch for GCC, so apologies in advance if the commit message is > missing something. Your patch mostly does the right thing. Note that when fsym is an unlimited polymorphic, some of its attributes are buried deep within its internal representation. I would also prefer to move the code to gfc_conv_intrinsic_to_class where it seems to fit better, like: diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index a0593b76f18..db906caa52e 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -1019,6 +1019,14 @@ gfc_conv_intrinsic_to_class (gfc_se *parmse, gfc_expr *e, tmp = gfc_typenode_for_spec (&class_ts); var = gfc_create_var (tmp, "class"); + /* Force a temporary for component or substring references. */ + if (unlimited_poly + && class_ts.u.derived->components->attr.dimension + && !class_ts.u.derived->components->attr.class_pointer + && !class_ts.u.derived->components->attr.allocatable + && is_subref_array (e)) + parmse->force_tmp = 1; + /* Set the vptr. */ ctree = gfc_class_vptr_get (var); (I am not entirely sure whether we need to exclude pointer and allocatable attributes here explicitly, given the constraints in F2023:15.5.2.6, but other may have an opinion, too. The above should be safe anyway.) > Tested on x86_64-pc-linux-gnu. > > The bug is present in gfortran back to 4.9, so should it also be backported? I think we'll target 14-mainline and might consider a backport to 13-branch. > Cheers, > Peter > > PR fortran/105658 > > gcc/fortran/ChangeLog > > * trans-expr.cc (gfc_conv_procedure_call): When passing an > array component reference of intrinsic type to a procedure > with an unlimited polymorphic dummy argument, a temporary > should be created. > > gcc/testsuite/ChangeLog > > * gfortran.dg/PR105658.f90: New test. > --- > gcc/fortran/trans-expr.cc | 8 ++++++++ > gcc/testsuite/gfortran.dg/PR105658.f90 | 25 +++++++++++++++++++++++++ > 2 files changed, 33 insertions(+) > create mode 100644 gcc/testsuite/gfortran.dg/PR105658.f90 > > diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc > index a0593b76f18..7fd3047c4e9 100644 > --- a/gcc/fortran/trans-expr.cc > +++ b/gcc/fortran/trans-expr.cc > @@ -6439,6 +6439,14 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, > CLASS object for the unlimited polymorphic formal. */ > gfc_find_vtab (&e->ts); > gfc_init_se (&parmse, se); > + /* The actual argument is a component reference to an array > + of derived types, so we need to force creation of a > + temporary */ > + if (e->expr_type == EXPR_VARIABLE > + && is_subref_array (e) > + && !(fsym && fsym->attr.pointer)) > + parmse.force_tmp = 1; > + > gfc_conv_intrinsic_to_class (&parmse, e, fsym->ts); > > } > diff --git a/gcc/testsuite/gfortran.dg/PR105658.f90 > b/gcc/testsuite/gfortran.dg/PR105658.f90 > new file mode 100644 > index 00000000000..407ee25f77c > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/PR105658.f90 > @@ -0,0 +1,25 @@ > +! { dg-do compile } > +! { dg-options "-Warray-temporaries" } > +! Test fix for incorrectly passing array component to unlimited > polymorphic procedure > + > +module test_PR105658_mod > + implicit none > + type :: foo > + integer :: member1 > + integer :: member2 > + end type foo > +contains > + subroutine print_poly(array) > + class(*), dimension(:), intent(in) :: array > + select type(array) > + type is (integer) > + print*, array > + end select > + end subroutine print_poly > + > + subroutine do_print(thing) > + type(foo), dimension(3), intent(in) :: thing > + call print_poly(thing%member1) ! { dg-warning "array temporary" } > + end subroutine do_print > + > +end module test_PR105658_mod One could extend this testcase to cover substrings as well: module test_PR105658_mod implicit none type :: foo integer :: member1 integer :: member2 end type foo contains subroutine print_poly(array) class(*), dimension(:), intent(in) :: array select type(array) type is (integer) print*, array type is (character(*)) print *, array end select end subroutine print_poly subroutine do_print(thing) type(foo), dimension(3), intent(in) :: thing type(foo), parameter :: y(3) = [foo(1,2),foo(3,4),foo(5,6)] integer :: i, j, uu(5,6) call print_poly(thing%member1) ! { dg-warning "array temporary" } call print_poly(y%member2) ! { dg-warning "array temporary" } call print_poly(y(1::2)%member2) ! { dg-warning "array temporary" } ! The following array sections work without temporaries uu = reshape([(((10*i+j),i=1,5),j=1,6)],[5,6]) print *, uu(2,2::2) call print_poly (uu(2,2::2)) ! no temp needed! print *, uu(1::2,6) call print_poly (uu(1::2,6)) ! no temp needed! end subroutine do_print subroutine do_print2(thing2) class(foo), dimension(:), intent(in) :: thing2 call print_poly (thing2% member2) ! { dg-warning "array temporary" } end subroutine do_print2 subroutine do_print3 () character(3) :: c(3) = ["abc","def","ghi"] call print_poly (c(1::2)) ! no temp needed! call print_poly (c(1::2)(2:3)) ! { dg-warning "array temporary" } end subroutine do_print3 end module test_PR105658_mod If you like, you can repackage the patch and sign it (see https://gcc.gnu.org/dco.html), and one of us will then commit it for you. Thanks! Harald ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] 2024-02-16 21:19 ` Harald Anlauf 2024-02-16 21:19 ` Harald Anlauf @ 2024-02-19 15:19 ` Peter Hill 2024-02-19 19:52 ` Harald Anlauf 2024-02-20 19:53 ` Harald Anlauf 1 sibling, 2 replies; 9+ messages in thread From: Peter Hill @ 2024-02-19 15:19 UTC (permalink / raw) To: Harald Anlauf; +Cc: gcc-patches, fortran Hi Harald, Thanks for your help, please see the updated and signed-off patch below. > (I am not entirely sure whether we need to exclude pointer and > allocatable attributes here explicitly, given the constraints > in F2023:15.5.2.6, but other may have an opinion, too. > The above should be safe anyway.) I've included them in the patch here, but it does seem to work fine without checking those attributes here -- and invalid code is still caught with that change. It also occurred to me that array temporaries aren't _required_ here (for arrays of derived type components), but in the general case with a type with differently sized components, the stride wouldn't be a multiple of the component's type's size. Is it possible in principle to have an arbitrary stride? Cheers, Peter From 907a104facfc7f35f48ebcfa9ef5f8f5430d4d3c Mon Sep 17 00:00:00 2001 From: Peter Hill <peter.hill@york.ac.uk> Date: Thu, 15 Feb 2024 16:58:33 +0000 Subject: [PATCH] Fortran: fix passing array component ref to polymorphic procedures PR fortran/105658 gcc/fortran/ChangeLog * trans-expr.cc (gfc_conv_intrinsic_to_class): When passing an array component reference of intrinsic type to a procedure with an unlimited polymorphic dummy argument, a temporary should be created. gcc/testsuite/ChangeLog * gfortran.dg/PR105658.f90: New test. Signed-off-by: Peter Hill <peter.hill@york.ac.uk> --- gcc/fortran/trans-expr.cc | 9 +++++ gcc/testsuite/gfortran.dg/PR105658.f90 | 50 ++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/PR105658.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index a0593b76f18..004081aa6c3 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -1019,6 +1019,14 @@ gfc_conv_intrinsic_to_class (gfc_se *parmse, gfc_expr *e, tmp = gfc_typenode_for_spec (&class_ts); var = gfc_create_var (tmp, "class"); + /* Force a temporary for component or substring references */ + if (unlimited_poly + && class_ts.u.derived->components->attr.dimension + && !class_ts.u.derived->components->attr.allocatable + && !class_ts.u.derived->components->attr.class_pointer + && is_subref_array (e)) + parmse->force_tmp = 1; + /* Set the vptr. */ ctree = gfc_class_vptr_get (var); @@ -6439,6 +6447,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, CLASS object for the unlimited polymorphic formal. */ gfc_find_vtab (&e->ts); gfc_init_se (&parmse, se); + gfc_conv_intrinsic_to_class (&parmse, e, fsym->ts); } diff --git a/gcc/testsuite/gfortran.dg/PR105658.f90 b/gcc/testsuite/gfortran.dg/PR105658.f90 new file mode 100644 index 00000000000..8aacecf806e --- /dev/null +++ b/gcc/testsuite/gfortran.dg/PR105658.f90 @@ -0,0 +1,50 @@ +! { dg-do compile } +! { dg-options "-Warray-temporaries" } +! Test fix for incorrectly passing array component to unlimited polymorphic procedure + +module test_PR105658_mod + implicit none + type :: foo + integer :: member1 + integer :: member2 + end type foo +contains + subroutine print_poly(array) + class(*), dimension(:), intent(in) :: array + select type(array) + type is (integer) + print*, array + type is (character(*)) + print *, array + end select + end subroutine print_poly + + subroutine do_print(thing) + type(foo), dimension(3), intent(in) :: thing + type(foo), parameter :: y(3) = [foo(1,2),foo(3,4),foo(5,6)] + integer :: i, j, uu(5,6) + + call print_poly(thing%member1) ! { dg-warning "array temporary" } + call print_poly(y%member2) ! { dg-warning "array temporary" } + call print_poly(y(1::2)%member2) ! { dg-warning "array temporary" } + + ! The following array sections work without temporaries + uu = reshape([(((10*i+j),i=1,5),j=1,6)],[5,6]) + print *, uu(2,2::2) + call print_poly (uu(2,2::2)) ! no temp needed! + print *, uu(1::2,6) + call print_poly (uu(1::2,6)) ! no temp needed! + end subroutine do_print + + subroutine do_print2(thing2) + class(foo), dimension(:), intent(in) :: thing2 + call print_poly (thing2% member2) ! { dg-warning "array temporary" } + end subroutine do_print2 + + subroutine do_print3 () + character(3) :: c(3) = ["abc","def","ghi"] + call print_poly (c(1::2)) ! no temp needed! + call print_poly (c(1::2)(2:3)) ! { dg-warning "array temporary" } + end subroutine do_print3 + +end module test_PR105658_mod -- 2.43.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] 2024-02-19 15:19 ` Peter Hill @ 2024-02-19 19:52 ` Harald Anlauf 2024-02-19 19:52 ` Harald Anlauf 2024-02-20 19:53 ` Harald Anlauf 1 sibling, 1 reply; 9+ messages in thread From: Harald Anlauf @ 2024-02-19 19:52 UTC (permalink / raw) To: Peter Hill; +Cc: gcc-patches, fortran Hi Peter, On 2/19/24 16:19, Peter Hill wrote: > Hi Harald, > > Thanks for your help, please see the updated and signed-off patch below. great! This is fine, and I'll commit it tomorrow unless others have further comments. > It also occurred to me that array temporaries aren't _required_ here > (for arrays of derived type components), but in the general case with > a type with differently sized components, the stride wouldn't be a > multiple of the component's type's size. Is it possible in principle > to have an arbitrary stride? It is possible to have an arbitrary (fixed, non-unit) stride, but it is not always taken advantage of. If you take the last version of the testcase and compile with option -fdump-tree-original, you can see that the cases commented with "no temp needed" actually create a suitable descriptor. E.g. call print_poly (uu(2,2::2)) becomes: { struct __class__STAR_1_0t class.28; struct array01_integer(kind=4) parm.29; class.28._vptr = (struct __vtype__STAR * {ref-all}) &__vtab_INTEGER_4_; parm.29.span = 4; parm.29.dtype = {.elem_len=4, .version=0, .rank=1, .type=1}; parm.29.dim[0].lbound = 1; parm.29.dim[0].ubound = 3; parm.29.dim[0].stride = 10; parm.29.data = (void *) &uu[6]; parm.29.offset = -10; class.28._data = parm.29; class.28._len = 0; print_poly (&class.28); } Since we know that 'uu' is a contiguous array, we can calculate the stride (10) for the 1-d section. The case of the section of the character array is quite similar, but the variant with the substring reference would need further work to avoid the temporary. (It would be possible.) But as you say, the general case, which may involve types/classes, does not map to a simple descriptor. Thanks for your patch! Harald ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] 2024-02-19 19:52 ` Harald Anlauf @ 2024-02-19 19:52 ` Harald Anlauf 0 siblings, 0 replies; 9+ messages in thread From: Harald Anlauf @ 2024-02-19 19:52 UTC (permalink / raw) To: gcc-patches; +Cc: fortran Hi Peter, On 2/19/24 16:19, Peter Hill wrote: > Hi Harald, > > Thanks for your help, please see the updated and signed-off patch below. great! This is fine, and I'll commit it tomorrow unless others have further comments. > It also occurred to me that array temporaries aren't _required_ here > (for arrays of derived type components), but in the general case with > a type with differently sized components, the stride wouldn't be a > multiple of the component's type's size. Is it possible in principle > to have an arbitrary stride? It is possible to have an arbitrary (fixed, non-unit) stride, but it is not always taken advantage of. If you take the last version of the testcase and compile with option -fdump-tree-original, you can see that the cases commented with "no temp needed" actually create a suitable descriptor. E.g. call print_poly (uu(2,2::2)) becomes: { struct __class__STAR_1_0t class.28; struct array01_integer(kind=4) parm.29; class.28._vptr = (struct __vtype__STAR * {ref-all}) &__vtab_INTEGER_4_; parm.29.span = 4; parm.29.dtype = {.elem_len=4, .version=0, .rank=1, .type=1}; parm.29.dim[0].lbound = 1; parm.29.dim[0].ubound = 3; parm.29.dim[0].stride = 10; parm.29.data = (void *) &uu[6]; parm.29.offset = -10; class.28._data = parm.29; class.28._len = 0; print_poly (&class.28); } Since we know that 'uu' is a contiguous array, we can calculate the stride (10) for the 1-d section. The case of the section of the character array is quite similar, but the variant with the substring reference would need further work to avoid the temporary. (It would be possible.) But as you say, the general case, which may involve types/classes, does not map to a simple descriptor. Thanks for your patch! Harald ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] 2024-02-19 15:19 ` Peter Hill 2024-02-19 19:52 ` Harald Anlauf @ 2024-02-20 19:53 ` Harald Anlauf 2024-02-20 19:53 ` Harald Anlauf 2024-02-20 20:09 ` Steve Kargl 1 sibling, 2 replies; 9+ messages in thread From: Harald Anlauf @ 2024-02-20 19:53 UTC (permalink / raw) To: Peter Hill; +Cc: gcc-patches, fortran On 2/19/24 16:19, Peter Hill wrote: > Hi Harald, > > Thanks for your help, please see the updated and signed-off patch below. Pushed: https://gcc.gnu.org/g:14ba8d5b87acd5f91ab8b8c02165a0fd53dcc2f2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] 2024-02-20 19:53 ` Harald Anlauf @ 2024-02-20 19:53 ` Harald Anlauf 2024-02-20 20:09 ` Steve Kargl 1 sibling, 0 replies; 9+ messages in thread From: Harald Anlauf @ 2024-02-20 19:53 UTC (permalink / raw) To: gcc-patches; +Cc: fortran On 2/19/24 16:19, Peter Hill wrote: > Hi Harald, > > Thanks for your help, please see the updated and signed-off patch below. Pushed: https://gcc.gnu.org/g:14ba8d5b87acd5f91ab8b8c02165a0fd53dcc2f2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] 2024-02-20 19:53 ` Harald Anlauf 2024-02-20 19:53 ` Harald Anlauf @ 2024-02-20 20:09 ` Steve Kargl 1 sibling, 0 replies; 9+ messages in thread From: Steve Kargl @ 2024-02-20 20:09 UTC (permalink / raw) To: Harald Anlauf; +Cc: Peter Hill, gcc-patches, fortran On Tue, Feb 20, 2024 at 08:53:37PM +0100, Harald Anlauf wrote: > On 2/19/24 16:19, Peter Hill wrote: > > Hi Harald, > > > > Thanks for your help, please see the updated and signed-off patch below. > > Pushed: https://gcc.gnu.org/g:14ba8d5b87acd5f91ab8b8c02165a0fd53dcc2f2 > Harald, Thanks for taking care of this commit. Peter, welcome to the menagerie. -- Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-02-20 20:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-02-15 17:50 [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658] Peter Hill 2024-02-16 21:19 ` Harald Anlauf 2024-02-16 21:19 ` Harald Anlauf 2024-02-19 15:19 ` Peter Hill 2024-02-19 19:52 ` Harald Anlauf 2024-02-19 19:52 ` Harald Anlauf 2024-02-20 19:53 ` Harald Anlauf 2024-02-20 19:53 ` Harald Anlauf 2024-02-20 20:09 ` Steve Kargl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).