Re: [Committed, Fortran, Patch, v1] Three small patches for character arrays
Hi Paul, thanks for the review. Committed as r254407 to trunk and r254408 to gcc-7. Regards, Andre On Sat, 4 Nov 2017 12:06:15 + Paul Richard Thomas wrote: > Hi Andre, > > These patches look fine to me. Thanks for taking account of the array > descriptor change between trunk and 7-branch :-) > > OK for trunk and 7-branch. > > Thanks > > Paul > > > On 4 November 2017 at 10:27, Andre Vehreschild wrote: > > Ping! > > > > On Wed, 1 Nov 2017 12:27:11 +0100 > > Andre Vehreschild wrote: > > > >> Hi all, > >> > >> during development on OpenCoarrays I found these three issues in gfortran: > >> > >> - caf_send: Did not treat char arrays as arrays when calling caf_send. > >> - gfc_trans_assignment_1: Conversion of character kind creates a loop > >> variant temporary which must not be put before the loop body, but within. > >> - get_array_span: When doing pointer arithmetic on char arrays the > >> character kind was not applied. > >> > >> The attached patch fixes all of these issues for trunk and gcc-7. > >> Bootstrapped and regtested on x86_64-linux-gnu/f25. Ok for trunk and gcc-7? > >> > >> Regards, > >> Andre > > > > > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > > > -- Andre Vehreschild * Email: vehre ad gmx dot de Index: gcc/fortran/ChangeLog === --- gcc/fortran/ChangeLog (Revision 254406) +++ gcc/fortran/ChangeLog (Arbeitskopie) @@ -1,3 +1,12 @@ +2017-11-04 Andre Vehreschild + + * trans-expr.c (gfc_trans_assignment_1): Character kind conversion may + create a loop variant temporary, too. + * trans-intrinsic.c (conv_caf_send): Treat char arrays as arrays and + not as scalars. + * trans.c (get_array_span): Take the character kind into account when + doing pointer arithmetic. + 2017-11-04 Thomas Koenig PR fortran/29600 Index: gcc/fortran/trans-expr.c === --- gcc/fortran/trans-expr.c (Revision 254406) +++ gcc/fortran/trans-expr.c (Arbeitskopie) @@ -10084,12 +10084,16 @@ NOTE: This relies on having the exact dependence of the length type parameter available to the caller; gfortran saves it in the .mod files. NOTE ALSO: The concatenation operation generates a temporary pointer, - whose allocation must go to the innermost loop. */ + whose allocation must go to the innermost loop. + NOTE ALSO (2): A character conversion may generate a temporary, too. */ if (flag_realloc_lhs && expr2->ts.type == BT_CHARACTER && expr1->ts.deferred && !(lss != gfc_ss_terminator - && expr2->expr_type == EXPR_OP - && expr2->value.op.op == INTRINSIC_CONCAT)) + && ((expr2->expr_type == EXPR_OP + && expr2->value.op.op == INTRINSIC_CONCAT) + || (expr2->expr_type == EXPR_FUNCTION + && expr2->value.function.isym != NULL + && expr2->value.function.isym->id == GFC_ISYM_CONVERSION gfc_add_block_to_block (&block, &rse.pre); /* Nullify the allocatable components corresponding to those of the lhs Index: gcc/fortran/trans-intrinsic.c === --- gcc/fortran/trans-intrinsic.c (Revision 254406) +++ gcc/fortran/trans-intrinsic.c (Arbeitskopie) @@ -1871,12 +1871,21 @@ gfc_init_se (&lhs_se, NULL); if (lhs_expr->rank == 0) { - symbol_attribute attr; - gfc_clear_attr (&attr); - gfc_conv_expr (&lhs_se, lhs_expr); - lhs_type = TREE_TYPE (lhs_se.expr); - lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr, attr); - lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr); + if (lhs_expr->ts.type == BT_CHARACTER && lhs_expr->ts.deferred) + { + lhs_se.expr = gfc_get_tree_for_caf_expr (lhs_expr); + lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr); + } + else + { + symbol_attribute attr; + gfc_clear_attr (&attr); + gfc_conv_expr (&lhs_se, lhs_expr); + lhs_type = TREE_TYPE (lhs_se.expr); + lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr, + attr); + lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr); + } } else if ((lhs_caf_attr.alloc_comp || lhs_caf_attr.pointer_comp) && lhs_caf_attr.codimension) Index: gcc/fortran/trans.c === --- gcc/fortran/trans.c (Revision 254406) +++ gcc/fortran/trans.c (Arbeitskopie) @@ -320,8 +320,12 @@ || DECL_CONTEXT (TYPE_MAX_VALUE (TYPE_DOMAIN (type))) == DECL_CONTEXT (decl))) { - span = TYPE_MAX_VALUE (TYPE_DOMAIN (type)); - span = fold_convert (gfc_array_index_type, span); + span = fold_convert (gfc_array_index_type, + TYPE_MAX_VALUE (TYPE_DOMAIN (type))); + span = fold_build2 (MULT_EXPR, gfc_array_index_type, + fold_convert (gfc_array_index_type, + TYPE_SIZE_UNIT (TREE_TYPE (type))), + span); } /* Likewise for class array or pointer array
Re: [Ping, Fortran, Patch, v1] Three small patches for character arrays
Hi Andre, These patches look fine to me. Thanks for taking account of the array descriptor change between trunk and 7-branch :-) OK for trunk and 7-branch. Thanks Paul On 4 November 2017 at 10:27, Andre Vehreschild wrote: > Ping! > > On Wed, 1 Nov 2017 12:27:11 +0100 > Andre Vehreschild wrote: > >> Hi all, >> >> during development on OpenCoarrays I found these three issues in gfortran: >> >> - caf_send: Did not treat char arrays as arrays when calling caf_send. >> - gfc_trans_assignment_1: Conversion of character kind creates a loop variant >> temporary which must not be put before the loop body, but within. >> - get_array_span: When doing pointer arithmetic on char arrays the character >> kind was not applied. >> >> The attached patch fixes all of these issues for trunk and gcc-7. >> Bootstrapped >> and regtested on x86_64-linux-gnu/f25. Ok for trunk and gcc-7? >> >> Regards, >> Andre > > > -- > Andre Vehreschild * Email: vehre ad gmx dot de -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Ping, Fortran, Patch, v1] Three small patches for character arrays
Ping! On Wed, 1 Nov 2017 12:27:11 +0100 Andre Vehreschild wrote: > Hi all, > > during development on OpenCoarrays I found these three issues in gfortran: > > - caf_send: Did not treat char arrays as arrays when calling caf_send. > - gfc_trans_assignment_1: Conversion of character kind creates a loop variant > temporary which must not be put before the loop body, but within. > - get_array_span: When doing pointer arithmetic on char arrays the character > kind was not applied. > > The attached patch fixes all of these issues for trunk and gcc-7. Bootstrapped > and regtested on x86_64-linux-gnu/f25. Ok for trunk and gcc-7? > > Regards, > Andre -- Andre Vehreschild * Email: vehre ad gmx dot de gcc/fortran/ChangeLog: 2017-11-01 Andre Vehreschild * trans-expr.c (gfc_trans_assignment_1): Character kind conversion may create a loop variant temporary, too. * trans-intrinsic.c (conv_caf_send): Treat char arrays as arrays and not as scalars. * trans.c (get_array_span): Take the character kind into account when doing pointer arithmetic. gcc/testsuite/ChangeLog: 2017-11-01 Andre Vehreschild * gfortran.dg/coarray/send_char_array_1.f90: New test. diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index 1a3e3d4..57b62a6 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -10084,12 +10084,16 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, NOTE: This relies on having the exact dependence of the length type parameter available to the caller; gfortran saves it in the .mod files. NOTE ALSO: The concatenation operation generates a temporary pointer, - whose allocation must go to the innermost loop. */ + whose allocation must go to the innermost loop. + NOTE ALSO (2): A character conversion may generate a temporary, too. */ if (flag_realloc_lhs && expr2->ts.type == BT_CHARACTER && expr1->ts.deferred && !(lss != gfc_ss_terminator - && expr2->expr_type == EXPR_OP - && expr2->value.op.op == INTRINSIC_CONCAT)) + && ((expr2->expr_type == EXPR_OP + && expr2->value.op.op == INTRINSIC_CONCAT) + || (expr2->expr_type == EXPR_FUNCTION + && expr2->value.function.isym != NULL + && expr2->value.function.isym->id == GFC_ISYM_CONVERSION gfc_add_block_to_block (&block, &rse.pre); /* Nullify the allocatable components corresponding to those of the lhs diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 532d3ab..b0f0ab2 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -1871,12 +1871,21 @@ conv_caf_send (gfc_code *code) { gfc_init_se (&lhs_se, NULL); if (lhs_expr->rank == 0) { - symbol_attribute attr; - gfc_clear_attr (&attr); - gfc_conv_expr (&lhs_se, lhs_expr); - lhs_type = TREE_TYPE (lhs_se.expr); - lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr, attr); - lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr); + if (lhs_expr->ts.type == BT_CHARACTER && lhs_expr->ts.deferred) + { + lhs_se.expr = gfc_get_tree_for_caf_expr (lhs_expr); + lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr); + } + else + { + symbol_attribute attr; + gfc_clear_attr (&attr); + gfc_conv_expr (&lhs_se, lhs_expr); + lhs_type = TREE_TYPE (lhs_se.expr); + lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr, + attr); + lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr); + } } else if ((lhs_caf_attr.alloc_comp || lhs_caf_attr.pointer_comp) && lhs_caf_attr.codimension) diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c index 53bc428..4115308 100644 --- a/gcc/fortran/trans.c +++ b/gcc/fortran/trans.c @@ -320,8 +320,12 @@ get_array_span (tree type, tree decl) || DECL_CONTEXT (TYPE_MAX_VALUE (TYPE_DOMAIN (type))) == DECL_CONTEXT (decl))) { - span = TYPE_MAX_VALUE (TYPE_DOMAIN (type)); - span = fold_convert (gfc_array_index_type, span); + span = fold_convert (gfc_array_index_type, + TYPE_MAX_VALUE (TYPE_DOMAIN (type))); + span = fold_build2 (MULT_EXPR, gfc_array_index_type, + fold_convert (gfc_array_index_type, + TYPE_SIZE_UNIT (TREE_TYPE (type))), + span); } /* Likewise for class array or pointer array references. */ else if (TREE_CODE (decl) == FIELD_DECL diff --git a/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 b/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 new file mode 100644 index 000..800a8ac --- /dev/null +++ b/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 @@ -0,0 +1,54 @@ +!{ dg-do run } + +program send_convert_char_array + + implicit none + + character(kind=1, len=:), allocatable, codimension[:] :: co_str_k1_scal + character(kind=1, len=:), allocatable :: str_k1_scal + character(kind=4, len=:), allocatable, codimension[:] ::
[Fortran, Patch, v1] Three small patches for character arrays
Hi all, during development on OpenCoarrays I found these three issues in gfortran: - caf_send: Did not treat char arrays as arrays when calling caf_send. - gfc_trans_assignment_1: Conversion of character kind creates a loop variant temporary which must not be put before the loop body, but within. - get_array_span: When doing pointer arithmetic on char arrays the character kind was not applied. The attached patch fixes all of these issues for trunk and gcc-7. Bootstrapped and regtested on x86_64-linux-gnu/f25. Ok for trunk and gcc-7? Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de gcc/fortran/ChangeLog: 2017-11-01 Andre Vehreschild * trans-expr.c (gfc_trans_assignment_1): Character kind conversion may create a loop variant temporary, too. * trans-intrinsic.c (conv_caf_send): Treat char arrays as arrays and not as scalars. * trans.c (get_array_span): Take the character kind into account when doing pointer arithmetic. gcc/testsuite/ChangeLog: 2017-11-01 Andre Vehreschild * gfortran.dg/coarray/send_char_array_1.f90: New test. diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index 1a3e3d4..57b62a6 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -10084,12 +10084,16 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, NOTE: This relies on having the exact dependence of the length type parameter available to the caller; gfortran saves it in the .mod files. NOTE ALSO: The concatenation operation generates a temporary pointer, - whose allocation must go to the innermost loop. */ + whose allocation must go to the innermost loop. + NOTE ALSO (2): A character conversion may generate a temporary, too. */ if (flag_realloc_lhs && expr2->ts.type == BT_CHARACTER && expr1->ts.deferred && !(lss != gfc_ss_terminator - && expr2->expr_type == EXPR_OP - && expr2->value.op.op == INTRINSIC_CONCAT)) + && ((expr2->expr_type == EXPR_OP + && expr2->value.op.op == INTRINSIC_CONCAT) + || (expr2->expr_type == EXPR_FUNCTION + && expr2->value.function.isym != NULL + && expr2->value.function.isym->id == GFC_ISYM_CONVERSION gfc_add_block_to_block (&block, &rse.pre); /* Nullify the allocatable components corresponding to those of the lhs diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 532d3ab..b0f0ab2 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -1871,12 +1871,21 @@ conv_caf_send (gfc_code *code) { gfc_init_se (&lhs_se, NULL); if (lhs_expr->rank == 0) { - symbol_attribute attr; - gfc_clear_attr (&attr); - gfc_conv_expr (&lhs_se, lhs_expr); - lhs_type = TREE_TYPE (lhs_se.expr); - lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr, attr); - lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr); + if (lhs_expr->ts.type == BT_CHARACTER && lhs_expr->ts.deferred) + { + lhs_se.expr = gfc_get_tree_for_caf_expr (lhs_expr); + lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr); + } + else + { + symbol_attribute attr; + gfc_clear_attr (&attr); + gfc_conv_expr (&lhs_se, lhs_expr); + lhs_type = TREE_TYPE (lhs_se.expr); + lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr, + attr); + lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr); + } } else if ((lhs_caf_attr.alloc_comp || lhs_caf_attr.pointer_comp) && lhs_caf_attr.codimension) diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c index 53bc428..4115308 100644 --- a/gcc/fortran/trans.c +++ b/gcc/fortran/trans.c @@ -320,8 +320,12 @@ get_array_span (tree type, tree decl) || DECL_CONTEXT (TYPE_MAX_VALUE (TYPE_DOMAIN (type))) == DECL_CONTEXT (decl))) { - span = TYPE_MAX_VALUE (TYPE_DOMAIN (type)); - span = fold_convert (gfc_array_index_type, span); + span = fold_convert (gfc_array_index_type, + TYPE_MAX_VALUE (TYPE_DOMAIN (type))); + span = fold_build2 (MULT_EXPR, gfc_array_index_type, + fold_convert (gfc_array_index_type, + TYPE_SIZE_UNIT (TREE_TYPE (type))), + span); } /* Likewise for class array or pointer array references. */ else if (TREE_CODE (decl) == FIELD_DECL diff --git a/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 b/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 new file mode 100644 index 000..800a8ac --- /dev/null +++ b/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 @@ -0,0 +1,54 @@ +!{ dg-do run } + +program send_convert_char_array + + implicit none + + character(kind=1, len=:), allocatable, codimension[:] :: co_str_k1_scal + character(kind=1, len=:), allocatable :: str_k1_scal + character(kind=4, len=:), allocatable, codimension[:] :: co_str_k4_scal + character(kind=4, len=:), allocatable :: str_k4_scal + + character(kind=1, len=