[Bug tree-optimization/60183] [4.7/4.8/4.9 Regression] phiprop creates invalid code

[Bug tree-optimization/60183] [4.7/4.8/4.9 Regression] phiprop creates invalid code

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.4

[Bug tree-optimization/60183] [4.7/4.8/4.9 Regression] phiprop creates invalid code

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2014-02-14
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Doesn't abort for me.  phiprop does

Inserting PHI for result of load _6 = *y_5;
  for edge defining &c inserting load _19 = MEM[(long unsigned int *)&c];
  for edge defining y_13 inserting load _20 = *y_13;
_6 = PHI <_19(2), _20(3)>

on

  <bb 2>:
  w_4 = *x_2(D);
  goto <bb 4>;

  <bb 3>:
  _6 = *y_5;
  w_8 = _6 ^ w_7;
  _9 = MEM[(long unsigned int *)y_5 + 8B];
  w_10 = w_8 + _9;
  _11 = MEM[(long unsigned int *)y_5 + 16B];
  w_12 = w_10 ^ _11;
  y_13 = &MEM[(void *)y_5 + 32B];
  _14 = MEM[(long unsigned int *)y_5 + 24B];
  w_15 = w_12 + _14;
  i_17 = i_16 + 1;

  <bb 4>:
  # y_5 = PHI <&c(2), y_13(3)>

which results in

  <bb 2>:
  w_4 = *x_2(D);
  _19 = MEM[(long unsigned int *)&c];
  goto <bb 4>;

  <bb 3>:
  w_8 = _6 ^ w_7;
  _9 = MEM[(long unsigned int *)y_5 + 8B];
  w_10 = w_8 + _9;
  _11 = MEM[(long unsigned int *)y_5 + 16B];
  w_12 = w_10 ^ _11;
  y_13 = &MEM[(void *)y_5 + 32B];
  _14 = MEM[(long unsigned int *)y_5 + 24B];
  w_15 = w_12 + _14;
  i_17 = i_16 + 1;
  _20 = *y_13;

  <bb 4>:
  # y_5 = PHI <&c(2), y_13(3)>
  # _6 = PHI <_19(2), _20(3)>

I see nothing wrong with that.  Ah, but the load is conditional on i <= 7,
so we effectively speculate it.

Mine.

[Bug tree-optimization/60183] [4.7/4.8/4.9 Regression] phiprop creates invalid code

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Updated testcase that segfaults for me, no -fsanitize=address is then needed to
trigger it.  Works with -O0 or -O? -fno-tree-phiprop:

unsigned char c[0x300001] = { 1 };
int j = 2;

static void
foo (unsigned long *x, unsigned char *y)
{
  int i;
  unsigned long w = x[0];
  for (i = 0; i < j; i++)
    {
      w += *y;
      y += 0x100000;
      w += *y;
      y += 0x100000;
    }
  x[1] = w;
}

__attribute__ ((noinline, noclone)) void
bar (unsigned long *x)
{
  foo (x, c);
}

int
main ()
{
  unsigned long a[2] = { 0, -1UL };
  asm volatile (""::"r" (c):"memory");
  c[0] = 0;
  bar (a);
  if (a[1] != 0)
    __builtin_abort ();
  return 0;
}

Perhaps phiprop is confused by the &MEM[(void *)y_5 + 2097152B] and thinks that
because of the MEM_REF in there it is safe to dereference it?

[Bug tree-optimization/60183] [4.7/4.8/4.9 Regression] phiprop creates invalid code

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> Updated testcase that segfaults for me, no -fsanitize=address is then needed
> to trigger it.  Works with -O0 or -O? -fno-tree-phiprop:
> 
> unsigned char c[0x300001] = { 1 };
> int j = 2;
> 
> static void
> foo (unsigned long *x, unsigned char *y)
> {
>   int i;
>   unsigned long w = x[0];
>   for (i = 0; i < j; i++)
>     {
>       w += *y;
>       y += 0x100000;
>       w += *y;
>       y += 0x100000;
>     }
>   x[1] = w;
> }
> 
> __attribute__ ((noinline, noclone)) void
> bar (unsigned long *x)
> {
>   foo (x, c);
> }
> 
> int
> main ()
> {
>   unsigned long a[2] = { 0, -1UL };
>   asm volatile (""::"r" (c):"memory");
>   c[0] = 0;
>   bar (a);
>   if (a[1] != 0)
>     __builtin_abort ();
>   return 0;
> }
> 
> Perhaps phiprop is confused by the &MEM[(void *)y_5 + 2097152B] and thinks
> that because of the MEM_REF in there it is safe to dereference it?

It doesn't check whether it's safe to dereference because it thinks it's
dereferenced anyway.  It wasn't supposed to speculate loads.  We miss

Index: tree-ssa-phiprop.c
===================================================================
--- tree-ssa-phiprop.c  (revision 207757)
+++ tree-ssa-phiprop.c  (working copy)
@@ -309,6 +309,10 @@ propagate_with_phi (basic_block bb, gimp
       gimple def_stmt;
       tree vuse;

+      /* Only replace loads in the same block as the PHI node.  */
+      if (gimple_bb (use_stmt) != bb)
+       continue;
+         
       /* Check whether this is a load of *ptr.  */
       if (!(is_gimple_assign (use_stmt)
            && TREE_CODE (gimple_assign_lhs (use_stmt)) == SSA_NAME

or really a post-dominator check - but we don't compute post-dominators
and I'm not sure it would be worth doing that.

[Bug tree-optimization/60183] [4.7/4.8/4.9 Regression] phiprop creates invalid code

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ok, we do need the post-dominators to avoid

FAIL: g++.dg/tree-ssa/pr21463.C -std=gnu++98  scan-tree-dump-times phiopt1
"MIN_
EXPR" 2
FAIL: g++.dg/tree-ssa/pr21463.C -std=gnu++98  scan-tree-dump-times phiopt1
"MAX_
EXPR" 2
FAIL: g++.dg/tree-ssa/pr21463.C -std=gnu++11  scan-tree-dump-times phiopt1
"MIN_
EXPR" 2
FAIL: g++.dg/tree-ssa/pr21463.C -std=gnu++11  scan-tree-dump-times phiopt1
"MAX_
EXPR" 2
FAIL: g++.dg/tree-ssa/pr57380.C -std=gnu++98  scan-tree-dump phiopt1 "MAX_EXPR"
FAIL: g++.dg/tree-ssa/pr57380.C -std=gnu++11  scan-tree-dump phiopt1 "MAX_EXPR"

Re-testing that variant.

[Bug tree-optimization/60183] [4.7/4.8/4.9 Regression] phiprop creates invalid code

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Sat Feb 15 09:54:52 2014
New Revision: 207797

URL: http://gcc.gnu.org/viewcvs?rev=207797&root=gcc&view=rev
Log:
2014-02-15  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/60183
    * tree-ssa-phiprop.c (propagate_with_phi): Avoid speculating
    loads.
    (tree_ssa_phiprop): Calculate and free post-dominators.

    * gcc.dg/torture/pr60183.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr60183.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-phiprop.c

[Bug tree-optimization/60183] [4.7/4.8 Regression] phiprop creates invalid code

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Tue Feb 25 10:47:21 2014
New Revision: 208118

URL: http://gcc.gnu.org/viewcvs?rev=208118&root=gcc&view=rev
Log:
2014-02-25  Richard Biener  <rguenther@suse.de>

    Backport from mainline
    2014-02-21  Richard Biener  <rguenther@suse.de>

    PR middle-end/60291
    * tree-ssa-live.c (mark_all_vars_used_1): Do not walk
    DECL_INITIAL for globals not in the current function context.

    2014-02-20  Richard Biener  <rguenther@suse.de>

    PR middle-end/60221
    * tree-eh.c (execute_cleanup_eh_1): Also cleanup empty EH
    regions at -O0.

    2014-02-14  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/60183
    * tree-ssa-phiprop.c (propagate_with_phi): Avoid speculating
    loads.
    (tree_ssa_phiprop): Calculate and free post-dominators.

    * gcc.dg/torture/pr60183.c: New testcase.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/torture/pr60183.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/tree-eh.c
    branches/gcc-4_8-branch/gcc/tree-ssa-live.c
    branches/gcc-4_8-branch/gcc/tree-ssa-phiprop.c

[Bug tree-optimization/60183] [4.7 Regression] phiprop creates invalid code

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.8.3
            Summary|[4.7/4.8 Regression]        |[4.7 Regression] phiprop
                   |phiprop creates invalid     |creates invalid code
                   |code                        |
      Known to fail|                            |4.8.2

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
And for 4.8.3.

[Bug tree-optimization/60183] [4.7 Regression] phiprop creates invalid code

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Mon Mar 17 14:38:55 2014
New Revision: 208618

URL: http://gcc.gnu.org/viewcvs?rev=208618&root=gcc&view=rev
Log:
2014-03-17  Richard Biener  <rguenther@suse.de>

    Backport from mainline
    2013-05-21  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/57303
    * tree-ssa-sink.c (statement_sink_location): Properly handle
    self-assignments.

    * gcc.dg/torture/pr57303.c: New testcase.

    2013-12-02  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/59139
    * tree-ssa-loop-niter.c (chain_of_csts_start): Properly match
    code in get_val_for.
    (get_val_for): Use gcc_checking_asserts.

    * gcc.dg/torture/pr59139.c: New testcase.

    2014-02-14  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/60183
    * tree-ssa-phiprop.c (propagate_with_phi): Avoid speculating
    loads.
    (tree_ssa_phiprop): Calculate and free post-dominators.

    * gcc.dg/torture/pr60183.c: New testcase.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr57303.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr59139.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr60183.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-ssa-loop-niter.c
    branches/gcc-4_7-branch/gcc/tree-ssa-phiprop.c
    branches/gcc-4_7-branch/gcc/tree-ssa-sink.c

[Bug tree-optimization/60183] [4.7 Regression] phiprop creates invalid code

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.