On Mon, May 20, 2019 at 10:16 AM Marc Glisse <marc.gli...@inria.fr> wrote:
>
> On Mon, 20 May 2019, Richard Biener wrote:
>
> > On Sun, May 19, 2019 at 6:16 PM Marc Glisse <marc.gli...@inria.fr> wrote:
> >>
> >> Hello,
> >>
> >> 2 pieces:
> >>
> >> - the first one handles the case where the denominator is negative. It
> >> doesn't happen often with exact_div, so I don't handle it everywhere, but
> >> this one looked trivial
> >>
> >> - handle the case where a pointer difference is cast to an unsigned type
> >> before being compared to a constant (I hit this in std::vector). With some
> >> range info we could probably handle some non-constant cases as well...
> >>
> >> The second piece breaks Walloca-13.c (-Walloca-larger-than=100 -O2)
> >>
> >> void f (void*);
> >> void g (int *p, int *q)
> >> {
> >> __SIZE_TYPE__ n = (__SIZE_TYPE__)(p - q);
> >> if (n < 100)
> >> f (__builtin_alloca (n));
> >> }
> >>
> >> At the time of walloca2, we have
> >>
> >> _1 = p_5(D) - q_6(D);
> >> # RANGE [-2305843009213693952, 2305843009213693951]
> >> _2 = _1 /[ex] 4;
> >> # RANGE ~[2305843009213693952, 16140901064495857663]
> >> n_7 = (long unsigned intD.10) _2;
> >> _11 = (long unsigned intD.10) _1;
> >> if (_11 <= 396)
> >> [...]
> >> _3 = allocaD.1059 (n_7);
> >>
> >> and warn.
> >
> > That's indeed to complicated relation of _11 to n_7 for
> > VRP predicate discovery.
> >
> >> However, DOM3 later produces
> >>
> >> _1 = p_5(D) - q_6(D);
> >> _11 = (long unsigned intD.10) _1;
> >> if (_11 <= 396)
> >
> > while _11 vs. _1 works fine.
> >
> >> [...]
> >> # RANGE [0, 99] NONZERO 127
> >> _2 = _1 /[ex] 4;
> >> # RANGE [0, 99] NONZERO 127
> >> n_7 = (long unsigned intD.10) _2;
> >> _3 = allocaD.1059 (n_7);
> >>
> >> so I am tempted to say that the walloca2 pass is too early, xfail the
> >> testcase and file an issue...
> >
> > Hmm, there's a DOM pass before walloca2 already and moving
> > walloca2 after loop opts doesn't look like the best thing to do?
> > I suppose it's not DOM but sinking that does the important transform
> > here? That is,
> >
> > Index: gcc/passes.def
> > ===================================================================
> > --- gcc/passes.def (revision 271395)
> > +++ gcc/passes.def (working copy)
> > @@ -241,9 +241,9 @@ along with GCC; see the file COPYING3.
> > NEXT_PASS (pass_optimize_bswap);
> > NEXT_PASS (pass_laddress);
> > NEXT_PASS (pass_lim);
> > - NEXT_PASS (pass_walloca, false);
> > NEXT_PASS (pass_pre);
> > NEXT_PASS (pass_sink_code);
> > + NEXT_PASS (pass_walloca, false);
> > NEXT_PASS (pass_sancov);
> > NEXT_PASS (pass_asan);
> > NEXT_PASS (pass_tsan);
> >
> > fixes it?
>
> I will check, but I don't think walloca uses any kind of on-demand VRP, so
> we still need some pass to update the ranges after sinking, which doesn't
> seem to happen until the next DOM pass.
Oh, ok... Aldy, why's this a separate pass anyways? I think similar
other warnigns are emitted from RTL expansion? So maybe we can
indeed move the pass towards warn_restrict or late_warn_uninit.
I also see that the Og pass pipeline misses the second walloca pass
completely (and also the warn_restrict pass).
Given code sinkings obvious effects on SSA value-range representation
it may make sense to add another instance of that pass earlier.
>
> --
> Marc Glisse
