[Bug tree-optimization/59303] New: [ARM/AArch32//AArch64] regressions in uninit-pred-8_b.c and uninit-pred-9_b.c

[Bug tree-optimization/59303] New: [ARM/AArch32//AArch64] regressions in uninit-pred-8_b.c and uninit-pred-9_b.c

[christophe.lyon at st dot com]

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

            Bug ID: 59303
           Summary: [ARM/AArch32//AArch64] regressions in
                    uninit-pred-8_b.c and uninit-pred-9_b.c
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: christophe.lyon at st dot com

Since commit 204194 from Andrew Pinski:
    2013-10-29  Andrew Pinski <apinski@cavium.com>

            * tree-ssa-ifcombine.c: Include rtl.h and tm_p.h.
            (ifcombine_ifandif): Handle cases where
            maybe_fold_and_comparisons fails, combining the branches
            anyways.
            (tree_ssa_ifcombine): Inverse the order of
            the basic block walk, increases the number of combinings.
            * gimple.h (gsi_start_nondebug_after_labels_bb): New function.

    2013-10-29  Andrew Pinski <apinski@cavium.com>
                Zhenqiang Chen  <zhenqiang.chen@linaro.org>

            * gcc.dg/tree-ssa/ssa-ifcombine-ccmp-1.c: New test case.
            * gcc.dg/tree-ssa/ssa-ifcombine-ccmp-2.c: New test case.
            * gcc.dg/tree-ssa/ssa-ifcombine-ccmp-3.c: New test case.
            * gcc.dg/tree-ssa/ssa-ifcombine-ccmp-4.c: New test case.
            * gcc.dg/tree-ssa/ssa-ifcombine-ccmp-5.c: New test case.
            * gcc.dg/tree-ssa/ssa-ifcombine-ccmp-6.c: New test case.
            * gcc.dg/tree-ssa/phi-opt-9.c: Use a function call to prevent
            conditional move to be used.
            * gcc.dg/tree-ssa/ssa-dom-thread-3.c: Remove.


I have noticed regressions on ARM targets. The 2 mentioned testcases now FAIL
(used to PASS):
  gcc.dg/uninit-pred-8_b.c bogus warning (test for bogus messages, line 20)
  gcc.dg/uninit-pred-9_b.c bogus warning (test for bogus messages, line 24)

It is the case in the following configurations:
targets: arm-none-linux-gnueabi, arm-none-linux-gnueabihf,
armeb-none-linux-gnueabihf
mode: arm,thumb
cpu: cortex-a9
fpu: default, vfpv3, neon-fp16

targets: aarch64-none-elf, aarch64-none-linux

These 2 regressions do not appear when configuring --target
arm-none-linux-gnueabihf --with-cpu=cortex-a5 --with-fpu=vfpv3-d16-fp16


However, when configuring --target arm-none-linux-gnueabihf
--with-cpu=cortex-a5 --with-fpu=vfpv3-d16-fp16, some of the newly introduced
tests fail:
  gcc.dg/tree-ssa/ssa-ifcombine-ccmp-1.c scan-tree-dump optimized "&"
  gcc.dg/tree-ssa/ssa-ifcombine-ccmp-4.c scan-tree-dump optimized "&"
  gcc.dg/tree-ssa/ssa-ifcombine-ccmp-5.c scan-tree-dump-times optimized "&" 2
  gcc.dg/tree-ssa/ssa-ifcombine-ccmp-6.c scan-tree-dump-times optimized "\\|" 2

[Bug tree-optimization/59303] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|arm                         |
            Version|unknown                     |4.9.0

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
One more comment, it now fails on x86_64 also.

[Bug tree-optimization/59303] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[ARM/AArch32//AArch64]      |uninit-pred-8_b.c and
                   |regressions in              |uninit-pred-9_b.c fail
                   |uninit-pred-8_b.c and       |after better optimizations
                   |uninit-pred-9_b.c           |

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> However, when configuring --target arm-none-linux-gnueabihf --with-cpu=cortex-a5
> --with-fpu=vfpv3-d16-fp16, some of the newly introduced tests fail:

That is a testsuite issue only and should be reported separately. BRANCH_COST
is low for cortex-a5.


The other testcases are really a buggy predicated unitialized warning pass, see
PR 49498 for another instance of the same testcase failing.  I had looked into
fixing that warning pass but it was complex to understand what was going wrong.

[Bug tree-optimization/59303] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[christophe.lyon at st dot com]

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

--- Comment #3 from christophe.lyon at st dot com ---
I have filed PR 59308 about the ssa-ifcombine-ccmp* tests failing on cortex-a5.

Should I mark this bug report (59303) as a duplicate of 49498?

[Bug tree-optimization/59303] [4.9 Regression] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |arm, x86_64
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |diagnostic
   Last reconfirmed|                            |2013-11-27
                 CC|                            |xinliangli at gmail dot com
     Ever confirmed|0                           |1
            Summary|uninit-pred-8_b.c and       |[4.9 Regression]
                   |uninit-pred-9_b.c fail      |uninit-pred-8_b.c and
                   |after better optimizations  |uninit-pred-9_b.c fail
                   |                            |after better optimizations
   Target Milestone|---                         |4.9.0

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  They fail for quite some while.  The uninit pass needs to cope
with combined predicates.

[Bug tree-optimization/59303] [4.9 Regression] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[rguenth at gcc dot gnu.org]

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

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

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

[Bug tree-optimization/59303] [4.9 Regression] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[xinliangli at gmail dot com]

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

--- Comment #5 from davidxl <xinliangli at gmail dot com> ---
Fixing this requires more powerful predicate analysis with the help of value
equivalent classes.

>From the dump:

      "Use in stmt blah (v_1);
      is guarded by :
      if (_23 != 0)"


_23 = pephitmp_22 | _8, so the use guard is

        prephitmp_22 != 0 || _8 != 0

prephitmp_22 = PHI <0(13), prephitmp_21(3), so the guard can be translated to

        prephitmp_21 != 0 || _8 != 0

prephitmp_21 = r_9(D) <= 19, so the predicate becomes

         r_9(D) <= 19 || _8 != 0


For the def:

[CHECK] Found def edge 1 in v_1 = PHI <v_14(D)(13), r_9(D)(3)>
Operand defs of phi v_1 = PHI <v_14(D)(13), r_9(D)(3)>
is guarded by :
       if (_8 != 0)
      (.OR.)
        (.NOT.) if (_8 != 0)
        (.AND.)
         if (_13 != 0)

the predicate is
       _8 != 0 || _13 != 0

where _13 = _10 | _12, 10_ = (r_9(D) <= 19), _12 = l_11(D) != 0

so the predicate is
       _8 != 0 || r_9(D) <= 19 || 1_11(D) != 0

which is the superset for the use.


Perhaps we need to do around of predicate normalization before comparison.

David

[Bug tree-optimization/59303] [4.9 Regression] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[xinliangli at gmail dot com]

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

--- Comment #6 from davidxl <xinliangli at gmail dot com> ---
I am working on a solution (and some cleanups).

David

[Bug tree-optimization/59303] [4.9 Regression] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[davidxl at google dot com]

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

davidxl at google dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |davidxl at google dot com

--- Comment #7 from davidxl at google dot com ---
Testing a fix that addresses the problem. With the fix, the dump shows:

Use in stmt: blah (v_1);
is guarded by :
m_6(D) > 100
(.OR.)
n_4(D) <= 9
(.OR.)
r_9(D) <= 19

Operand defs of phi: v_1 = PHI <v_14(D)(13), r_9(D)(3)>
is guarded by :
m_6(D) > 100
(.OR.)
n_4(D) <= 9
(.OR.)
l_11(D) != 0
(.OR.)
r_9(D) <= 19

which is correct.

[Bug tree-optimization/59303] [4.9 Regression] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 31496
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31496&action=edit
cleanups

I had also a brief look at this recently, but haven't made progress beyond
attached formatting/cleanup patch so far (plus only dumping details with
*-details, otherwise the dumps are pretty much inconsistent).
My conclusion has also been that to fix this up the predicates would need to be
normalized before comparison, and simplified, at least if the initial subset
check fails.  On uninit-pred-8_b.c there has been additional complication
because PRE decides to change the IL and we end up with:
  <bb 3>:
  pretmp_24 = r_10(D) <= 19;
  goto <bb 5>;

  <bb 4>:
  _11 = r_10(D) <= 19;
  _13 = l_12(D) != 0;
  _14 = _11 | _13;
  if (_14 != 0)
    goto <bb 14>;
  else
    goto <bb 15>;

  <bb 14>:
  goto <bb 3>;

  <bb 15>:

  <bb 5>:
  # v_1 = PHI <v_15(D)(15), r_10(D)(3)>
  # prephitmp_30 = PHI <0(15), pretmp_24(3)>
  if (m_7(D) != 0)
    goto <bb 6>;
  else
    goto <bb 7>;

  <bb 6>:
  g.0_17 = g;
  g.1_18 = g.0_17 + 1;
  g = g.1_18;
  goto <bb 8>;

  <bb 7>:
  bar ();

  <bb 8>:
  _3 = prephitmp_30 | _9;
  if (_3 != 0)
so the prephitmp_30 would need to be canonicalized into _14 != 0 && r_10(D) <=
19 aka (r_10(D) <= 19 || l_12(D) != 0) && r_10(D) <= 19 and from there to
r_10(D) <= 19.

[Bug tree-optimization/59303] [4.9 Regression] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[davidxl at google dot com]

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

--- Comment #10 from davidxl at google dot com ---
My patch does this.
1) it first does aggressive simplification iteratively (more rules can be added
later). 
2) it then does normalization by building up larger predicate trees by
following ud chain and bitwise or/and operations. It handles simple PHIs
including also degenerated phis.

The new dump reports:

in foo,

predicate for def:

m_7(D) > 100
(.OR.)
n_5(D) <= 9
(.OR.)
l_12(D) != 0
(.OR.)
r_10(D) <= 19

use1:

m_7(D) > 100
(.OR.)
n_5(D) <= 9
(.OR.)
r_10(D) <= 9

use2:

m_7(D) > 100
(.OR.)
n_5(D) <= 9
(.OR.)
r_10(D) <= 19


For foo2, the predicates are properly built, but the patch has a bug in
predicate set inclusion testing leading to a false negative.

David

(In reply to Jakub Jelinek from comment #9)
> Created attachment 31496 [details]
> cleanups
> 
> I had also a brief look at this recently, but haven't made progress beyond
> attached formatting/cleanup patch so far (plus only dumping details with
> *-details, otherwise the dumps are pretty much inconsistent).
> My conclusion has also been that to fix this up the predicates would need to
> be normalized before comparison, and simplified, at least if the initial
> subset check fails.  On uninit-pred-8_b.c there has been additional
> complication because PRE decides to change the IL and we end up with:
>   <bb 3>:
>   pretmp_24 = r_10(D) <= 19;
>   goto <bb 5>;
> 
>   <bb 4>:
>   _11 = r_10(D) <= 19;
>   _13 = l_12(D) != 0;
>   _14 = _11 | _13;
>   if (_14 != 0)
>     goto <bb 14>;
>   else
>     goto <bb 15>;
> 
>   <bb 14>:
>   goto <bb 3>;
>   
>   <bb 15>:
> 
>   <bb 5>:
>   # v_1 = PHI <v_15(D)(15), r_10(D)(3)>
>   # prephitmp_30 = PHI <0(15), pretmp_24(3)>
>   if (m_7(D) != 0)
>     goto <bb 6>;
>   else
>     goto <bb 7>;
> 
>   <bb 6>:
>   g.0_17 = g;
>   g.1_18 = g.0_17 + 1;
>   g = g.1_18;
>   goto <bb 8>;
>   
>   <bb 7>:
>   bar ();
> 
>   <bb 8>:
>   _3 = prephitmp_30 | _9;
>   if (_3 != 0)
> so the prephitmp_30 would need to be canonicalized into _14 != 0 && r_10(D)
> <= 19 aka (r_10(D) <= 19 || l_12(D) != 0) && r_10(D) <= 19 and from there to
> r_10(D) <= 19.

[Bug tree-optimization/59303] [4.9 Regression] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[davidxl at google dot com]

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

--- Comment #11 from davidxl at google dot com ---
The false negative bug introduced in the patch is fixed. Will submit the patch
for review soon.

(In reply to davidxl from comment #10)
> My patch does this.
> 1) it first does aggressive simplification iteratively (more rules can be
> added later). 
> 2) it then does normalization by building up larger predicate trees by
> following ud chain and bitwise or/and operations. It handles simple PHIs
> including also degenerated phis.
> 
> The new dump reports:
> 
> in foo,
> 
> predicate for def:
> 
> m_7(D) > 100
> (.OR.)
> n_5(D) <= 9
> (.OR.)
> l_12(D) != 0
> (.OR.)
> r_10(D) <= 19
> 
> use1:
> 
> m_7(D) > 100
> (.OR.)
> n_5(D) <= 9
> (.OR.)
> r_10(D) <= 9
> 
> use2:
> 
> m_7(D) > 100
> (.OR.)
> n_5(D) <= 9
> (.OR.)
> r_10(D) <= 19
> 
> 
> For foo2, the predicates are properly built, but the patch has a bug in
> predicate set inclusion testing leading to a false negative.
> 
> David
> 
> (In reply to Jakub Jelinek from comment #9)
> > Created attachment 31496 [details]
> > cleanups
> > 
> > I had also a brief look at this recently, but haven't made progress beyond
> > attached formatting/cleanup patch so far (plus only dumping details with
> > *-details, otherwise the dumps are pretty much inconsistent).
> > My conclusion has also been that to fix this up the predicates would need to
> > be normalized before comparison, and simplified, at least if the initial
> > subset check fails.  On uninit-pred-8_b.c there has been additional
> > complication because PRE decides to change the IL and we end up with:
> >   <bb 3>:
> >   pretmp_24 = r_10(D) <= 19;
> >   goto <bb 5>;
> > 
> >   <bb 4>:
> >   _11 = r_10(D) <= 19;
> >   _13 = l_12(D) != 0;
> >   _14 = _11 | _13;
> >   if (_14 != 0)
> >     goto <bb 14>;
> >   else
> >     goto <bb 15>;
> > 
> >   <bb 14>:
> >   goto <bb 3>;
> >   
> >   <bb 15>:
> > 
> >   <bb 5>:
> >   # v_1 = PHI <v_15(D)(15), r_10(D)(3)>
> >   # prephitmp_30 = PHI <0(15), pretmp_24(3)>
> >   if (m_7(D) != 0)
> >     goto <bb 6>;
> >   else
> >     goto <bb 7>;
> > 
> >   <bb 6>:
> >   g.0_17 = g;
> >   g.1_18 = g.0_17 + 1;
> >   g = g.1_18;
> >   goto <bb 8>;
> >   
> >   <bb 7>:
> >   bar ();
> > 
> >   <bb 8>:
> >   _3 = prephitmp_30 | _9;
> >   if (_3 != 0)
> > so the prephitmp_30 would need to be canonicalized into _14 != 0 && r_10(D)
> > <= 19 aka (r_10(D) <= 19 || l_12(D) != 0) && r_10(D) <= 19 and from there to
> > r_10(D) <= 19.

[Bug tree-optimization/59303] [4.9 Regression] uninit-pred-8_b.c and uninit-pred-9_b.c fail after better optimizations

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: davidxl
Date: Fri Jan  3 00:40:57 2014
New Revision: 206309

URL: http://gcc.gnu.org/viewcvs?rev=206309&root=gcc&view=rev
Log:
Fix PR/59303 -- uninit analysis enhancement

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-uninit.c