On Thu, Jan 19, 2017 at 2:02 PM, Aldy Hernandez <al...@redhat.com> wrote:
> On 01/19/2017 07:45 AM, Richard Biener wrote:
>>
>> On Thu, Jan 19, 2017 at 1:17 PM, Aldy Hernandez <al...@redhat.com> wrote:
>>>
>>> In the attached testcase, we have a clearly bounded case of alloca which
>>> is
>>> being incorrectly reported:
>>>
>>> void g (int *p, int *q)
>>> {
>>> size_t n = (size_t)(p - q);
>>>
>>> if (n < 10)
>>> f (__builtin_alloca (n));
>>> }
>>>
>>> The problem is that VRP gives us an anti-range for `n' which may be out
>>> of
>>> range:
>>>
>>> # RANGE ~[2305843009213693952, 16140901064495857663]
>>> n_9 = (long unsigned int) _4;
>>>
>>> We do a less than stellar job with casts and VR_ANTI_RANGE's, mostly
>>> because
>>> we're trying various heuristics to make up for the fact that we have
>>> crappy
>>> range info from VRP. More specifically, we're basically punting on an
>>> VR_ANTI_RANGE and ignoring that the casted result (n_9) has a bound later
>>> on.
>>>
>>> Luckily, we already have code to check simple ranges coming into the
>>> alloca
>>> by looking into all the basic blocks feeding it. The attached patch
>>> delays
>>> the final decision on anti ranges until we have examined the basic blocks
>>> and determined for that we are definitely out of range.
>>>
>>> I expect all this to disappear with Andrew's upcoming range info
>>> overhaul.
>>>
>>> OK for trunk?
>>
>>
>> I _really_ wonder why all the range consuming warnings are not emitted
>> from VRP itself (like we do for -Warray-bounds). There we'd still see
>> a range for the argument derived from the if () rather than needing to
>> do our own mini-VRP from the needessly "incomplete" range-info on
>> SSA vars.
>>
>> Richard.
>
>
> My original implementation was within VRP itself, using the ASSERT_EXPR's,
> but Jeff suggested doing it in it's own pass. I can't remember the details
> (I could look it up), but I think it had to do with getting better
> information post-inlining about the call to alloca (not sure).
>
> Also, with Andrew's GCC8 work on ranges, it seemed pointless to bend over
> backwards handling ASSERT_EXPR's etc. And IIRC, implementing it in VRP was
> ugly.
You don't need to handle ASSERT_EXPRs you simply use the VRP lattice after
propagation (or during if you hook into EVRP of course). And VRP runs after
inlining.
Richard.
> Perhaps Jeff remembers the details better.
>
> Aldy
