[Bug middle-end/81889] [7/8 Regression] bogus warnings with -Wmaybe-uninitialized -O3

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener  ---
Author: rguenth
Date: Tue Dec 12 08:50:31 2017
New Revision: 255573

URL: https://gcc.gnu.org/viewcvs?rev=255573=gcc=rev
Log:
2017-12-12  Richard Biener  

PR tree-optimization/81889
* tree-ssa-loop-niter.c (infer_loop_bounds_from_signedness): Use
range info from the non-wrapping IV instead of just the range
of the type.

* gfortran.dg/pr81889.f90: New testcase.
* gcc.dg/tree-ssa/pr64183.c: Adjust.

Added:
trunk/gcc/testsuite/gfortran.dg/pr81889.f90
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.dg/tree-ssa/pr64183.c
trunk/gcc/tree-ssa-loop-niter.c

[Bug middle-end/81889] [7/8 Regression] bogus warnings with -Wmaybe-uninitialized -O3

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener  ---
Specifically it is PRE code hoisting hoisting (integer(kind=8)) _2 above
if (_2 > 3) making it impossible to set a SSA range info on it.

At cunroll time we still have somewhat useful info on other SSA names in
the loop:

   [local count: 912680545]:
  # val.11_94 = PHI <0.0(17), val.11_56(20)>
  # RANGE [1, 17] NONZERO 7
  # S.12_93 = PHI <1(17), S.12_57(20)>
  # RANGE [0, 2] NONZERO 3
  _16 = S.12_93 + -1;
  _17 = xx[_16];
  val.11_56 = _17 + val.11_94;
  # RANGE [2, 4] NONZERO 7
  S.12_57 = S.12_93 + 1;
  if (S.12_57 > _106)
goto ; [15.00%]
  else
goto ; [85.00%]

   [local count: 136902080]:
  # val.11_122 = PHI 
  goto ; [100.00%]

specifically the range info on S.12 (even if somewhat strange) could be used
to derive a range "backwards" for the number of iterations from base and step
from the increment stmt for example.

Thus for example the following fixes the testcase and exploits any
range info on undefined overflow stmts.  Giving that wider testing.

Index: gcc/tree-ssa-loop-niter.c
===
--- gcc/tree-ssa-loop-niter.c   (revision 255539)
+++ gcc/tree-ssa-loop-niter.c   (working copy)
@@ -3510,6 +3510,12 @@ infer_loop_bounds_from_signedness (struc

   low = lower_bound_in_type (type, type);
   high = upper_bound_in_type (type, type);
+  wide_int minv, maxv;
+  if (get_range_info (def, , ) == VR_RANGE)
+{
+  low = wide_int_to_tree (type, minv);
+  high = wide_int_to_tree (type, maxv);
+}

   record_nonwrapping_iv (loop, base, step, stmt, low, high, false, true);
 }

[Bug middle-end/81889] [7/8 Regression] bogus warnings with -Wmaybe-uninitialized -O3

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener  ---
Created attachment 42838
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42838=edit
old non-functional patch

VRP1 has figured out ranges that bound the iteration of the loop in question
but there's too much stuff happening inbewteen that and cunrool to have any
hope to preserve it enough.

niter analysis would benefit from some ad-hoc walking of dominator stmts
like simplify_using_initial_conditions does for the purpose of figuring
an upper bound for the iteration...  like the attached old non-functional
patch computing ranges for a GENERIC expr which nowadays should use
register_edge_assert_for () to get at "asserts" for conditions it visits
and extract_range_for_var_from_comparison_expr to get at range info.

That said, niter analysis might want to populate a sparse lattice of
ranges seeded by dominating conditions and propagate those via the suggested
recursion (lacking SSA use->def handling).  Not sure if a sparse lattice
can be easily instantiated with the current vr_values class - it seems to
include lattice storage at the moment rather than abstracting that.

Note that re-ordering passes to somehow make VRP2 appear before cunroll
might also fix things but cunroll is in the loop pipeline and VRP2 is not.
Hacking things up so that early range info is copied/preserved enough might
be possible for this particular case.

[Bug middle-end/81889] [7/8 Regression] bogus warnings with -Wmaybe-uninitialized -O3

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener  ---
(In reply to Jeffrey A. Law from comment #5)
> I wonder why forwprop doesn't simplify bb6 by pushing the RHS of the
> assignment to _40 into the use of _40 in the conditional.
> 
> I'd guess the fortran front-end's boolean types are goofy and that's getting
> in the way of forwprop doing its job.  It may be impeding other bits as well.

It is because we fold (_39 < 0) != 0 to (logical(kind=1)) (_39 >> 63) and
canonicalize_cond_expr_cond doesn't recognize that form.

Index: gcc/gimple.c
===
--- gcc/gimple.c(revision 255539)
+++ gcc/gimple.c(working copy)
@@ -2148,6 +2148,17 @@ canonicalize_cond_expr_cond (tree t)
   else if (TREE_CODE (t) == BIT_XOR_EXPR)
 t = build2 (NE_EXPR, TREE_TYPE (t),
TREE_OPERAND (t, 0), TREE_OPERAND (t, 1));
+  /* For (bool)(x >> 63) use x < 0.  */
+  else if (CONVERT_EXPR_P (t)
+  && TREE_CODE (TREE_OPERAND (t, 0)) == RSHIFT_EXPR
+  && TREE_CODE (TREE_OPERAND (TREE_OPERAND (t, 0), 1)) == INTEGER_CST
+  && ! TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (t, 0)))
+  && compare_tree_int (TREE_OPERAND (TREE_OPERAND (t, 0), 1),
+   TYPE_PRECISION
+ (TREE_TYPE (TREE_OPERAND (t, 0))) - 1) == 0)
+t = build2 (LT_EXPR, TREE_TYPE (t),
+   TREE_OPERAND (TREE_OPERAND (t, 0), 0),
+   build_zero_cst (TREE_TYPE (TREE_OPERAND (t, 0;

   if (is_gimple_condexpr (t))
 return t;

fixes that but not the warnings (didn't expect that).  As suspected the array
accesses are introduced by cunroll.  I'll see what to recover from there.

[Bug middle-end/81889] [7/8 Regression] bogus warnings with -Wmaybe-uninitialized -O3

[law at redhat dot com]

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

Jeffrey A. Law  changed:

   What|Removed |Added

 CC||law at redhat dot com

--- Comment #5 from Jeffrey A. Law  ---
I wonder why forwprop doesn't simplify bb6 by pushing the RHS of the assignment
to _40 into the use of _40 in the conditional.

I'd guess the fortran front-end's boolean types are goofy and that's getting in
the way of forwprop doing its job.  It may be impeding other bits as well.

[Bug middle-end/81889] [7/8 Regression] bogus warnings with -Wmaybe-uninitialized -O3

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Keywords||missed-optimization
 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |rguenth at gcc dot 
gnu.org

--- Comment #4 from Richard Biener  ---
There is a path in the IL that has the array not initialized which is when
n is 0.

# iftmp.13_23 = PHI <_31(3), 1(36)>
_2 = *n_34(D);
_107 = (integer(kind=8)) _2;
if (_2 > 3)
  goto ; [50.00%]
else
  goto ; [50.00%]

 [local count: 161061274]:
_39 = _107 + -1;
_40 = _39 < 0;
if (_40 != 0)
  goto ; [50.00%]
else
  goto ; [50.00%]

then we also somehow end up with a lot of threaded(?) paths exposing all
(and even impossible) loads from xx.  Note the warnings are _all_ from
the path dominated by the n <= 3 check!  So the warning is obviously
valid just the compiler-generated code must be dead in some way it doesn't
see...

Need some closer investigation, mine for that.

[Bug middle-end/81889] [7/8 Regression] bogus warnings with -Wmaybe-uninitialized -O3

[aldyh at gcc dot gnu.org]

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

Aldy Hernandez  changed:

   What|Removed |Added

 CC||rguenth at gcc dot gnu.org

--- Comment #3 from Aldy Hernandez  ---
Started with r245840.

commit 4d2b9d1e3c794a05c00708035519290e920768e8
Author: rguenth 
Date:   Thu Mar 2 13:42:05 2017 +

2017-03-02  Richard Biener  

PR tree-optimization/79345
PR c++/42000
* tree-ssa-alias.c (walk_aliased_vdefs_1): Take a limit
param and abort the walk, returning -1 if it is hit.
(walk_aliased_vdefs): Take a limit param and pass it on.
* tree-ssa-alias.h (walk_aliased_vdefs): Add a limit param,
defaulting to 0 and return a signed int.
* tree-ssa-uninit.c (struct check_defs_data): New struct.
(check_defs): New helper.
(warn_uninitialized_vars): Use walk_aliased_vdefs to warn
about uninitialized memory.

* fixed-value.c (fixed_from_string): Use ulow/uhigh to avoid
bogus uninitialized warning.
(fixed_convert_from_real): Likewise.

* g++.dg/warn/Wuninitialized-7.C: New testcase.
* c-c++-common/ubsan/bounds-2.c: Add -Wno-uninitialized.
* gcc.dg/uninit-pr19430-2.c: Add expected warning.

[Bug middle-end/81889] [7/8 Regression] bogus warnings with -Wmaybe-uninitialized -O3

[aldyh at gcc dot gnu.org]

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

Aldy Hernandez  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-12-09
 CC||aldyh at gcc dot gnu.org,
   ||law at gcc dot gnu.org
 Ever confirmed|0   |1

--- Comment #2 from Aldy Hernandez  ---
Confirmed.

Jeff, FWIW, the new threader makes no difference here.

[Bug middle-end/81889] [7/8 Regression] bogus warnings with -Wmaybe-uninitialized -O3

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Target Milestone|--- |7.3
Summary|[7 Regression] bogus|[7/8 Regression] bogus
   |warnings with   |warnings with
   |-Wmaybe-uninitialized -O3   |-Wmaybe-uninitialized -O3