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.
