Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Tobias Burnus]

Hi,

Jakub wrote:
> So would the following be acceptable?  It will warn both for
> logical l
>   l = 4Hfoob
> and
>   l = transfer (42, .true.)

I think that would be sufficent. For completeness:
There is no warning given for
     b = transfer(i, b)
I think that is OK, but one could also give one as NAG f95 does:
  Warning: TRANSFER to LOGICAL might produce invalid value

I got the first replies to my c.l.f question,
http://groups.google.com/group/comp.lang.fortran/browse_thread/thread/cf874197dc460679
and reading Glen Herrmannsfeldt's and Richard Maine's reply,
there should be no problem:
- Holleriths were usually used with integer since real/logical
  are system dependent (e.g. normalization of REAL Holleriths)
- TRANSFER requires that the bit pattern matches
  (Richard: "I think to be a mistake"), but
  "even if you literally follow that requirement, there turn out to
   be gaping loopholes that pretty much prevent the user from being
   able to count on bit pattern preservation."
- Assignments, COMMON and EQUIVALENCE: No bit pattern needs to be
  preserved

Thus, your patch is OK from the standard and common practice point of
view.

       if (expr->representation.string)
	{
+	  if (TREE_CODE (tmp) == INTEGER_CST)
+	  else

I think there is no possibility to ever enter the else branch:
expr->representation.string only occurs for either Holleriths
or for TRANSFER of constants; I would go for a gcc_assert
instead.

The patch is OK from my side.


Brooks Moses wrote:
+		gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
+			     " at %L has undefined result", &expr->where);
> The warning could perhaps be edited a little, too, to reflect that the
> user isn't necessarily thinking of the input as an integer.  Maybe:
> "Cannot assign value with bitwise representation other than 0x0 or 0x1
> to LOGICAL at %L".

I find the original string clearer than especially the "0x0 or 0x1".

Tobias

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Jakub Jelinek]

Hi!

On Tue, Nov 11, 2008 at 11:34:29PM +0100, Tobias Burnus wrote:
> Brooks Moses wrote:
> +		gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
> +			     " at %L has undefined result", &expr->where);
> > The warning could perhaps be edited a little, too, to reflect that the
> > user isn't necessarily thinking of the input as an integer.  Maybe:
> > "Cannot assign value with bitwise representation other than 0x0 or 0x1
> > to LOGICAL at %L".
> 
> I find the original string clearer than especially the "0x0 or 0x1".

So, here is an updated patch, which
1) handles transfer (transfer (x, .false.), something) the same way
   as transfer (transfer (x, 0), something) (i.e. uses INTEGER_TYPE
   of the same mode as the BOOLEAN_TYPE that was used previously)
2) testcases have been updated
3) no checking for INTEGER_CST result from fold_buil1 (V_C_E, ...),
   it just uses integer_zerop and integer_onep.

The middle-end side hasn't changed.

Ok for trunk?

2008-11-11  Jakub Jelinek  <jakub@redhat.com>

	PR target/35366
	* fold-const.c (native_encode_string): New function.
	(native_encode_expr): Use it for STRING_CST.

	* trans-const.c (gfc_conv_constant_to_tree): Warn when
	converting an integer outside of LOGICAL's range to
	LOGICAL.
	* trans-intrinsic.c (gfc_conv_intrinsic_function,
	gfc_conv_intrinsic_array_transfer, gfc_conv_intrinsic_transfer):
	Use INTEGER_TYPE instead of BOOLEAN_TYPE for TRANSFER as
	argument of another TRANSFER.

	* gfortran.dg/hollerith.f90: Don't assume a 32-bit value
	stored into logical variable will be preserved.
	* gfortran.dg/transfer_simplify_4.f90: Remove undefined
	cases.  Run at all optimization levels.  Add a couple of
	new tests.
	* gfortran.dg/hollerith5.f90: New test.
	* gfortran.dg/hollerith_legacy.f90: Add dg-warning.

--- gcc/fold-const.c.jj	2008-11-12 00:43:54.000000000 +0100
+++ gcc/fold-const.c	2008-11-12 11:09:40.000000000 +0100
@@ -7315,6 +7315,37 @@ native_encode_vector (const_tree expr, u
 }
 
 
+/* Subroutine of native_encode_expr.  Encode the STRING_CST
+   specified by EXPR into the buffer PTR of length LEN bytes.
+   Return the number of bytes placed in the buffer, or zero
+   upon failure.  */
+
+static int
+native_encode_string (const_tree expr, unsigned char *ptr, int len)
+{
+  tree type = TREE_TYPE (expr);
+  HOST_WIDE_INT total_bytes;
+
+  if (TREE_CODE (type) != ARRAY_TYPE
+      || TREE_CODE (TREE_TYPE (type)) != INTEGER_TYPE
+      || GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (type))) != BITS_PER_UNIT
+      || !host_integerp (TYPE_SIZE_UNIT (type), 0))
+    return 0;
+  total_bytes = tree_low_cst (TYPE_SIZE_UNIT (type), 0);
+  if (total_bytes > len)
+    return 0;
+  if (TREE_STRING_LENGTH (expr) < total_bytes)
+    {
+      memcpy (ptr, TREE_STRING_POINTER (expr), TREE_STRING_LENGTH (expr));
+      memset (ptr + TREE_STRING_LENGTH (expr), 0,
+	      total_bytes - TREE_STRING_LENGTH (expr));
+    }
+  else
+    memcpy (ptr, TREE_STRING_POINTER (expr), total_bytes);
+  return total_bytes;
+}
+
+
 /* Subroutine of fold_view_convert_expr.  Encode the INTEGER_CST,
    REAL_CST, COMPLEX_CST or VECTOR_CST specified by EXPR into the
    buffer PTR of length LEN bytes.  Return the number of bytes
@@ -7337,6 +7368,9 @@ native_encode_expr (const_tree expr, uns
     case VECTOR_CST:
       return native_encode_vector (expr, ptr, len);
 
+    case STRING_CST:
+      return native_encode_string (expr, ptr, len);
+
     default:
       return 0;
     }
--- gcc/fortran/trans-intrinsic.c.jj	2008-11-12 00:43:54.000000000 +0100
+++ gcc/fortran/trans-intrinsic.c	2008-11-12 11:35:05.000000000 +0100
@@ -3707,6 +3707,14 @@ gfc_conv_intrinsic_array_transfer (gfc_s
       mold_type = gfc_get_element_type (TREE_TYPE (argse.expr));
     }
 
+  if (strcmp (expr->value.function.name, "__transfer_in_transfer") == 0)
+    {
+      /* If this TRANSFER is nested in another TRANSFER, use a type
+	 that preserves all bits.  */
+      if (arg->expr->ts.type == BT_LOGICAL)
+	mold_type = gfc_get_int_type (arg->expr->ts.kind);
+    }
+
   if (arg->expr->ts.type == BT_CHARACTER)
     {
       tmp = size_of_string_in_bytes (arg->expr->ts.kind, argse.string_length);
@@ -3835,6 +3843,13 @@ gfc_conv_intrinsic_transfer (gfc_se * se
 
   arg = arg->next;
   type = gfc_typenode_for_spec (&expr->ts);
+  if (strcmp (expr->value.function.name, "__transfer_in_transfer") == 0)
+    {
+      /* If this TRANSFER is nested in another TRANSFER, use a type
+	 that preserves all bits.  */
+      if (expr->ts.type == BT_LOGICAL)
+	type = gfc_get_int_type (expr->ts.kind);
+    }
 
   if (expr->ts.type == BT_CHARACTER)
     {
@@ -4750,20 +4765,30 @@ gfc_conv_intrinsic_function (gfc_se * se
       break;
 
     case GFC_ISYM_TRANSFER:
-      if (se->ss)
+      if (se->ss && se->ss->useflags)
 	{
-	  if (se->ss->useflags)
-	    {
-	      /* Access the previously obtained result.  */
-	      gfc_conv_tmp_array_ref (se);
-	      gfc_advance_se_ss_chain (se);
-	      break;
-	    }
-	  else
-	    gfc_conv_intrinsic_array_transfer (se, expr);
+	  /* Access the previously obtained result.  */
+	  gfc_conv_tmp_array_ref (se);
+	  gfc_advance_se_ss_chain (se);
 	}
       else
-	gfc_conv_intrinsic_transfer (se, expr);
+	{
+	  /* Ensure double transfer through LOGICAL preserves all
+	     the needed bits.  */
+	  gfc_expr *source = expr->value.function.actual->expr;
+	  if (source->expr_type == EXPR_FUNCTION
+	      && source->value.function.esym == NULL
+	      && source->value.function.isym != NULL
+	      && source->value.function.isym->id == GFC_ISYM_TRANSFER
+	      && source->ts.type == BT_LOGICAL
+	      && expr->ts.type != source->ts.type)
+	    source->value.function.name = "__transfer_in_transfer";
+
+	  if (se->ss)
+	    gfc_conv_intrinsic_array_transfer (se, expr);
+	  else
+	    gfc_conv_intrinsic_transfer (se, expr);
+	}
       break;
 
     case GFC_ISYM_TTYNAM:
--- gcc/fortran/trans-const.c.jj	2008-11-12 00:43:54.000000000 +0100
+++ gcc/fortran/trans-const.c	2008-11-12 11:55:01.000000000 +0100
@@ -281,13 +281,19 @@ gfc_conv_constant_to_tree (gfc_expr * ex
 
     case BT_LOGICAL:
       if (expr->representation.string)
-	return fold_build1 (VIEW_CONVERT_EXPR,
-			    gfc_get_logical_type (expr->ts.kind),
-			    gfc_build_string_const (expr->representation.length,
-						    expr->representation.string));
+	{
+	  tree tmp = fold_build1 (VIEW_CONVERT_EXPR,
+				  gfc_get_int_type (expr->ts.kind),
+				  gfc_build_string_const (expr->representation.length,
+							  expr->representation.string));
+	  if (!integer_zerop (tmp) && !integer_onep (tmp))
+	    gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
+			 " has undefined result at %L", &expr->where);
+	  return fold_convert (gfc_get_logical_type (expr->ts.kind), tmp);
+	}
       else
 	return build_int_cst (gfc_get_logical_type (expr->ts.kind),
-			    expr->value.logical);
+			      expr->value.logical);
 
     case BT_COMPLEX:
       if (expr->representation.string)
--- gcc/testsuite/gfortran.dg/transfer_simplify_4.f90.jj	2008-11-12 00:43:54.000000000 +0100
+++ gcc/testsuite/gfortran.dg/transfer_simplify_4.f90	2008-11-12 12:30:46.000000000 +0100
@@ -1,30 +1,39 @@
 ! { dg-do run }
-! { dg-options "-O0" }
 ! Tests that the in-memory representation of a transferred variable
 ! propagates properly.
 !
   implicit none
 
   integer, parameter :: ip1 = 42
-  logical, parameter :: ap1 = transfer(ip1, .true.)
-  integer, parameter :: ip2 = transfer(ap1, 0)
+  integer, parameter :: ip2 = transfer(transfer(ip1, .true.), 0)
+  integer :: i, ai(4)
+  logical :: b
 
-  logical :: a
-  integer :: i
+  if (ip2 .ne. ip1) call abort ()
   
   i = transfer(transfer(ip1, .true.), 0)
   if (i .ne. ip1) call abort ()
 
-  i = transfer(ap1, 0)
-  if (i .ne. ip1) call abort ()
-  
-  a = transfer(ip1, .true.)
-  i = transfer(a, 0)
+  i = 42
+  i = transfer(transfer(i, .true.), 0)
   if (i .ne. ip1) call abort ()
 
-  i = ip1
-  a = transfer(i, .true.)
-  i = transfer(a, 0)
-  if (i .ne. ip1) call abort ()
+  b = transfer(transfer(.true., 3.1415), .true.)
+  if (.not.b) call abort ()
+
+  b = transfer(transfer(.false., 3.1415), .true.)
+  if (b) call abort ()
+
+  i = 0
+  b = transfer(i, .true.)
+  ! The standard doesn't guarantee here that b will be .false.,
+  ! though in gfortran for all targets it will.
+
+  ai = (/ 42, 42, 42, 42 /)
+  ai = transfer (transfer (ai, .false., 4), ai)
+  if (any(ai .ne. 42)) call abort
 
+  ai = transfer (transfer ((/ 42, 42, 42, 42 /), &
+&                          (/ .false., .false., .false., .false. /)), ai)
+  if (any(ai .ne. 42)) call abort
 end
--- gcc/testsuite/gfortran.dg/hollerith5.f90.jj	2008-11-12 12:34:26.000000000 +0100
+++ gcc/testsuite/gfortran.dg/hollerith5.f90	2008-11-12 12:40:54.000000000 +0100
@@ -0,0 +1,8 @@
+       ! { dg-do compile }
+       implicit none
+       logical b
+       b = 4Habcd ! { dg-warning "has undefined result" }
+       end
+
+! { dg-warning "Hollerith constant" "const" { target *-*-* } 4 }
+! { dg-warning "Conversion" "conversion" { target *-*-* } 4 }
--- gcc/testsuite/gfortran.dg/hollerith_legacy.f90.jj	2008-09-30 16:56:06.000000000 +0200
+++ gcc/testsuite/gfortran.dg/hollerith_legacy.f90	2008-11-12 12:41:47.000000000 +0100
@@ -21,13 +21,13 @@ data z2/4h(i7),'xxxx','xxxx','xxxx'/
 
 z2 (1,2) = 4h(i8)
 i = 4hHell
-l = 4Ho wo
+l = 4Ho wo	! { dg-warning "has undefined result" }
 r = 4Hrld! 
 write (line, '(3A4)') i, l, r
 if (line .ne. 'Hello world!') call abort
 i = 2Hab
 r = 2Hab
-l = 2Hab
+l = 2Hab	! { dg-warning "has undefined result" }
 c = 2Hab
 write (line, '(3A4, 8A)') i, l, r, c
 if (line .ne. 'ab  ab  ab  ab      ') call abort
--- gcc/testsuite/gfortran.dg/hollerith.f90.jj	2008-11-12 00:43:54.000000000 +0100
+++ gcc/testsuite/gfortran.dg/hollerith.f90	2008-11-12 12:39:24.000000000 +0100
@@ -8,7 +8,7 @@ character z1(4)
 character*4 z2(2,2)
 character*80 line
 integer i
-logical l
+integer j
 real r
 character*8 c
 
@@ -20,15 +20,15 @@ data z2/4h(i7),'xxxx','xxxx','xxxx'/
 
 z2 (1,2) = 4h(i8)
 i = 4hHell
-l = 4Ho wo
+j = 4Ho wo
 r = 4Hrld! 
-write (line, '(3A4)') i, l, r
+write (line, '(3A4)') i, j, r
 if (line .ne. 'Hello world!') call abort
 i = 2Hab
+j = 2Hab
 r = 2Hab
-l = 2Hab
 c = 2Hab
-write (line, '(3A4, 8A)') i, l, r, c
+write (line, '(3A4, 8A)') i, j, r, c
 if (line .ne. 'ab  ab  ab  ab      ') call abort
 
 write(line, '(4A8, "!")' ) x


	Jakub

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Paul Richard Thomas]

Jakub,

I presume that this clears PR33759 as well?

> Ok for trunk?

Yes indeed. Many thanks for the intestinal fortitude that you have displayed:-)

Cheers

Paul

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Feng Wang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=big5, Size: 1640 bytes --]

Hi Jakub, hi all,

--- Jakub Jelinek <jakub@redhat.com> wrote:

> Hi!
> 
> On Tue, Nov 11, 2008 at 11:34:29PM +0100, Tobias Burnus wrote:
> > Brooks Moses wrote:
> > +		gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
> > +			     " at %L has undefined result", &expr->where);
> > > The warning could perhaps be edited a little, too, to reflect that the
> > > user isn't necessarily thinking of the input as an integer.  Maybe:
> > > "Cannot assign value with bitwise representation other than 0x0 or 0x1
> > > to LOGICAL at %L".
> > 
> > I find the original string clearer than especially the "0x0 or 0x1".
> 
> So, here is an updated patch, which
> 1) handles transfer (transfer (x, .false.), something) the same way
>    as transfer (transfer (x, 0), something) (i.e. uses INTEGER_TYPE
>    of the same mode as the BOOLEAN_TYPE that was used previously)
> 2) testcases have been updated
> 3) no checking for INTEGER_CST result from fold_buil1 (V_C_E, ...),
>    it just uses integer_zerop and integer_onep.
> 
> The middle-end side hasn't changed.

I agree with the use of INTEGER_TYPE, but I don't think it is necessary to give
a warning message. After using INTEGER_TYPE, will other parts still change the
value? What's more, if the warning is given to the transfer and Hollerith
constants, what about the assignment of integer constants? For example:

logical l
l = 45

I cannot access my machine now, so I am not sure that whether this has been
dealt with in some other place.


Best Regards,
Feng Wang

_______________________________________
 »¶Ó­Ê¹Óó¬´óÈÝÁ¿ÑÅ»¢ÓÊÏä 
 http://cn.mail.yahoo.com 

[PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Jakub Jelinek]

Hi!

Fortran FE generates VIEW_CONVERT_EXPR of STRING_CST quite a lot, but
fold-const isn't able to fold it, so GCC generates very inefficient
and (in some cases also buggy, as can be seen on the equiv_7.f90
on powerpc64-darwin with -m64 -Os) code.

I've bootstrapped/regtested this on x86_64-linux.

Unfortrunately, it causes 2 Fortran testsuite failures, hollerith.f90
and transfer_simplify_4.f90, both at all optimization levels.
I believe the tests are just invalid though.  The first one does:
logical l
l = 4Ho wo
and expects all the 32 bits preserved, but that is certainly against
the semantics of BOOLEAN_TYPE and from quick skimming of the Fortran
standard also LOGICAL type.  BOOLEAN_TYPE has just two values, false
and true (similarly for LOGICAL .false. and .true.) and so the folder
IMHO correctly folds this into
l = .true.
(4Ho wo is non-zero).  The transfer_simplify_4.f90 testcase transfers an
integer into logical and back and expects again all the 32-bits to be
preserved.

Fortran folks, can you please look at these 2 testcases and say whether
they are valid Fortran or just undefined behavior?

2008-11-11  Jakub Jelinek  <jakub@redhat.com>

	PR target/35366
	* fold-const.c (native_encode_string): New function.
	(native_encode_expr): Use it for STRING_CST.

	* gfortran.dg/hollerith.f90: Don't assume a 32-bit value
	stored into logical variable will be preserved.
	* gfortran.dg/transfer_simplify_4.f90: Removed.

--- gcc/fold-const.c.jj	2008-10-29 18:49:06.000000000 +0100
+++ gcc/fold-const.c	2008-11-11 11:41:34.000000000 +0100
@@ -7315,6 +7315,37 @@ native_encode_vector (const_tree expr, u
 }
 
 
+/* Subroutine of native_encode_expr.  Encode the STRING_CST
+   specified by EXPR into the buffer PTR of length LEN bytes.
+   Return the number of bytes placed in the buffer, or zero
+   upon failure.  */
+
+static int
+native_encode_string (const_tree expr, unsigned char *ptr, int len)
+{
+  tree type = TREE_TYPE (expr);
+  HOST_WIDE_INT total_bytes;
+
+  if (TREE_CODE (type) != ARRAY_TYPE
+      || TREE_CODE (TREE_TYPE (type)) != INTEGER_TYPE
+      || GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (type))) != BITS_PER_UNIT
+      || !host_integerp (TYPE_SIZE_UNIT (type), 0))
+    return 0;
+  total_bytes = tree_low_cst (TYPE_SIZE_UNIT (type), 0);
+  if (total_bytes > len)
+    return 0;
+  if (TREE_STRING_LENGTH (expr) < total_bytes)
+    {
+      memcpy (ptr, TREE_STRING_POINTER (expr), TREE_STRING_LENGTH (expr));
+      memset (ptr + TREE_STRING_LENGTH (expr), 0,
+	      total_bytes - TREE_STRING_LENGTH (expr));
+    }
+  else
+    memcpy (ptr, TREE_STRING_POINTER (expr), total_bytes);
+  return total_bytes;
+}
+
+
 /* Subroutine of fold_view_convert_expr.  Encode the INTEGER_CST,
    REAL_CST, COMPLEX_CST or VECTOR_CST specified by EXPR into the
    buffer PTR of length LEN bytes.  Return the number of bytes
@@ -7337,6 +7368,9 @@ native_encode_expr (const_tree expr, uns
     case VECTOR_CST:
       return native_encode_vector (expr, ptr, len);
 
+    case STRING_CST:
+      return native_encode_string (expr, ptr, len);
+
     default:
       return 0;
     }
--- gcc/testsuite/gfortran.dg/hollerith.f90.jj2	2008-09-30 16:56:06.000000000 +0200
+++ gcc/testsuite/gfortran.dg/hollerith.f90	2008-11-11 13:52:38.000000000 +0100
@@ -8,7 +8,7 @@ character z1(4)
 character*4 z2(2,2)
 character*80 line
 integer i
-logical l
+integer j
 real r
 character*8 c
 
@@ -20,15 +20,15 @@ data z2/4h(i7),'xxxx','xxxx','xxxx'/
 
 z2 (1,2) = 4h(i8)
 i = 4hHell
-l = 4Ho wo
+j = 4Ho wo
 r = 4Hrld! 
-write (line, '(3A4)') i, l, r
+write (line, '(3A4)') i, j, r
 if (line .ne. 'Hello world!') call abort
 i = 2Hab
 r = 2Hab
-l = 2Hab
+j = 2Hab
 c = 2Hab
-write (line, '(3A4, 8A)') i, l, r, c
+write (line, '(3A4, 8A)') i, j, r, c
 if (line .ne. 'ab  ab  ab  ab      ') call abort
 
 write(line, '(4A8, "!")' ) x
--- gcc/testsuite/gfortran.dg/transfer_simplify_4.f90	2008-09-30 16:56:06.000000000 +0200
+++ gcc/testsuite/gfortran.dg/transfer_simplify_4.f90	2008-09-30 10:18:57.740011359 +0200
@@ -1,30 +0,0 @@
-! { dg-do run }
-! { dg-options "-O0" }
-! Tests that the in-memory representation of a transferred variable
-! propagates properly.
-!
-  implicit none
-
-  integer, parameter :: ip1 = 42
-  logical, parameter :: ap1 = transfer(ip1, .true.)
-  integer, parameter :: ip2 = transfer(ap1, 0)
-
-  logical :: a
-  integer :: i
-  
-  i = transfer(transfer(ip1, .true.), 0)
-  if (i .ne. ip1) call abort ()
-
-  i = transfer(ap1, 0)
-  if (i .ne. ip1) call abort ()
-  
-  a = transfer(ip1, .true.)
-  i = transfer(a, 0)
-  if (i .ne. ip1) call abort ()
-
-  i = ip1
-  a = transfer(i, .true.)
-  i = transfer(a, 0)
-  if (i .ne. ip1) call abort ()
-
-end

	Jakub

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Richard Guenther]

On Tue, Nov 11, 2008 at 7:17 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Fortran FE generates VIEW_CONVERT_EXPR of STRING_CST quite a lot, but
> fold-const isn't able to fold it, so GCC generates very inefficient
> and (in some cases also buggy, as can be seen on the equiv_7.f90
> on powerpc64-darwin with -m64 -Os) code.
>
> I've bootstrapped/regtested this on x86_64-linux.
>
> Unfortrunately, it causes 2 Fortran testsuite failures, hollerith.f90
> and transfer_simplify_4.f90, both at all optimization levels.
> I believe the tests are just invalid though.  The first one does:
> logical l
> l = 4Ho wo
> and expects all the 32 bits preserved, but that is certainly against
> the semantics of BOOLEAN_TYPE and from quick skimming of the Fortran
> standard also LOGICAL type.  BOOLEAN_TYPE has just two values, false
> and true (similarly for LOGICAL .false. and .true.) and so the folder
> IMHO correctly folds this into
> l = .true.
> (4Ho wo is non-zero).  The transfer_simplify_4.f90 testcase transfers an
> integer into logical and back and expects again all the 32-bits to be
> preserved.
>
> Fortran folks, can you please look at these 2 testcases and say whether
> they are valid Fortran or just undefined behavior?

The patch is ok if the fortran folks are happy with it.

Thanks,
Richard.

> 2008-11-11  Jakub Jelinek  <jakub@redhat.com>
>
>        PR target/35366
>        * fold-const.c (native_encode_string): New function.
>        (native_encode_expr): Use it for STRING_CST.
>
>        * gfortran.dg/hollerith.f90: Don't assume a 32-bit value
>        stored into logical variable will be preserved.
>        * gfortran.dg/transfer_simplify_4.f90: Removed.
>
> --- gcc/fold-const.c.jj 2008-10-29 18:49:06.000000000 +0100
> +++ gcc/fold-const.c    2008-11-11 11:41:34.000000000 +0100
> @@ -7315,6 +7315,37 @@ native_encode_vector (const_tree expr, u
>  }
>
>
> +/* Subroutine of native_encode_expr.  Encode the STRING_CST
> +   specified by EXPR into the buffer PTR of length LEN bytes.
> +   Return the number of bytes placed in the buffer, or zero
> +   upon failure.  */
> +
> +static int
> +native_encode_string (const_tree expr, unsigned char *ptr, int len)
> +{
> +  tree type = TREE_TYPE (expr);
> +  HOST_WIDE_INT total_bytes;
> +
> +  if (TREE_CODE (type) != ARRAY_TYPE
> +      || TREE_CODE (TREE_TYPE (type)) != INTEGER_TYPE
> +      || GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (type))) != BITS_PER_UNIT
> +      || !host_integerp (TYPE_SIZE_UNIT (type), 0))
> +    return 0;
> +  total_bytes = tree_low_cst (TYPE_SIZE_UNIT (type), 0);
> +  if (total_bytes > len)
> +    return 0;
> +  if (TREE_STRING_LENGTH (expr) < total_bytes)
> +    {
> +      memcpy (ptr, TREE_STRING_POINTER (expr), TREE_STRING_LENGTH (expr));
> +      memset (ptr + TREE_STRING_LENGTH (expr), 0,
> +             total_bytes - TREE_STRING_LENGTH (expr));
> +    }
> +  else
> +    memcpy (ptr, TREE_STRING_POINTER (expr), total_bytes);
> +  return total_bytes;
> +}
> +
> +
>  /* Subroutine of fold_view_convert_expr.  Encode the INTEGER_CST,
>    REAL_CST, COMPLEX_CST or VECTOR_CST specified by EXPR into the
>    buffer PTR of length LEN bytes.  Return the number of bytes
> @@ -7337,6 +7368,9 @@ native_encode_expr (const_tree expr, uns
>     case VECTOR_CST:
>       return native_encode_vector (expr, ptr, len);
>
> +    case STRING_CST:
> +      return native_encode_string (expr, ptr, len);
> +
>     default:
>       return 0;
>     }
> --- gcc/testsuite/gfortran.dg/hollerith.f90.jj2 2008-09-30 16:56:06.000000000 +0200
> +++ gcc/testsuite/gfortran.dg/hollerith.f90     2008-11-11 13:52:38.000000000 +0100
> @@ -8,7 +8,7 @@ character z1(4)
>  character*4 z2(2,2)
>  character*80 line
>  integer i
> -logical l
> +integer j
>  real r
>  character*8 c
>
> @@ -20,15 +20,15 @@ data z2/4h(i7),'xxxx','xxxx','xxxx'/
>
>  z2 (1,2) = 4h(i8)
>  i = 4hHell
> -l = 4Ho wo
> +j = 4Ho wo
>  r = 4Hrld!
> -write (line, '(3A4)') i, l, r
> +write (line, '(3A4)') i, j, r
>  if (line .ne. 'Hello world!') call abort
>  i = 2Hab
>  r = 2Hab
> -l = 2Hab
> +j = 2Hab
>  c = 2Hab
> -write (line, '(3A4, 8A)') i, l, r, c
> +write (line, '(3A4, 8A)') i, j, r, c
>  if (line .ne. 'ab  ab  ab  ab      ') call abort
>
>  write(line, '(4A8, "!")' ) x
> --- gcc/testsuite/gfortran.dg/transfer_simplify_4.f90   2008-09-30 16:56:06.000000000 +0200
> +++ gcc/testsuite/gfortran.dg/transfer_simplify_4.f90   2008-09-30 10:18:57.740011359 +0200
> @@ -1,30 +0,0 @@
> -! { dg-do run }
> -! { dg-options "-O0" }
> -! Tests that the in-memory representation of a transferred variable
> -! propagates properly.
> -!
> -  implicit none
> -
> -  integer, parameter :: ip1 = 42
> -  logical, parameter :: ap1 = transfer(ip1, .true.)
> -  integer, parameter :: ip2 = transfer(ap1, 0)
> -
> -  logical :: a
> -  integer :: i
> -
> -  i = transfer(transfer(ip1, .true.), 0)
> -  if (i .ne. ip1) call abort ()
> -
> -  i = transfer(ap1, 0)
> -  if (i .ne. ip1) call abort ()
> -
> -  a = transfer(ip1, .true.)
> -  i = transfer(a, 0)
> -  if (i .ne. ip1) call abort ()
> -
> -  i = ip1
> -  a = transfer(i, .true.)
> -  i = transfer(a, 0)
> -  if (i .ne. ip1) call abort ()
> -
> -end
>
>        Jakub
>

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Tobias Schlüter]

Jakub Jelinek wrote:
> Unfortrunately, it causes 2 Fortran testsuite failures, hollerith.f90
> and transfer_simplify_4.f90, both at all optimization levels.
> I believe the tests are just invalid though.  The first one does:
> logical l
> l = 4Ho wo
> and expects all the 32 bits preserved, but that is certainly against
> the semantics of BOOLEAN_TYPE and from quick skimming of the Fortran
> standard also LOGICAL type.  BOOLEAN_TYPE has just two values, false
> and true (similarly for LOGICAL .false. and .true.) and so the folder
> IMHO correctly folds this into
> l = .true.
> (4Ho wo is non-zero).  The transfer_simplify_4.f90 testcase transfers an
> integer into logical and back and expects again all the 32-bits to be
> preserved.
> 
> Fortran folks, can you please look at these 2 testcases and say whether
> they are valid Fortran or just undefined behavior?

They are not standard Fortran.  Using Hollerith constants this way was 
the way of encoding strings before there was a Character type in 
Fortran, so the current behavior is intentional.  Whether there actually 
is code that uses logicals to encode strings, I can't tell.  I CCed Feng 
Wang who added the original Hollerith support, and Steve who last 
modified the testcase.

Cheers,
- Tobi

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Jakub Jelinek]

On Tue, Nov 11, 2008 at 04:45:40PM +0100, Tobias Schlüter wrote:
> Jakub Jelinek wrote:
>> Unfortrunately, it causes 2 Fortran testsuite failures, hollerith.f90
>> and transfer_simplify_4.f90, both at all optimization levels.
>> I believe the tests are just invalid though.  The first one does:
>> logical l
>> l = 4Ho wo
>> and expects all the 32 bits preserved, but that is certainly against
>> the semantics of BOOLEAN_TYPE and from quick skimming of the Fortran
>> standard also LOGICAL type.  BOOLEAN_TYPE has just two values, false
>> and true (similarly for LOGICAL .false. and .true.) and so the folder
>> IMHO correctly folds this into
>> l = .true.
>> (4Ho wo is non-zero).  The transfer_simplify_4.f90 testcase transfers an
>> integer into logical and back and expects again all the 32-bits to be
>> preserved.
>>
>> Fortran folks, can you please look at these 2 testcases and say whether
>> they are valid Fortran or just undefined behavior?
>
> They are not standard Fortran.  Using Hollerith constants this way was  
> the way of encoding strings before there was a Character type in  
> Fortran, so the current behavior is intentional.  Whether there actually  
> is code that uses logicals to encode strings, I can't tell.  I CCed Feng  
> Wang who added the original Hollerith support, and Steve who last  
> modified the testcase.

If using logicals is required to store all the bits of the constants
(and similarly for TRANSFER from integer to logical), then I'm afraid the
Fortran FE would have to stop using BOOLEAN_TYPE for logical, as the
middle-end BOOLEAN_TYPE really has the semantics that it is either false or
true, nothing else.

For TRANSFER I found:
http://gcc.gnu.org/ml/gcc-patches/2007-05/msg01903.html
but I'm still unconvinced.  I'd say arguing that the logical must
hold all the bits is the same as if you said
real(kind=8), parameter :: a = transfer (transfer (3.1415926d0, 0), 0.d0)
print *, a
end
must print 3.14159...  A precondition of the two transfers giving identity
is IMHO that no bits are lost, but if the precision of the middle-type
is smaller than of the other type, it can't be fully preserved.  The
precision of integer(kind=4) is smaller than of real(kind=8), and similarly
I'd say for logical(kind=4) and integer(kind=4), because logical:
"The logical type has two values which represent true and false."

	Jakub

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Tobias Schlüter]

Jakub Jelinek wrote:
>> They are not standard Fortran.  Using Hollerith constants this way was  
>> the way of encoding strings before there was a Character type in  
>> Fortran, so the current behavior is intentional.  Whether there actually  
>> is code that uses logicals to encode strings, I can't tell.  I CCed Feng  
>> Wang who added the original Hollerith support, and Steve who last  
>> modified the testcase.
> 
> If using logicals is required to store all the bits of the constants
> (and similarly for TRANSFER from integer to logical), then I'm afraid the
> Fortran FE would have to stop using BOOLEAN_TYPE for logical, as the
> middle-end BOOLEAN_TYPE really has the semantics that it is either false or
> true, nothing else.

> For TRANSFER I found:
> http://gcc.gnu.org/ml/gcc-patches/2007-05/msg01903.html
> but I'm still unconvinced.  I'd say arguing that the logical must
> hold all the bits is the same as if you said
> real(kind=8), parameter :: a = transfer (transfer (3.1415926d0, 0), 0.d0)
> print *, a
> end
> must print 3.14159...  A precondition of the two transfers giving identity
> is IMHO that no bits are lost, but if the precision of the middle-type
> is smaller than of the other type, it can't be fully preserved.  The
> precision of integer(kind=4) is smaller than of real(kind=8), and similarly
> I'd say for logical(kind=4) and integer(kind=4), because logical:
> "The logical type has two values which represent true and false."

Hm, I haven't found Richard Maine's post that is mentioned in the old 
thread, but Fortran has the notion of storage unit.  LOGICAL*4, 
INTEGER*4 and REAL*4 all occupy 4 storage units.  If a circular 
transfer(transfer (...) ...) between types with the same number of 
storage units is required to return the original value (as Richard Maine 
apparently argued), then we'll probably have no choice but to stop using 
BOOLEAN_TYPE for LOGICALs.

Maybe Brooks can point us to the comp.lang.fortran thread.

Cheers,
- Tobi

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Jakub Jelinek]

On Tue, Nov 11, 2008 at 05:21:36PM +0100, Tobias Schlüter wrote:
> Hm, I haven't found Richard Maine's post that is mentioned in the old  
> thread, but Fortran has the notion of storage unit.  LOGICAL*4,  
> INTEGER*4 and REAL*4 all occupy 4 storage units.  If a circular  
> transfer(transfer (...) ...) between types with the same number of  
> storage units is required to return the original value (as Richard Maine  
> apparently argued), then we'll probably have no choice but to stop using  
> BOOLEAN_TYPE for LOGICALs.

That's pretty sad.  Stopping using BOOLEAN_TYPE is going to break
basically everything, so certainly can't be done for 4.4.  I expect
that you'd need to add a lot of conversions everywhere, from the
logical*N INTEGER_TYPE to correspondingly sized BOOLEAN_TYPE and back
all the time.
I wonder what you can do with LOGICALs that have values transfered from
INTEGER (other than 0/1).  Does the standard define whether something
is executed or not in:
logical, parameter :: l = transfer (234, .false.)
if (l) print *, ".true."
?  I.e. is it supposed to be .false. or .true., or undefined behavior?

I guess I could in fold_view_convert_expr temporarily

  /* XXX: Fortran workaround - the FE sometimes wants BOOLEAN_TYPE semantics
     and sometimes not.  */
  if (TREE_CODE (type) == BOOLEAN_TYPE && TREE_CODE (expr) == STRING_CST)
    return NULL_TREE;

and the FE would need to make sure the STRING_CST has enough aligned type
(on the STRING_CST TREE_TYPE (string_cst) = copy_node (TREE_TYPE (string_cst));
and TYPE_ALIGN (TREE_TYPE (string_cst)) = TYPE_ALIGN (gfc_get_logical_type
(expr->ts.kind)); TYPE_USER_ALIGN (TREE_TYPE (string_cst)) = 1;),
but guess Richi isn't going to look favourably at it.  Unfortunately
the result of TRANSFER is supposed to be a constant :(, so we can't play
games with asm, volatile or some other optimization barrier.

	Jakub

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Tobias Schlüter]

Jakub Jelinek wrote:
> On Tue, Nov 11, 2008 at 05:21:36PM +0100, Tobias Schlüter wrote:
>> Hm, I haven't found Richard Maine's post that is mentioned in the old  
>> thread, but Fortran has the notion of storage unit.  LOGICAL*4,  
>> INTEGER*4 and REAL*4 all occupy 4 storage units.  If a circular  
>> transfer(transfer (...) ...) between types with the same number of  
>> storage units is required to return the original value (as Richard Maine  
>> apparently argued), then we'll probably have no choice but to stop using  
>> BOOLEAN_TYPE for LOGICALs.
> 
> That's pretty sad.  Stopping using BOOLEAN_TYPE is going to break
> basically everything, so certainly can't be done for 4.4.  I expect
> that you'd need to add a lot of conversions everywhere, from the
> logical*N INTEGER_TYPE to correspondingly sized BOOLEAN_TYPE and back
> all the time.

Maybe setting TYPE_PRECISION to bit_size in gfc_build_logical_type() 
would do the trick without any further changes?  This depends on how the 
middle-end deals with BOOLEAN_TYPES with TYPE_PRECISION > 1.

Cheers,
- Tobi

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Jakub Jelinek]

On Tue, Nov 11, 2008 at 06:01:52PM +0100, Tobias Schlüter wrote:
> Jakub Jelinek wrote:
>> On Tue, Nov 11, 2008 at 05:21:36PM +0100, Tobias Schlüter wrote:
>>> Hm, I haven't found Richard Maine's post that is mentioned in the old 
>>>  thread, but Fortran has the notion of storage unit.  LOGICAL*4,   
>>> INTEGER*4 and REAL*4 all occupy 4 storage units.  If a circular   
>>> transfer(transfer (...) ...) between types with the same number of   
>>> storage units is required to return the original value (as Richard 
>>> Maine  apparently argued), then we'll probably have no choice but to 
>>> stop using  BOOLEAN_TYPE for LOGICALs.
>>
>> That's pretty sad.  Stopping using BOOLEAN_TYPE is going to break
>> basically everything, so certainly can't be done for 4.4.  I expect
>> that you'd need to add a lot of conversions everywhere, from the
>> logical*N INTEGER_TYPE to correspondingly sized BOOLEAN_TYPE and back
>> all the time.
>
> Maybe setting TYPE_PRECISION to bit_size in gfc_build_logical_type()  
> would do the trick without any further changes?  This depends on how the  
> middle-end deals with BOOLEAN_TYPES with TYPE_PRECISION > 1.

It wouldn't help:

/* Boolean type (true or false are the only values).  Looks like an
   INTEGRAL_TYPE.  */
DEFTREECODE (BOOLEAN_TYPE, "boolean_type", tcc_type, 0)

On the other side, if you have to convert from INTEGER_TYPE to BOOLEAN_TYPE
all the time, the generated code will be very abysmal.

How is LOGICAL supposed to behave if it is storage associated with an
integer?  Defined only if the integer stored has been 0 or 1, or 0 .false.
and non-zero .true., or something else?

Also, even with my patch
integer, parameter :: i = transfer (transfer (42, .true.), 0)
will work as the standard requires (in this case the transfers
are resolved in the FE, and only one VIEW_CONVERT_EXPR is created
(and folded to 42).  But that's not what is failing in
transfer_simplify_4.f90.  What fails is:
  a = transfer(ip1, .true.)
  i = transfer(a, 0)
  if (i .ne. ip1) call abort ()
(as ip1 is parameter, then this is the same as
  a = transfer(42, .true.)
  i = transfer(a, 0)
  if (i .ne. ip1) call abort ()
).  If I modify the testcase slightly:

  implicit none
  integer :: ip1
  logical :: a
  integer :: i
  ip1 = 42
  a = transfer(ip1, .true.)
  i = transfer(a, 0)
  if (i .ne. ip1) call abort ()
end

then it fails even with unpatched gfortran (even with 4.3) at -O1 and higher.
But does the standard really say anything about this?  Isn't assignment
to a LOGICAL value supposed to set to to .false. or .true.?

	Jakub

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Janne Blomqvist]

On Tue, Nov 11, 2008 at 19:49, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Nov 11, 2008 at 06:01:52PM +0100, Tobias Schlüter wrote:
>> Jakub Jelinek wrote:
>>> On Tue, Nov 11, 2008 at 05:21:36PM +0100, Tobias Schlüter wrote:
>>>> Hm, I haven't found Richard Maine's post that is mentioned in the old
>>>>  thread, but Fortran has the notion of storage unit.  LOGICAL*4,
>>>> INTEGER*4 and REAL*4 all occupy 4 storage units.  If a circular
>>>> transfer(transfer (...) ...) between types with the same number of
>>>> storage units is required to return the original value (as Richard
>>>> Maine  apparently argued), then we'll probably have no choice but to
>>>> stop using  BOOLEAN_TYPE for LOGICALs.
>>>
>>> That's pretty sad.  Stopping using BOOLEAN_TYPE is going to break
>>> basically everything, so certainly can't be done for 4.4.  I expect
>>> that you'd need to add a lot of conversions everywhere, from the
>>> logical*N INTEGER_TYPE to correspondingly sized BOOLEAN_TYPE and back
>>> all the time.
>>
>> Maybe setting TYPE_PRECISION to bit_size in gfc_build_logical_type()
>> would do the trick without any further changes?  This depends on how the
>> middle-end deals with BOOLEAN_TYPES with TYPE_PRECISION > 1.
>
> It wouldn't help:
>
> /* Boolean type (true or false are the only values).  Looks like an
>   INTEGRAL_TYPE.  */
> DEFTREECODE (BOOLEAN_TYPE, "boolean_type", tcc_type, 0)
>
> On the other side, if you have to convert from INTEGER_TYPE to BOOLEAN_TYPE
> all the time, the generated code will be very abysmal.
>
> How is LOGICAL supposed to behave if it is storage associated with an
> integer?  Defined only if the integer stored has been 0 or 1, or 0 .false.
> and non-zero .true., or something else?

The standard doesn't specify what is the internal representation of
logical values, other than that the size of default LOGICAL is the
same as other types of default kind.  If you search on google groups
for "Richard Maine transfer logical" you'll find the threads "Transfer
and variables that don't use all their storage space." and "Trying to
use a logical as an array index" with arguments about this point
(Richard Maine was the editor of the Fortran 2003 standard, so he
generally knows what he's talking about).

The loophole seems to be that if the transfer generates an illegal
value, then the program becomes non-standard and the compiler can do
whatever it wants.

I.e. while the point of transfer is more or less to do type-punning
via straight-forward copying of bits, the resulting bit patterns must
be a valid bit pattern for the type in question if the program is to
be standard-conforming.

-- 
Janne Blomqvist

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Brooks Moses]

Jakub Jelinek wrote, at 11/11/2008 9:49 AM:
> How is LOGICAL supposed to behave if it is storage associated with an
> integer?  Defined only if the integer stored has been 0 or 1, or 0 .false.
> and non-zero .true., or something else?

Either one of those makes sense to me.  Like all type-punning (other
than transfer round-trips in cases where they're legal), it's
implementation defined, and either one of those is as good a definition
as any.

(Personally, I'm in favor of the 0->false, 1->true, other->undefined
version, on grounds that it's the least necessary specification and thus
easiest to maintain compatibility with in the future.)

- Brooks

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Tobias Burnus]

Jakub Jelinek wrote:
> How is LOGICAL supposed to behave if it is storage associated with an
> integer?  Defined only if the integer stored has been 0 or 1, or 0 .false.
> and non-zero .true., or something else?
>   
Note: I strongly believe that EQUIVALENCE*, COMMON* and TRANSFER are no
true problems - using them is outside the Fortran standard and using
them does not make much sense.  (* here applies: "When a variable of a
given type becomes defined, all associated variables of different type
become undefined.").

The only real problem are Hollerith variables. Here, preserving the bit
pattern is crucial. As written before, I think the number of
still-in-use programs which use this Fortran-66-only features with
LOGICAL variables is very small. However, as (if?) they still exist,
some solution needs to be found.

(My preferred solution is to stick by default to the current middle-end
representation any print a warning or error for assignments of Hollerith
constants to logical variables; having a compiler flag to uses
internally integer values instead of booleans would be a bonus [I don't
see ad hoc how much work this would be].)

> But does the standard really say anything about this?  Isn't assignment
> to a LOGICAL value supposed to set to to .false. or .true.?
>   
I think the standard leaves this to the implementor as long as for
"logical = .true." "logical" is true and for "logical = .false." it is
false, the program should be standard conform, independent what internal
value "logical" has; that the bit pattern remains the same is a pure
convenience.

 * * *

For whatever it is worth: I posted to comp.lang.fortran to ask whether
there is anything in the Fortran 77 to 2008 standard which mandates that
the bit pattern is preserved (I don't think so); I further asked whether
there is some compelling real-world case for preserving it. See:
http://groups.google.com/group/comp.lang.fortran/browse_thread/thread/cf874197dc460679#

Tobias

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Brooks Moses]

Tobias Burnus wrote, at 11/11/2008 11:21 AM:
> The only real problem are Hollerith variables. Here, preserving the bit
> pattern is crucial. As written before, I think the number of
> still-in-use programs which use this Fortran-66-only features with
> LOGICAL variables is very small. However, as (if?) they still exist,
> some solution needs to be found.
> 
> (My preferred solution is to stick by default to the current middle-end
> representation any print a warning or error for assignments of Hollerith
> constants to logical variables; having a compiler flag to uses
> internally integer values instead of booleans would be a bonus [I don't
> see ad hoc how much work this would be].)

I agree.  I can see no sensible reason why anyone would have combined
Hollerith values with LOGICAL variables, and thus see no reason to
suppose that such code exists.  As such, I think it's reasonable to
print an error if someone tries to do such a thing, and then not worry
about implementations and compiler flags unless someone happens to
complain about the error and provides a good reason why they can't
change their code.

- Brooks

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Jakub Jelinek]

On Tue, Nov 11, 2008 at 11:54:25AM -0800, Brooks Moses wrote:
> Tobias Burnus wrote, at 11/11/2008 11:21 AM:
> > The only real problem are Hollerith variables. Here, preserving the bit
> > pattern is crucial. As written before, I think the number of
> > still-in-use programs which use this Fortran-66-only features with
> > LOGICAL variables is very small. However, as (if?) they still exist,
> > some solution needs to be found.
> > 
> > (My preferred solution is to stick by default to the current middle-end
> > representation any print a warning or error for assignments of Hollerith
> > constants to logical variables; having a compiler flag to uses
> > internally integer values instead of booleans would be a bonus [I don't
> > see ad hoc how much work this would be].)
> 
> I agree.  I can see no sensible reason why anyone would have combined
> Hollerith values with LOGICAL variables, and thus see no reason to
> suppose that such code exists.  As such, I think it's reasonable to
> print an error if someone tries to do such a thing, and then not worry
> about implementations and compiler flags unless someone happens to
> complain about the error and provides a good reason why they can't
> change their code.

So would the following be acceptable?  It will warn both for
logical l
l = 4Hfoob
and
l = transfer (42, .true.)
BTW, even the unmodified gfortran.dg/transfer_simplify_4.f90 tests
fails with vanilla 4.3 or trunk when compiled with -O1 and above,
so claiming we support all of those transfers that way and actually
supporting them only at -O0 is weird.  The middle-end hunk hasn't
changed, so I only need Fortran approval...

2008-11-11  Jakub Jelinek  <jakub@redhat.com>

	PR target/35366
	* fold-const.c (native_encode_string): New function.
	(native_encode_expr): Use it for STRING_CST.

	* trans-const.c (gfc_conv_constant_to_tree): Warn when
	converting an integer outside of LOGICAL's range to
	LOGICAL.

	* gfortran.dg/hollerith.f90: Don't assume a 32-bit value
	stored into logical variable will be preserved.
	* gfortran.dg/transfer_simplify_4.f90: Remove undefined
	cases.

--- gcc/fold-const.c.jj	2008-10-29 18:49:06.000000000 +0100
+++ gcc/fold-const.c	2008-11-11 20:33:49.000000000 +0100
@@ -7315,6 +7315,37 @@ native_encode_vector (const_tree expr, u
 }
 
 
+/* Subroutine of native_encode_expr.  Encode the STRING_CST
+   specified by EXPR into the buffer PTR of length LEN bytes.
+   Return the number of bytes placed in the buffer, or zero
+   upon failure.  */
+
+static int
+native_encode_string (const_tree expr, unsigned char *ptr, int len)
+{
+  tree type = TREE_TYPE (expr);
+  HOST_WIDE_INT total_bytes;
+
+  if (TREE_CODE (type) != ARRAY_TYPE
+      || TREE_CODE (TREE_TYPE (type)) != INTEGER_TYPE
+      || GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (type))) != BITS_PER_UNIT
+      || !host_integerp (TYPE_SIZE_UNIT (type), 0))
+    return 0;
+  total_bytes = tree_low_cst (TYPE_SIZE_UNIT (type), 0);
+  if (total_bytes > len)
+    return 0;
+  if (TREE_STRING_LENGTH (expr) < total_bytes)
+    {
+      memcpy (ptr, TREE_STRING_POINTER (expr), TREE_STRING_LENGTH (expr));
+      memset (ptr + TREE_STRING_LENGTH (expr), 0,
+	      total_bytes - TREE_STRING_LENGTH (expr));
+    }
+  else
+    memcpy (ptr, TREE_STRING_POINTER (expr), total_bytes);
+  return total_bytes;
+}
+
+
 /* Subroutine of fold_view_convert_expr.  Encode the INTEGER_CST,
    REAL_CST, COMPLEX_CST or VECTOR_CST specified by EXPR into the
    buffer PTR of length LEN bytes.  Return the number of bytes
@@ -7337,6 +7368,9 @@ native_encode_expr (const_tree expr, uns
     case VECTOR_CST:
       return native_encode_vector (expr, ptr, len);
 
+    case STRING_CST:
+      return native_encode_string (expr, ptr, len);
+
     default:
       return 0;
     }
--- gcc/fortran/trans-const.c.jj	2008-09-30 16:56:44.000000000 +0200
+++ gcc/fortran/trans-const.c	2008-11-11 21:50:16.000000000 +0100
@@ -281,13 +281,25 @@ gfc_conv_constant_to_tree (gfc_expr * ex
 
     case BT_LOGICAL:
       if (expr->representation.string)
-	return fold_build1 (VIEW_CONVERT_EXPR,
-			    gfc_get_logical_type (expr->ts.kind),
-			    gfc_build_string_const (expr->representation.length,
-						    expr->representation.string));
+	{
+	  tree tmp = fold_build1 (VIEW_CONVERT_EXPR,
+				  gfc_get_int_type (expr->ts.kind),
+				  gfc_build_string_const (expr->representation.length,
+							  expr->representation.string));
+	  if (TREE_CODE (tmp) == INTEGER_CST)
+	    {
+	      if (!integer_zerop (tmp) && !integer_onep (tmp))
+		gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
+			     " at %L has undefined result", &expr->where);
+	    }
+	  else
+	    gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
+			 " at %L might have undefined result", &expr->where);
+	  return fold_convert (gfc_get_logical_type (expr->ts.kind), tmp);
+	}
       else
 	return build_int_cst (gfc_get_logical_type (expr->ts.kind),
-			    expr->value.logical);
+			      expr->value.logical);
 
     case BT_COMPLEX:
       if (expr->representation.string)
--- gcc/testsuite/gfortran.dg/hollerith.f90.jj	2008-09-30 16:56:06.000000000 +0200
+++ gcc/testsuite/gfortran.dg/hollerith.f90	2008-11-11 13:52:38.000000000 +0100
@@ -8,7 +8,7 @@ character z1(4)
 character*4 z2(2,2)
 character*80 line
 integer i
-logical l
+integer j
 real r
 character*8 c
 
@@ -20,15 +20,15 @@ data z2/4h(i7),'xxxx','xxxx','xxxx'/
 
 z2 (1,2) = 4h(i8)
 i = 4hHell
-l = 4Ho wo
+j = 4Ho wo
 r = 4Hrld! 
-write (line, '(3A4)') i, l, r
+write (line, '(3A4)') i, j, r
 if (line .ne. 'Hello world!') call abort
 i = 2Hab
 r = 2Hab
-l = 2Hab
+j = 2Hab
 c = 2Hab
-write (line, '(3A4, 8A)') i, l, r, c
+write (line, '(3A4, 8A)') i, j, r, c
 if (line .ne. 'ab  ab  ab  ab      ') call abort
 
 write(line, '(4A8, "!")' ) x
--- gcc/testsuite/gfortran.dg/transfer_simplify_4.f90.jj	2008-09-30 16:56:06.000000000 +0200
+++ gcc/testsuite/gfortran.dg/transfer_simplify_4.f90	2008-11-11 21:39:07.000000000 +0100
@@ -6,25 +6,9 @@
   implicit none
 
   integer, parameter :: ip1 = 42
-  logical, parameter :: ap1 = transfer(ip1, .true.)
-  integer, parameter :: ip2 = transfer(ap1, 0)
-
-  logical :: a
+  integer, parameter :: ip2 = transfer(transfer(ip1, .true.), 0)
   integer :: i
   
   i = transfer(transfer(ip1, .true.), 0)
   if (i .ne. ip1) call abort ()
-
-  i = transfer(ap1, 0)
-  if (i .ne. ip1) call abort ()
-  
-  a = transfer(ip1, .true.)
-  i = transfer(a, 0)
-  if (i .ne. ip1) call abort ()
-
-  i = ip1
-  a = transfer(i, .true.)
-  i = transfer(a, 0)
-  if (i .ne. ip1) call abort ()
-
 end


	Jakub

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Brooks Moses]

Jakub Jelinek wrote, at 11/11/2008 1:01 PM:
> So would the following be acceptable?  It will warn both for
> logical l
> l = 4Hfoob

Personally, I feel that this should be an error, not a warning.  But if
distinguishing this from the transfer case complicates the code (which
my interpretation of your patch would suggest that it does), or if other
people feel differently, I don't have a strong opinion here.

> and
> l = transfer (42, .true.)

That sounds like a reasonable result.  Note, though, that the following
should not give a warning:

l = transfer(transfer(.true., 3.1415), .true.)

Similarly, there are other things that reliably transfer into integer
values of 0 and 1 (e.g., appropriate string values, though those differ
with endianness) that should not give warnings when transferred to a
logical.

I think we should also add a testcase to confirm that this compiles
without warning, if we don't have one already:

i = [something defined at runtime]
l = transfer(i, .true.)

(Also, we should stop using lower-case ells for logical variables,
because they are horrid for legibility.)

> --- gcc/fortran/trans-const.c.jj	2008-09-30 16:56:44.000000000 +0200
> +++ gcc/fortran/trans-const.c	2008-11-11 21:50:16.000000000 +0100
[...]
> +	  if (TREE_CODE (tmp) == INTEGER_CST)
> +	    {
> +	      if (!integer_zerop (tmp) && !integer_onep (tmp))
> +		gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
> +			     " at %L has undefined result", &expr->where);
> +	    }

This looks good.

> +	  else
> +	    gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
> +			 " at %L might have undefined result", &expr->where);

This doesn't -- if I'm understanding it correctly, it warns on any cast
of a non-integer value to a logical, regardless of whether the result
might or might not in fact be undefined.

I'd be okay with a lack of warning here.  Or we could be really fancy
and transfer all non-integer inputs to integers and then do the integer
checks, but that seems excessive.

> --- gcc/testsuite/gfortran.dg/hollerith.f90.jj	2008-09-30 16:56:06.000000000 +0200
> +++ gcc/testsuite/gfortran.dg/hollerith.f90	2008-11-11 13:52:38.000000000 +0100
> @@ -8,7 +8,7 @@ character z1(4)
>  character*4 z2(2,2)
>  character*80 line
>  integer i
> -logical l
> +integer j
>  real r
>  character*8 c
>  
> @@ -20,15 +20,15 @@ data z2/4h(i7),'xxxx','xxxx','xxxx'/
>  
>  z2 (1,2) = 4h(i8)
>  i = 4hHell
> -l = 4Ho wo
> +j = 4Ho wo
>  r = 4Hrld! 
> -write (line, '(3A4)') i, l, r
> +write (line, '(3A4)') i, j, r
>  if (line .ne. 'Hello world!') call abort
>  i = 2Hab
>  r = 2Hab
> -l = 2Hab
> +j = 2Hab
>  c = 2Hab
> -write (line, '(3A4, 8A)') i, l, r, c
> +write (line, '(3A4, 8A)') i, j, r, c
>  if (line .ne. 'ab  ab  ab  ab      ') call abort
>  
>  write(line, '(4A8, "!")' ) x

Here, we should just get rid of "l" entirely, rather than making it into
a clone of "i".  There's no point in the redundant tests.

> --- gcc/testsuite/gfortran.dg/transfer_simplify_4.f90.jj	2008-09-30 16:56:06.000000000 +0200
> +++ gcc/testsuite/gfortran.dg/transfer_simplify_4.f90	2008-11-11 21:39:07.000000000 +0100
> @@ -6,25 +6,9 @@
>    implicit none
>  
>    integer, parameter :: ip1 = 42
> -  logical, parameter :: ap1 = transfer(ip1, .true.)
> -  integer, parameter :: ip2 = transfer(ap1, 0)
> -
> -  logical :: a
> +  integer, parameter :: ip2 = transfer(transfer(ip1, .true.), 0)
>    integer :: i
>    
>    i = transfer(transfer(ip1, .true.), 0)
>    if (i .ne. ip1) call abort ()

This needs a comment: "This is a quality-of-implementation issue, not
required by the standard.  See discussion at:" and a link to this thread.

Also, you never test the result of ip2.  It looks redundant with the
test for i; I don't see the need for both.  So I'd suggest either
getting rid of it or testing it.

I've been out of the GFortran loop for long enough that I'd like to have
someone else approve this before committing, but my take is that it's
fine with the above changes.

- Brooks

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Jakub Jelinek]

On Tue, Nov 11, 2008 at 01:27:30PM -0800, Brooks Moses wrote:
> That sounds like a reasonable result.  Note, though, that the following
> should not give a warning:
> 
> l = transfer(transfer(.true., 3.1415), .true.)

Yes, this compiles without a warning.  I'll add a testcase.

> I think we should also add a testcase to confirm that this compiles
> without warning, if we don't have one already:
> 
> i = [something defined at runtime]
> l = transfer(i, .true.)

Ok.

> > +	  else
> > +	    gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
> > +			 " at %L might have undefined result", &expr->where);
> 
> This doesn't -- if I'm understanding it correctly, it warns on any cast
> of a non-integer value to a logical, regardless of whether the result
> might or might not in fact be undefined.

No, that's not what it does.  This warning is only in case it is constant,
and stored in memory.  So in theory tmp should always be INTEGER_CST.
I guess I could just assume it (or assert it).

> Here, we should just get rid of "l" entirely, rather than making it into
> a clone of "i".  There's no point in the redundant tests.

Ok.

	Jakub

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Brooks Moses]

Jakub Jelinek wrote, at 11/11/2008 1:35 PM:
> On Tue, Nov 11, 2008 at 01:27:30PM -0800, Brooks Moses wrote:
>>> +	  else
>>> +	    gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
>>> +			 " at %L might have undefined result", &expr->where);
>> This doesn't -- if I'm understanding it correctly, it warns on any cast
>> of a non-integer value to a logical, regardless of whether the result
>> might or might not in fact be undefined.
> 
> No, that's not what it does.  This warning is only in case it is constant,
> and stored in memory.  So in theory tmp should always be INTEGER_CST.
> I guess I could just assume it (or assert it).

Oh, okay; my mistake from not reading the code thoroughly enough.  But,
yes, if it's something that in theory should always be true, then
checking for it as if it's a plausible condition just makes the code
harder to understand.

I think it's reasonable to either assume or assert it, depending on how
confident you are that theory and practice match in this case.  Given
that you're creating tmp from a fold_build1 call with an integer type,
it seems safe to assume that it will in fact have an integer TREE_CODE,
and if it doesn't, something is sufficiently broken that it will be
showing up elsewhere too.  So I'd just assume it.

The warning could perhaps be edited a little, too, to reflect that the
user isn't necessarily thinking of the input as an integer.  Maybe:
"Cannot assign value with bitwise representation other than 0x0 or 0x1
to LOGICAL at %L".

- Brooks

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Thomas Koenig]

On Tue, 2008-11-11 at 18:49 +0100, Jakub Jelinek wrote:

> How is LOGICAL supposed to behave if it is storage associated with an
> integer?  Defined only if the integer stored has been 0 or 1, or 0 .false.
> and non-zero .true., or something else?

0 is .false., 1 is .true., anything else is undefined.

We already make use of this in the library.  For example, for mask
arguments, we only check the lower byte.

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Brooks Moses]

Tobias Schlüter wrote, at 11/11/2008 8:21 AM:
> Jakub Jelinek wrote:
>> For TRANSFER I found:
>> http://gcc.gnu.org/ml/gcc-patches/2007-05/msg01903.html
>> but I'm still unconvinced.  I'd say arguing that the logical must
>> hold all the bits is the same as if you said
>> real(kind=8), parameter :: a = transfer (transfer (3.1415926d0, 0), 0.d0)
>> print *, a
>> end
>> must print 3.14159... 

IMO, this equivalence isn't entirely clear.  The double-transfer case is
called out as a special case in the standard, and it appears to be
different from what's required to actually be stored in a logical variable.

I'll start with the summary of my conclusions, since this gets a bit
complicated and tangly.  The conclusion is that the testcases in
question are not testing things required by the standard.

This bit of code in the transfer testcase is a complicated mess,
standard-wise; it's probably illegal, but if we can preserve it as a
quality-of-implementation issue, that arguably a good thing.  It should
be documented in the testcase as QOI, not standard-mandated, though:

  i = transfer(transfer(ip1, .true.), 0)
  if (i .ne. ip1) call abort ()

Meanwhile, these bits of the testcase are definitely relying on
undefined behavior when the compiler encounters illegal code, as the
function call returns an out-of-range value and the standard doesn't
call them out as exceptions.  I definitely should have documented these
as QOI when I wrote them.  It would be nice to preserve them, but I
think they're rather unlikely to show up in real code.

  i = transfer(ap1, 0)
  if (i .ne. ip1) call abort ()

  a = transfer(ip1, .true.)
  i = transfer(a, 0)
  if (i .ne. ip1) call abort ()

  i = ip1
  a = transfer(i, .true.)
  i = transfer(a, 0)
  if (i .ne. ip1) call abort ()

As for the Hollerith case: Hollerith with logicals was never included in
any Fortran standard, and IMHO there is no call whatsoever for
supporting it.

Now, to the analysis.  I apologize that it's as tangled as it is; I
don't have time to further clarify it.

>> A precondition of the two transfers giving identity
>> is IMHO that no bits are lost, but if the precision of the middle-type
>> is smaller than of the other type, it can't be fully preserved.  The
>> precision of integer(kind=4) is smaller than of real(kind=8), and similarly
>> I'd say for logical(kind=4) and integer(kind=4), because logical:
>> "The logical type has two values which represent true and false."

"IMHO" is unfortunately not the best guide here.  According to the F95
spec, the precondition of transfer(transfer(E,D),E) returning identity
is "the physical representation of D is as long as or longer than that
of E."  This is rather more clearly defined than "no bits are lost."  In
particular, ...

> Hm, I haven't found Richard Maine's post that is mentioned in the old 
> thread, but Fortran has the notion of storage unit.  LOGICAL*4, 
> INTEGER*4 and REAL*4 all occupy 4 storage units.

...the storage unit stuff requires that LOGICAL*4 and INTEGER*4
variables occupy the same amount of space.

I assume that we do meet this requirement, even though we use
BOOLEAN_TYPE, yes?  This is pretty critical, and is clearly required by
the standard.

> If a circular 
> transfer(transfer (...) ...) between types with the same number of 
> storage units is required to return the original value (as Richard Maine 
> apparently argued), then we'll probably have no choice but to stop using 
> BOOLEAN_TYPE for LOGICALs.
> 
> Maybe Brooks can point us to the comp.lang.fortran thread.

Wow, it's been a long time since I've looked at this!  :)  The thread is
titled "Transfer and variables that don't use all their storage space."
and occurred on 2007-04-08.

And, when I go back and reread that, I find that I appear to have made a
misinterpretation of what Richard was saying.  The crux of his argument
was this, about why TRANSFER(TRANSFER(-5_8, L), -5_8) is actually
illegal when L is a LOGICAL*8:

Richard Maine wrote, in comp.lang.fortran, on 2007-04-08:
> No. I'm using a back door argument to explain why the code is illegal.
> More or less a proof by contradiction, which is perhaps why it seems
> contradictory. :-) I arise at a contradiction if I assume the code is
> standard conforming. The contradiction is that the standard requires a
> result that the standard also prohibits. Thus, I conclude that the
> assumption of standard conformance is false.

(In particular, the definition of TRANSFER requires this to round-trip,
which means that the inner one must return a value that's not a legal
LOGICAL value, which means that it must be illegal to call TRANSFER with
those arguments.)

He then went on to say, on my question of whether it would be valid for
the compiler to just throw an error in this case:
> I think you could argue that. But then you might get users who
> considered it a poor quality of implementation; that one is harder. I
> have trouble imagining why anyone would ever want to do such a strange
> thing, but James Buskirk can probably come up with something useful. :-)
> 
> I think the more "obvious" thing to do is to use all 8 bytes. But I
> don't know what the costs of that are and what the tradoffs might be of
> those costs versus the (rare?) usage of an idiom like that.

So, given that we seem to have found that the costs of using all 8
bytes, or 4 bytes, or whatever, are rather higher than otherwise
expected.  As a result, I suppose that the tradeoff is that we go with
what might be considered a poor quality of implementation in this case,
in return for a better quality of implementation in the case of anything
that uses logical variables sensibly.

Jakub Jelinek wrote, at 11/11/2008 8:52 AM:
> I wonder what you can do with LOGICALs that have values transfered from
> INTEGER (other than 0/1).  Does the standard define whether something
> is executed or not in:
> logical, parameter :: l = transfer (234, .false.)
> if (l) print *, ".true."
> ?  I.e. is it supposed to be .false. or .true., or undefined behavior?

Even if we were to accept that the double-transfer idiom was legal, the
only thing you could legally do with such a LOGICAL value is TRANSFER it
to something else.  Anything else is undefined behavior.

Note that assigning the result of transfer(234, .false.) to a logical
variable, in the above example, is undefined behavior.  It seems fair to
me to declare that it normalizes the result to a standard .true. or
.false. value, even if we were to preserve the weird double-transfer
semantics.

- Brooks

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Jakub Jelinek]

On Tue, Nov 11, 2008 at 10:54:11AM -0800, Brooks Moses wrote:
> So, given that we seem to have found that the costs of using all 8
> bytes, or 4 bytes, or whatever, are rather higher than otherwise
> expected.  As a result, I suppose that the tradeoff is that we go with
> what might be considered a poor quality of implementation in this case,
> in return for a better quality of implementation in the case of anything
> that uses logical variables sensibly.

As I said earlier, transfer (transfer (234, .false.), 0) is already
identity, as that never goes through middle-end and BOOLEAN_TYPE.
If assigning transfer (234, .false.) to logical is undefined behavior,
then the only problematic case is
integer :: i, j
i = 234
...
j = transfer (transfer (i, .false.), 0)
(when it is non-constant).  Currently the FE expands this as
2 separate transfers and the optimizers will be able to see through them,
but I'd say the FE could look through the
transfer in the first argument and do whatever the standard requires,
before handing it to the middle-end.  In this case change it to
j = transfer (i, 0) (or just i).

	Jakub

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Brooks Moses]

Jakub Jelinek wrote, at 11/11/2008 11:12 AM:
> As I said earlier, transfer (transfer (234, .false.), 0) is already
> identity, as that never goes through middle-end and BOOLEAN_TYPE.
> If assigning transfer (234, .false.) to logical is undefined behavior,
> then the only problematic case is
> integer :: i, j
> i = 234
> ...
> j = transfer (transfer (i, .false.), 0)
> (when it is non-constant).  Currently the FE expands this as
> 2 separate transfers and the optimizers will be able to see through them,
> but I'd say the FE could look through the
> transfer in the first argument and do whatever the standard requires,
> before handing it to the middle-end.  In this case change it to
> j = transfer (i, 0) (or just i).

We could, yes.  At this point, I don't think it's worth the bother (and
risk of additional bugs) of implementing it, until we get a case where
someone actually complains about it.  (And, frankly, I wish I hadn't
implemented it for the constant-folding case; as I recall, it made the
code rather a good bit more complex, and from today's perspective I
don't see the point.)

It might be useful to put something explicit into the TRANSFER
documentation pointing out the undefined behavior in the case of LOGICAL
 values for the MOLD argument, though.

- Brooks

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Tobias Burnus]

Hi,

Jakub Jelinek wrote:
> If using logicals is required to store all the bits of the constants
> (and similarly for TRANSFER from integer to logical), then I'm afraid the
> Fortran FE would have to stop using BOOLEAN_TYPE for logical, as the
> middle-end BOOLEAN_TYPE really has the semantics that it is either false or
> true, nothing else.
>   

According to the standard, what TRANSFER does is implementation
dependent (though one can assume that most of the time the bits are
transferred one-to-one).

In words of the standard: "Result Value. If the physical representation
of the result has the same length as that of SOURCE, the physical
representation of the result is that of SOURCE." The point is: The
standard does not tell what is meant by "physical representation"; I
read it such that the standard does not 100% require that an integer ->
logical -> integer round trip yields the same result. (Though it should
if possible.=

In any case, I don't see any use in doing such round trips - and I also
do not see any use in TRANSFERring anything to logical as it is
completely opaque which bit pattern is regarded as .true. and what as
.false. For instance:

- gfortran (and C) use "== 0" for .false. and "!= 0" for .true. where by
default transfer(.true., 0) == 1.
- Intel's ifort has the same 0 and 1 defaults, but e.g. "2" is FALSE
- (other compilers might have different choices, taking only the
highest/lowest bit, or ...)

(I think TRANSFERring to logical is based on the misconception that
INTEGER and LOGICAL are internally the same data type, similarly to
_Bool and int in C.)

* * *

Regarding Holleriths: They were added in Fortran 66 while LOGICAL exists
since Fortran IV. In Fortran 77 the CHARACTER type was introduced and
Hollerith was deleted from the standard (but it is still listed in
Appendix C). Appendix C has: "Hollerith data, other than constants, are
identified under the guise of a name of type integer, real, or logical."
(See http://gcc.gnu.org/wiki/GFortranStandards for links to the various
Fortran standards.)

Thus that "logical = 4Ho wo" yielding "1" _is_ a problem for certain old
code, though as Holleriths were deleted, this conversion is OK from the
standard perspective.

I assume that by far the most Fortran programs use INTEGER for
Holleriths, but I don't rule out the possibility that some use LOGICAL.

* * *

You see that I'm mainly concerned about Holleriths and less about "(some
type) -> logical -> (some type)" transfers.

I'm wondering whether one needs some special flag to use optionally
internally integers instead of booleans for LOGICAL, with a warning
printed if the option is disabled (as it should be by default) in case a
TRANSFER to a LOGICAL or an assignment of a Hollerith to a logical
variable is found.

But as default I'm in favour to keep the status quo in the front end:
Use the middle end's boolean type for LOGICAL. And I'm in favour of
doing the folding as Jakub's patch does.

Tobias

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Paul Richard Thomas]

Tobi and Jakub,

> They are not standard Fortran.  Using Hollerith constants this way was the
> way of encoding strings before there was a Character type in Fortran, so the
> current behavior is intentional.  Whether there actually is code that uses
> logicals to encode strings, I can't tell.  I CCed Feng Wang who added the
> original Hollerith support, and Steve who last modified the testcase.

I am not so sure about that in the case of transfer_simplify_4.f90 -
we have already had serious amounts of correspondence about it as
witnessed in PR33759.  What the standard says is:
>
13.14.110 TRANSFER (SOURCE, MOLD [, SIZE])
Description. Returns a result with a physical representation identical
to that of SOURCE but interpreted with the type and type parameters of
MOLD.
Class. Transformational function.
Arguments.
SOURCE may be of any type and may be scalar or array valued.
MOLD may be of any type and may be scalar or array valued.
SIZE (optional) shall be scalar and of type integer. The corresponding
actual argument shall not be an optional dummy argument.
Result Characteristics. The result is of the same type and type
parameters as MOLD.
Case (i): If MOLD is a scalar and SIZE is absent, the result is a scalar.
Case (ii): If MOLD is array valued and SIZE is absent, the result is
array valued and of rank one. Its size is as small as possible such
that its physical representation is not shorter than that of SOURCE.
Case (iii): If SIZE is present, the result is array valued of rank one
and size SIZE.

Result Value. If the physical representation of the result has the
same length as that of SOURCE, the physical representation of the
result is that of SOURCE. If the physical representation of the result
is longer than that of SOURCE, the physical representation of the
leading part is that of SOURCE and the remainder is undefined. If the
physical representation of the result is shorter than that of SOURCE,
the physical representation of the result is the leading part of
SOURCE. If D and E are scalar variables such that the physical
representation of D is as long as or longer than that of E, the value
of TRANSFER (TRANSFER (E, D), E) shall be the value of E. IF D is an
array and E is an array of rank one, the value of TRANSFER (TRANSFER
(E, D), E, SIZE (E)) shall be the value of E.

or... as Jakub paraphrases it:

> (4Ho wo is non-zero).  The transfer_simplify_4.f90 testcase transfers an
> integer into logical and back and expects again all the 32-bits to be
> preserved.

I would say that it conforms to the standard.

Cheers

Paul

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Tobias Schlüter]

Paul Richard Thomas wrote:
> Tobi and Jakub,
> 
>> They are not standard Fortran.  Using Hollerith constants this way was the
>> way of encoding strings before there was a Character type in Fortran, so the
>> current behavior is intentional.  Whether there actually is code that uses
>> logicals to encode strings, I can't tell.  I CCed Feng Wang who added the
>> original Hollerith support, and Steve who last modified the testcase.
> 
> I am not so sure about that in the case of transfer_simplify_4.f90 -
> we have already had serious amounts of correspondence about it as
> witnessed in PR33759.  What the standard says is:

Yes sorry, I phrased this wrongly, I had intended to only talk about the 
use of Hollerith constants.  But this is moot because due to what both 
Jakub and you rightfully pointed out, LOGICAL will likely have to seize 
being a BOOLEAN_TYPE.  Which in turn will restore the traditional 
Hollerith behavior.  If all goes well.

Cheers,
- Tobi

Re: [PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Steve Kargl]

On Tue, Nov 11, 2008 at 04:45:40PM +0100, Tobias Schlüter wrote:
> Jakub Jelinek wrote:
> >Unfortrunately, it causes 2 Fortran testsuite failures, hollerith.f90
> >and transfer_simplify_4.f90, both at all optimization levels.
> >I believe the tests are just invalid though.  The first one does:
> >logical l
> >l = 4Ho wo
> >and expects all the 32 bits preserved, but that is certainly against
> >the semantics of BOOLEAN_TYPE and from quick skimming of the Fortran
> >standard also LOGICAL type.  BOOLEAN_TYPE has just two values, false
> >and true (similarly for LOGICAL .false. and .true.) and so the folder
> >IMHO correctly folds this into
> >l = .true.
> >(4Ho wo is non-zero).  The transfer_simplify_4.f90 testcase transfers an
> >integer into logical and back and expects again all the 32-bits to be
> >preserved.
> >
> >Fortran folks, can you please look at these 2 testcases and say whether
> >they are valid Fortran or just undefined behavior?
> 
> They are not standard Fortran.  Using Hollerith constants this way was 
> the way of encoding strings before there was a Character type in 
> Fortran, so the current behavior is intentional.  Whether there actually 
> is code that uses logicals to encode strings, I can't tell.  I CCed Feng 
> Wang who added the original Hollerith support, and Steve who last 
> modified the testcase.
> 

My changes in hollerith.f90 were of the form 'complex*16 --> complex(kind=8)'.
It would not have any affect on Jakub proposed patched.

In reviewing the thread, it appears that gfortran may need
to abandon BOOLEAN_TYPE because the description of TRANSFER is
quite clear:

   If D and E are scalar variables such that the physical representation 
   of D is as long as or longer than that of E, the value of
   TRANSFER (TRANSFER (E, D), E) shall be the value of E. 

unless gfortran can provide a special case for the above.  Also, note
that

   integer f, e
   logical d
   e = 42
   f = transfer(transfer(e,d),e)
   if (f /= e) abort()
   end

and

   integer f, e
   logical d
   e = 42
   d = transfer(e,d) 
   f = transfer(d,e)
   if (f /= e) abort()
   end

can be argued to have different results because the latter has two 
explicitly assignments and hence type conversions whereas the 
former has only a single assignment with an accompanying type 
conversion.

-- 
Steve