[Bug tree-optimization/59417] [4.9 Regression] ICE in determine_value_range, at tree-ssa-loop-niter.c:176

["jakub at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> Yeah, I wondered about this as well after reviewing the original patches.
> If you consider
> 
>   a_2 = a_1;
>   if (a_2 > 5)
>     a_3 = a_2;
> 
> then VRP would say a_3 is [6, +INF].  If then copyprop comes along it
> sees that a_3 = a_2 = a_1 and would transfer the value-range to a_1.
> 
> Note that the loop doing that may replace SSA annotation multiple times
> if copy_of[N].value == X for multiple N.
> 
> Basically the code relies on the information being not control sensitive.
> That's fine for points-to info (but alignment tracking now uses
> nonzero_bits?),
> but for the rest only if var is post-dominated by copy_of[].value's
> definition.

So, shall we drop that code from fini_copy_prop (I mean, don't
duplicate_ssa_name_range_info at all, and for duplicate_ssa_name_ptr_info
do it but clear align/misalign)?

> I don't think we should limit what copyprop/forwprop does though.

Does it buy is really that much optimization-wise?  I mean, is it that
important
if we have one or another SSA_NAME in the stmt?  Propagating of non-SSA_NAME
values is of course important, and if we don't have any better align/misalign
or range info than on the original, we should keep doing what we are.
But if not propagating some SSA_NAME would mean we don't lose important range
info or alignment info, is it worth it?

> Now, why do we arrive at a value-range where we fail that assertion?

So, during VRP1 we have a nested loop like:
  # t_2 = PHI <t_3(5), t_6(20)>
lbl1:
  d = 0;
  a.2_36 = a;
  a.3_37 = a.2_36 + 1;
  a = a.3_37;

  <bb 5>:
  # t_3 = PHI <t_19(D)(3), t_2(4)>
  b.0_39 = b;
  if (b.0_39 != 0)
    goto <bb 4> (lbl1);
  else
    goto <bb 6>;

  <bb 6>:
  goto <bb 11>;
...
  <bb 11>:
  d.1_40 = d;
  if (d.1_40 <= 1)
    goto <bb 7>;
  else
    goto <bb 12>;

  <bb 12>:
  # t_4 = PHI <t_19(D)(3), t_3(11)>
  e ={v} {CLOBBER};
  goto <bb 19> (lbl2);
...
  # t_5 = PHI <t_6(20), t_4(12)>
lbl2:
  t_34 = t_5 + 1;

  <bb 20>:
  # t_6 = PHI <t_19(D)(18), t_34(19)>
  if (t_6 <= 0)
    goto <bb 19> (lbl2);
  else
    goto <bb 4> (lbl1);

and VRP1 from this figures out
# RANGE [1, 2147483647] NONZERO 0x0000000007fffffff
for t_2, t_3 and t_4.  The reasoning for it is t_19(D), being VR_UNDEFINED, is
ignored in the PHIs, and loop to bb4 from bb20 (and thus indirectly to say
bb11)
only if t_6 is <= 0, therefore used ASSERT_EXPR of t_6 > 0.  I think this isn't
wrong, we are just assuming undefined-behavior doesn't happen.
Then copyprop6 apparently comes and changes:
   # RANGE [1, 2147483647] NONZERO 0x0000000007fffffff
-  # t_4 = PHI <t_19(D)(3)>
+  t_4 = t_19(D);
and because of the code we're discussing propagates the [1, INT_MAX] range to
t_19(D) as well.
Then VRP2 sees:
  # t_5 = PHI <t_34(17), t_19(D)(11)>
lbl2:
  t_34 = t_5 + 1;
  if (t_34 <= 0)
    goto <bb 17> (lbl2);
  else
    goto <bb 18> (lbl1);
VRP right now ignores the SSA_NAME_RANGE_INFO stuff (to be handled later?),
ignores as usually VR_UNDEFINED in phis and here the loop loops if t_34 <= 0,
thus VRP2 derives range [INT_MIN, 0] out of it.  So, what niters then sees
is that t_19(D) has [0, INT_MAX] range and t_5 has [INT_MIN, 0] range and is
upset about it.