[Bug middle-end/65504] New: [4.9/5 Regression] select case with strings and -fgcse -O

[Bug middle-end/65504] New: [4.9/5 Regression] select case with strings and -fgcse -O

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

            Bug ID: 65504
           Summary: [4.9/5 Regression] select case with strings and -fgcse
                    -O
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tkoenig at gcc dot gnu.org

Original bug report from

https://stackoverflow.com/questions/29163373/why-do-i-have-random-return-values-in-my-type-in-fortran-with-o2
by "Sebastian".

The following program

program testbug
  implicit none
  integer, parameter :: STRLEN = 256
  type :: ncVarnames_t
    ! variables names for the different ice models
    character (len=STRLEN) :: surf
    character (len=STRLEN) :: x, y
    character (len=STRLEN) :: bed
    character (len=STRLEN) :: thick
  end type ncVarnames_t

  type (ncVarnames_t) :: testvar
  type (ncVarnames_t) :: testvar2

  print *, "hello"

  testvar = get_ncVarnames ("test")
  testvar2 = get_ncVarnames ("test")

  print *, trim(testvar%surf)
  print *, trim(testvar%bed)
  print *, trim(testvar%bed)
  print *, trim(testvar%surf)

  contains
  type (ncVarnames_t) function get_ncVarnames (model) result (v)
    character(len=*), intent(in) :: model
    ! type (ncVarnames_t)          :: v

    select case (model)
      case ("test")
        ! test model
        v%x = 'x'
        v%y = 'y'
        v%surf = 'surf'
        v%bed = 'bed'
       case ("pism")
         ! pism data
         v%x = 'x'
         v%y = 'y'
         v%surf = 'usurf'
         v%bed = 'topg'
      case default
        print *, "unknown model, please use one of [test pism sico]"
        stop
    end select
  end function get_ncVarnames
end program testbug

prints random values with 4.9 and 5.0 when invoked with -O -fgcse or with -O2,
but not with 4.8 or when -fgcse is missing.  Valgrind complains with

==4327== Syscall param write(buf) points to uninitialised byte(s)
==4327==    at 0x522FE80: __write_nocancel (in /lib64/libc-2.18.so)
==4327==    by 0x4F1B6B7: raw_write (unix.c:323)
==4327==    by 0x4F2234E: _gfortrani_fbuf_flush (unix.h:59)
==4327==    by 0x4F17437: _gfortrani_next_record (transfer.c:3528)
==4327==    by 0x4F19E32: finalize_transfer (transfer.c:3616)
==4327==    by 0x4F19FB8: _gfortran_st_write_done (transfer.c:3741)
==4327==    by 0x400C02: MAIN__ (in /home/ig25/Krempel/Select/a.out)
==4327==    by 0x400D09: main (in /home/ig25/Krempel/Select/a.out)
==4327==  Address 0x5c5fbe1 is 1 bytes inside a block of size 512 alloc'd
==4327==    at 0x4C277AB: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==4327==    by 0x4E4F964: _gfortrani_xmalloc (memory.c:43)
==4327==    by 0x4F221EC: _gfortrani_fbuf_init (fbuf.c:43)
==4327==    by 0x4F1ACFE: _gfortrani_init_units (unit.c:650)
==4327==    by 0x4E4EF67: init (main.c:265)
==4327==    by 0x400E859: call_init.part.0 (in /lib64/ld-2.18.so)
==4327==    by 0x400E942: _dl_init (in /lib64/ld-2.18.so)
==4327==    by 0x40011C9: ??? (in /lib64/ld-2.18.so)

[Bug middle-end/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
   Target Milestone|---                         |4.9.3

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|middle-end                  |rtl-optimization

--- Comment #1 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
gcse is RTL...

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
I don't see it on x86_64-apple-darwin14: I get

 hello
 surf
 bed
 bed
 surf

with all the revisions and options I have tested.

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #3 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
It may be target dependent. I can reproduce this on x86_64-unknown-linux-gnu .
By the way, -fno-gcse cures it.

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #4 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
... and -fsanitize=address also makes the bug go away.  Seems to be quite
picky.

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[howarth at bromo dot med.uc.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

howarth at bromo dot med.uc.edu changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |howarth at bromo dot med.uc.edu

--- Comment #5 from howarth at bromo dot med.uc.edu ---
(In reply to Dominique d'Humieres from comment #2)
> I don't see it on x86_64-apple-darwin14: I get
> 
>  hello
>  surf
>  bed
>  bed
>  surf
> 
> with all the revisions and options I have tested.

On x86_64-apple-darwinw14, using '-O2 -mtune=generic' switches the output to...

 hello
 surf
 d
 d
 surf

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[howarth at bromo dot med.uc.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #6 from howarth at bromo dot med.uc.edu ---
(In reply to howarth from comment #5)

Also, as on linux, using '-O2 -mtune=generic -fsanitize=address' suppresses the
error.

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-03-21
     Ever confirmed|0                           |1

--- Comment #7 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> On x86_64-apple-darwinw14, using '-O2 -mtune=generic' switches the output to...
>
>  hello
>  surf
>  d
>  d
>  surf

Confirmed.

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #8 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Reduced test case:

program testbug
  implicit none
  integer, parameter :: STRLEN = 256
  type :: ncVarnames_t
    ! variables names for the different ice models
    character (len=STRLEN) :: surf
    character (len=STRLEN) :: bed
  end type ncVarnames_t

  type (ncVarnames_t) :: testvar
  type (ncVarnames_t) :: testvar2

  testvar = get_ncVarnames ("test")
  testvar2 = get_ncVarnames ("test")

  print *, trim(testvar%bed)

  contains
  type (ncVarnames_t) function get_ncVarnames (model) result (v)
    character(len=*), intent(in) :: model
    ! type (ncVarnames_t)          :: v

    select case (model)
    case ("test")
       ! test model
       v%bed = 'bed'
     case ("pism")
        ! pism data
        v%bed = 'topg'
     case default
        print *, "unknown model, please use one of [test pism sico]"
        stop
    end select
  end function get_ncVarnames
end program testbug

You need two cases in the select case statement, you need two calls to
get_ncVarnames, and you need the two components in the type.

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[howarth at bromo dot med.uc.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #9 from howarth at bromo dot med.uc.edu ---
Created attachment 35085
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35085&action=edit
assembly from reduced testcase at -O2 -mtune=generic

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[howarth at bromo dot med.uc.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #10 from howarth at bromo dot med.uc.edu ---
Created attachment 35086
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35086&action=edit
assembly from reduced testcase at -O2 -mtune=core2

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[howarth at bromo dot med.uc.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #11 from howarth at bromo dot med.uc.edu ---
Created attachment 35087
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35087&action=edit
diff of assembly from reduced testcase at -O2 -mtune=core2 and -O2
-mtune=generic

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[sebastian.beyer at riseup dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #12 from Sebastian Beyer <sebastian.beyer at riseup dot net> ---
Hi, I'm the author of the original post on stackoverflow.
I am not sure if this is relevant, but I can suppress the error if I add an
arbitrary print statement at the end of the function.
Also changing the number of STRLEN can make it work again.
For example STRLEN=255 works, also 254 and 257 do, but 1024 does not.

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org

--- Comment #13 from H.J. Lu <hjl.tools at gmail dot com> ---
It was caused by r204830.

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |ebotcazou at gcc dot gnu.org

--- Comment #14 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> It was caused by r204830.

This looks more like a latent issue to me, investigating...

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #15 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
It's a bug in Dead Store Elimination (so -fno-dse is the workaround) exposed by
a combination of GCSE and FWPROP which changes the canonicalization of
addresses (so -fno-gcse and -fno-forward-propagate paper over the bug).

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug rtl-optimization/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
--- gcc/config/i386/i386.c.jj    2015-03-23 08:47:53.000000000 +0100
+++ gcc/config/i386/i386.c    2015-03-23 13:11:43.416147457 +0100
@@ -23457,12 +23457,19 @@ counter_mode (rtx count_exp)
 static rtx
 ix86_copy_addr_to_reg (rtx addr)
 {
+  rtx reg;
   if (GET_MODE (addr) == Pmode || GET_MODE (addr) == VOIDmode)
-    return copy_addr_to_reg (addr);
+    {
+      reg = copy_addr_to_reg (addr);
+      REG_POINTER (reg) = 1;
+      return reg;
+    }
   else
     {
       gcc_assert (GET_MODE (addr) == DImode && Pmode == SImode);
-      return gen_rtx_SUBREG (SImode, copy_to_mode_reg (DImode, addr), 0);
+      reg = copy_to_mode_reg (DImode, addr);
+      REG_POINTER (reg) = 1;
+      return gen_rtx_SUBREG (SImode, reg, 0);
     }
 }

@@ -24354,6 +24361,8 @@ expand_set_or_movmem_prologue_epilogue_b
       *destptr = expand_simple_binop (GET_MODE (*destptr), PLUS, *destptr,
                       GEN_INT (prolog_size),
                       NULL_RTX, 1, OPTAB_DIRECT);
+      if (REG_P (*destptr) && REG_P (saveddest) && REG_POINTER (*saveddest))
+    REG_POINTER (*destptr) = 1;
       *destptr = expand_simple_binop (GET_MODE (*destptr), AND, *destptr,
                       GEN_INT (-desired_align),
                       *destptr, 1, OPTAB_DIRECT);
@@ -24363,8 +24372,8 @@ expand_set_or_movmem_prologue_epilogue_b
                        saveddest, 1, OPTAB_DIRECT);
       /* Adjust srcptr and count.  */
       if (!issetmem)
-    *srcptr = expand_simple_binop (GET_MODE (*srcptr), MINUS, *srcptr,
saveddest,
-                    *srcptr, 1, OPTAB_DIRECT);
+    *srcptr = expand_simple_binop (GET_MODE (*srcptr), MINUS, *srcptr,
+                       saveddest, *srcptr, 1, OPTAB_DIRECT);
       *count = expand_simple_binop (GET_MODE (*count), PLUS, *count,
                     saveddest, *count, 1, OPTAB_DIRECT);
       /* We copied at most size + prolog_size.  */

fixes this for me.  Without the REG_POINTER flags, aliasing is really confused
on what is a pointer and what is not, as the prologue of the misaligned memcpy
uses
adjusteddest = dest & -align;
src = src - (adjusteddest - dest)
and from these only one has REG_POINTER flag after the propagation, so aliasing
makes a wrong decision from that.

[Bug target/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 35108
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35108&action=edit
gcc5-pr65504.patch

Patch I'm going to bootstrap/regtest now.

[Bug target/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
           Assignee|ebotcazou at gcc dot gnu.org       |unassigned at gcc dot gnu.org

[Bug target/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

[Bug target/65504] [4.9/5 Regression] select case with strings and -fgcse -O

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Mon Mar 23 15:31:59 2015
New Revision: 221597

URL: https://gcc.gnu.org/viewcvs?rev=221597&root=gcc&view=rev
Log:
    PR target/65504
    * config/i386/i386.c (ix86_copy_addr_to_reg): Set REG_POINTER
    on the pseudo.
    (expand_set_or_movmem_prologue_epilogue_by_misaligned_moves): Set
    REG_POINTER on *destptr after adjusting it for prologue size.

    * gfortran.dg/pr65504.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/pr65504.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/testsuite/ChangeLog

[Bug target/65504] [4.9 Regression] select case with strings and -fgcse -O

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.9/5 Regression] select   |[4.9 Regression] select
                   |case with strings and       |case with strings and
                   |-fgcse -O                   |-fgcse -O

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

[Bug target/65504] [4.9 Regression] select case with strings and -fgcse -O

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Wed Jun  3 15:30:58 2015
New Revision: 224089

URL: https://gcc.gnu.org/viewcvs?rev=224089&root=gcc&view=rev
Log:
2015-06-03  Jakub Jelinek  <jakub@redhat.com>

        Backported from mainline
        2015-03-23  Jakub Jelinek  <jakub@redhat.com>

        PR target/65504
        * config/i386/i386.c (ix86_copy_addr_to_reg): Set REG_POINTER
        on the pseudo.
        (expand_set_or_movmem_prologue_epilogue_by_misaligned_moves): Set
        REG_POINTER on *destptr after adjusting it for prologue size.

        * gfortran.dg/pr65504.f90: New test.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/gfortran.dg/pr65504.f90
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/config/i386/i386.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog

[Bug target/65504] [4.9 Regression] select case with strings and -fgcse -O

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65504

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #21 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 4.9.3+.