Re: [Committed, Fortran, Patch, v1] Three small patches for character arrays

[Andre Vehreschild]

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 (, );
 
   /* 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 (_se, NULL);
   if (lhs_expr->rank == 0)
 {
-  symbol_attribute attr;
-  gfc_clear_attr ();
-  gfc_conv_expr (_se, lhs_expr);
-  lhs_type = TREE_TYPE (lhs_se.expr);
-  lhs_se.expr = gfc_conv_scalar_to_descriptor (_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 ();
+	  gfc_conv_expr (_se, lhs_expr);
+	  lhs_type = TREE_TYPE (lhs_se.expr);
+	  lhs_se.expr = gfc_conv_scalar_to_descriptor (_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))),
+			  

Re: [Ping, Fortran, Patch, v1] Three small patches for character arrays

[Paul Richard Thomas]

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

[Andre Vehreschild]

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 (, );
 
   /* 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 (_se, NULL);
   if (lhs_expr->rank == 0)
 {
-  symbol_attribute attr;
-  gfc_clear_attr ();
-  gfc_conv_expr (_se, lhs_expr);
-  lhs_type = TREE_TYPE (lhs_se.expr);
-  lhs_se.expr = gfc_conv_scalar_to_descriptor (_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 ();
+	  gfc_conv_expr (_se, lhs_expr);
+	  lhs_type = TREE_TYPE (lhs_se.expr);
+	  lhs_se.expr = gfc_conv_scalar_to_descriptor (_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, 

[Fortran, Patch, v1] Three small patches for character arrays

[Andre Vehreschild]

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 (, );
 
   /* 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 (_se, NULL);
   if (lhs_expr->rank == 0)
 {
-  symbol_attribute attr;
-  gfc_clear_attr ();
-  gfc_conv_expr (_se, lhs_expr);
-  lhs_type = TREE_TYPE (lhs_se.expr);
-  lhs_se.expr = gfc_conv_scalar_to_descriptor (_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 ();
+	  gfc_conv_expr (_se, lhs_expr);
+	  lhs_type = TREE_TYPE (lhs_se.expr);
+	  lhs_se.expr = gfc_conv_scalar_to_descriptor (_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=:),