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

[rguenther at suse dot de]

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

--- Comment #5 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 10 Dec 2013, jakub at gcc dot gnu.org wrote:

> 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)?

Yes, I think we have to :/

> > 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?

Well, passes still assume copy-proped IL when doing pattern matching
so yes, it matters (but only because of that).  If they cannot rely
on this then this also is a compile-time issue (basically re-doing
copy-prop at pattern matching time).

But we can also be more careful which range/alignment info we substitute
and not remove it all.

> > 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.

Ah, ok - I thought we'd have an SSA name with bogus range info but we
just have two SSA names with range info that don't agree in a way
that niter likes.  That should be fixed in niter.

Richard.