[Bug tree-optimization/51988] New: value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[Bug tree-optimization/51988] New: value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

             Bug #: 51988
           Summary: value_replacement in PHIOPT should handle even the
                    cases where there are other PHIs even with non equal
                    value
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: pinskia@gcc.gnu.org


Take:
int g(int,int);
int f(int t, int c)
{
  int d = 0;
  int e = 0;
  if (t)
    {
      d = t;
      if (c) e = 1;
    }
  else d = 0, e = 0;
  return g(d,e);
}

--- CUT ---
Currently we get:
<bb 2>:
  if (t_5(D) != 0)
    goto <bb 3>;
  else
    goto <bb 5>;

<bb 3>:
  if (c_7(D) != 0)
    goto <bb 4>;
  else
    goto <bb 5>;

<bb 4>:

<bb 5>:
  # d_1 = PHI <t_5(D)(4), 0(2), t_5(D)(3)>
  # e_2 = PHI <3(4), 0(2), 0(3)>

But we could reduce it down to:
<bb 2>:
  if (t_5(D) != 0)
    goto <bb 3>;
  else
    goto <bb 5>;

<bb 3>:
  if (c_7(D) != 0)
    goto <bb 4>;
  else
    goto <bb 5>;

<bb 4>:

<bb 5>:
  # d_1 = PHI <t_5(D)(4), t_5(D)(2), t_5(D)(3)>
  # e_2 = PHI <3(4), 0(2), 0(3)>

As t_5 on the edge from bb 2 to bb 5, t_5 is 0.

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-01-25
         AssignedTo|unassigned at gcc dot       |pinskia at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-01-25 00:02:18 UTC ---
I am working on this, I don't have a patch yet though.

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-01-25 00:04:01 UTC ---
Note this is like http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01195.html but
different as we should do this even if the arguments for the other PHIs are non
equal.

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-01-25 00:24:41 UTC ---
Here is a better testcase:
int g(int,int);
int f(int t, int c)
{
  int d = 0;
  int e = 0;
  if (t)
    {
      d = 1;
      e = t;
    }
  else d = 0, e = 0;
  return g(d,e);
}

--- CUT ---
I have a simple patch now.

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-01-25 00:26:41 UTC ---
The first testcase is still valid but is not handled currently without some
changes dealing with how PHI-OPT works which I am fixing right now.

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-01-25 00:45:37 UTC ---
Here is another testcase which also fails:
int g(int,int);
int f(int t, int c)
{
  int d = 0;
  int e = 0;
  if (t)
    {
      d = c+1;
      e = t;
    }
  else d = 0, e = 0;
  return g(d,e);
}

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-01-25 00:58:45 UTC ---
I have a patch which fixes the testcases in comment #3 and comment #5.  The
testcase in comment #0 will be fixed with the patch in
http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01195.html and this new patch. 
Here is a testcase which still cannot be done with the other patch:
int g(int,int);
int f(int t, int c)
{
  int d = 0;
  int e = 0;
  if (t)
    {
      d = t;
      if (c) e = 3;
    }
  else d = 0, e = 0;
  return g(d,e);
}
--- CUT ---
I am going to submit the current patch after testing and leave this bug report
open for the above testcase (since the first one is fixed as the patch I
reference here is approved).

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-01-25 21:35:01 UTC ---
Here is another testcase (which passes with the patch I posted but does not
with a patch which I created internally to create COND_EXPR's).
int g(int,int);
int h(int)
int f(int t, int c)
{
  int d = 0;
  int e = 0;
  if (t)
    {
      d = h(c);
      e = t;
    }
  else d = 0, e = 0;
  return g(d,e);
}

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-03-09 09:27:35 UTC ---
Author: pinskia
Date: Fri Mar  9 09:27:29 2012
New Revision: 185131

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185131
Log:
2012-03-09  Andrew Pinski  <apinski@cavium.com>

    PR middle-end/51988
    * tree-ssa-phiopt.c: Include tree-pretty-print.h for
    print_generic_expr.
    (tree_ssa_phiopt_worker): Go through all the PHIs for
    value_replacement instead of just the singleton one.
    (value_replacement): Change return type to int.  Return 0 instead of
    false.
    Allow the middle basic block to contain more than just the definings
    tatement. 
    Handle non empty middle basic blocks.
    * Makefile.in (tree-ssa-phiopt.o): Add tree-pretty-print.h.

2012-03-09  Andrew Pinski  <apinski@cavium.com>

    PR middle-end/51988
    * gcc.dg/tree-ssa/phi-opt-8.c: New testcase.
    * gcc.dg/tree-ssa/phi-opt-9.c: New testcase.


Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-8.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-9.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/Makefile.in
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-phiopt.c

[Bug tree-optimization/51988] value_replacement in PHIOPT should handle even the cases where there are other PHIs even with non equal value

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |4.8.0

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed for a while now.