[Bug fortran/52861] New: (missed optimisation) missed transformation to memset with -O3

[Bug fortran/52861] New: (missed optimisation) missed transformation to memset with -O3

[arnaud02 at users dot sourceforge.net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52861

             Bug #: 52861
           Summary: (missed optimisation) missed transformation to memset
                    with -O3
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: fortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: arnaud02@users.sourceforge.net


This is a minor missed-optimisation noticed as follow up to PR52835.
This is not a regression.

>cat qq1.f
      SUBROUTINE QQ2( ISU, ZSPM, RSV, MS )
      INTEGER :: ISU(MS)
      CHARACTER(8) :: ZSPM(MS)
      REAL :: RSV(MS)

      DO IS=1,MS
         ISU(IS)=0
         ZSPM(IS)=' '
         RSV(IS) =0.0
      ENDDO
      END subroutine qq2
>cat qq2.f
      SUBROUTINE QQ2( ISU, ZSPM, RSV, MS )
      INTEGER :: ISU(MS)
      CHARACTER(8) :: ZSPM(MS)
      REAL :: RSV(MS)

      DO IS=1,MS
         ISU(IS)=0
         RSV(IS) =0.0
      ENDDO
      DO IS=1,MS
         ZSPM(IS)=' '
      ENDDO
      END subroutine qq2
>gfortran470 -O3 -c qq1.f ; gfortran470 -O3 -c qq2.f
>nm qq1.o qq2.o

qq1.o:
0000000000000000 T qq2_

qq2.o:
                 U memset
0000000000000000 T qq2_

The compiler has not be able to transform the assignments to ISU and RSV as
calls to memset when the loop contains an assignment to a CHARACTER variable.
This suggests that the dependency generator can be improved or that the Fortran
front-end could pass a better representation to the middle-end.

Additionally, it should be possible to transform 
      DO IS=1,MS
         ZSPM(IS)=' '
      ENDDO
as a call to memset. gcc 4.7.0 cannot:

>cat qq3.f
      SUBROUTINE QQ2( ZSPM, MS )
      CHARACTER(8) :: ZSPM(MS)
      DO IS=1,MS
         ZSPM(IS)=' '
      ENDDO
      END subroutine qq2
>/usr/local/gcc/gfortran470 -O3 -c qq3.f
>nm qq3
qq3.f  qq3.o
>nm qq3.o
0000000000000000 T qq2_

[Bug middle-end/52861] (missed optimisation) missed transformation to memset with -O3

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52861

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
                 CC|                            |burnus at gcc dot gnu.org
          Component|fortran                     |middle-end

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-04-04 16:35:18 UTC ---
Regarding the first test cases:

For qq1.f, the loop remains and one gets in the loop:
    MEM[base: isu_34(D), index: ivtmp.19_49, step: 4, offset: 0B] = 0;
    __builtin_memcpy (D.1901_38, &"        "[0], 8);
    MEM[base: rsv_45(D), index: ivtmp.19_49, step: 4, offset: 0B] = 0.0;


For qq2, the first loop is replaced by:
    __builtin_memset (isu_35(D), 0, D.1935_54);
    __builtin_memset (rsv_38(D), 0, D.1935_54);
while for the character string, the loop is kept.



For the rather common (Fortran) case of assigning blanks (" ") to a string (cf.
also the third test case), it would be good to replace the loop
    __builtin_memcpy (D.1909_46, &"        "[0], 8);
by (outside the loop)
    __builtin_memset (D.1909_46, " ", <array-size>*<string-length>);

[Bug fortran/52861] (missed optimisation) missed transformation to memset with -O3

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52861

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-06-06
          Component|middle-end                  |fortran
     Ever Confirmed|0                           |1

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-06 11:10:00 UTC ---
memset recognition is confused by the memcpy the frontend emits
for ZSPM(IS) = ' '.  data-reference/dependence analysis does not hande this.
Why does the frontend not emit an assignment for this?  Thus, simply

  *D.1909_46 = "        ";

?

[Bug fortran/52861] (missed optimisation) missed transformation to memset with -O3

[tkoenig at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52861

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |tkoenig at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #3 from Thomas Koenig <tkoenig at gcc dot gnu.org> 2012-06-06 18:08:10 UTC ---
This should be rather simple.  Not-yet-regression-tested patch:

Index: frontend-passes.c
===================================================================
--- frontend-passes.c   (Revision 188138)
+++ frontend-passes.c   (Arbeitskopie)
@@ -37,6 +37,7 @@ static bool optimize_comparison (gfc_expr *, gfc_i
 static bool optimize_trim (gfc_expr *);
 static bool optimize_lexical_comparison (gfc_expr *);
 static void optimize_minmaxloc (gfc_expr **);
+static bool empty_string (gfc_expr *e);

 /* How deep we are inside an argument list.  */

@@ -734,11 +735,17 @@ optimize_assignment (gfc_code * c)
   lhs = c->expr1;
   rhs = c->expr2;

-  /* Optimize away a = trim(b), where a is a character variable.  */
-
   if (lhs->ts.type == BT_CHARACTER)
-    remove_trim (rhs);
+    {
+      /* Optimize away a = trim(b), where a is a character variable.  */
+      remove_trim (rhs);

+      /* Replace a = ' ' by a = '' to optimize away a memcpy.  */
+
+      if (empty_string(rhs))
+       rhs->value.character.length = 0;
+    }
+
   if (lhs->rank > 0 && gfc_check_dependency (lhs, rhs, true) == 0)
     optimize_binop_array_assignment (c, &rhs, false);

[Bug fortran/52861] (missed optimisation) missed transformation to memset with -O3

[tkoenig at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52861

--- Comment #4 from Thomas Koenig <tkoenig at gcc dot gnu.org> 2012-06-07 11:12:02 UTC ---
Author: tkoenig
Date: Thu Jun  7 11:11:55 2012
New Revision: 188300

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188300
Log:
2012-06-07  Thomas König  <tkoenig@gcc.gnu.org>

    PR fortran/52861
    * frontend-passes (empty_string):  Add prototype.
    (optimize_assignment):  Set the length of an empty string
    constant to zero.

2012-06-07  Thomas König  <tkoenig@gcc.gnu.org>

    PR fortran/52861
    * gfortran.dg/string_assign_1.f90:  New test case.


Added:
    trunk/gcc/testsuite/gfortran.dg/string_assign_1.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/frontend-passes.c
    trunk/gcc/testsuite/ChangeLog

[Bug fortran/52861] (missed optimisation) missed transformation to memset with -O3

[tkoenig at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52861

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #5 from Thomas Koenig <tkoenig at gcc dot gnu.org> 2012-06-07 11:13:13 UTC ---
Fixed on trunk, closing.

Thanks for the bug report!

[Bug fortran/52861] (missed optimisation) missed transformation to memset with -O3

[tkoenig at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52861

--- Comment #6 from Thomas Koenig <tkoenig at gcc dot gnu.org> 2012-06-07 14:33:56 UTC ---
Author: tkoenig
Date: Thu Jun  7 14:33:51 2012
New Revision: 188305

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188305
Log:
2012-06-07  Thomas König  <tkoenig@gcc.gnu.org>

    PR fortran/52861
    * frontend-passes.c (optimize_assignment):  Don't set the
    length of an empty string for deferred-length character
    variables.

2012-06-07  Thomas König  <tkoenig@gcc.gnu.org>

    PR fortran/52861
    * gfortran.dg/string_assign_2.f90:  New test case.



Added:
    trunk/gcc/testsuite/gfortran.dg/string_assign_2.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/frontend-passes.c
    trunk/gcc/testsuite/ChangeLog

[Bug fortran/52861] (missed optimisation) missed transformation to memset with -O3

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52861

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-06-07 15:25:37 UTC ---
Created attachment 27579
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27579
Proof-of-concept patch (works but has many regression-test failures)

(In reply to comment #5)
> Fixed on trunk, closing.

Well, not completely - only for the special case. I think there is more room
for improvement: Hoisting the memset out of the loop and using ARRAY_RANGE_REF
for the assignments.

 * * *

The attach patch contains a proof-of-concept implementation of ARRAY_RANGE_REF.

The problem is that one needs to strip off several operations on the LHS/RHS in
order to use ARRAY_RANGE_REF. For instance, for a literal, one gets *&"abcd"[1]
but the bare "abcd" is needed (note the ARRAY_REF "[1]"); but one needs to have
just the literal "abcd". For "var(2:0)" one not only needs to strip off the
ARRAY_REF, but one also needs to obtain the offset. While for "string(1)(:)"
and "string(1)(2:3)" the ARRAY_REF for the array has to be kept, but not for
the string part (except for the lower bound). The current version is rather
hacky :-(

Additionally, one runs into the problem that a pointer to a "char" and a
pointer to a "char[:]" aren't the same; many temporary vars have the wrong kind
of "char" type.

While the code works rather nicely for various small test cases, there are many
test-suite failures with the patch. The problem is simply that removing the
(sub)string ARRAY_REF while keeping the array ARRAY_REF is difficult. Fixing
the arguments to gfc_trans_string_copy is also not that simple.

Example for a failing test case:
  character(len=4, kind=1),pointer :: str(:)
  allocate(str(1))
  str(1) = 1_"abcd"     ! Fails
  str(1)(2:) = 1_"abcd" ! Works

[Bug fortran/52861] (missed optimisation) missed transformation to memset with -O3

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52861

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |NEW
         Resolution|FIXED                       |