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

["Richard Guenther" <richard dot guenther at gmail dot com>]

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
>