On 01/20/2017 03:17 AM, Richard Biener wrote:
On Thu, Jan 19, 2017 at 5:53 PM, Martin Sebor <mse...@gmail.com> wrote:
On 01/19/2017 05: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.
Can you explain why the range info is only available in VRP and
not outside, via the get_range_info() API? It sounds as though
the API shouldn't be relied on (it was virtually unused before
GCC 7).
It's very simple. Look at the testcase from above
void g (int *p, int *q)
{
size_t n = (size_t)(p - q);
if (n < 10)
f (__builtin_alloca (n));
}
The IL outside of VRP is
<bb 2> [100.00%]:
p.0_1 = (long int) p_7(D);
q.1_2 = (long int) q_8(D);
_3 = p.0_1 - q.1_2;
_4 = _3 /[ex] 4;
n_9 = (size_t) _4;
if (n_9 <= 9)
goto <bb 3>; [36.64%]
else
goto <bb 4>; [63.36%]
<bb 3> [36.64%]:
_5 = __builtin_alloca (n_9);
f (_5);
so there is no SSA name we can tack a range to covering the n_9 <= 9
condition that dominates __builtin_alloca. Inside VRP we have
<bb 2> [100.00%]:
p.0_1 = (long int) p_7(D);
q.1_2 = (long int) q_8(D);
_3 = p.0_1 - q.1_2;
_4 = _3 /[ex] 4;
n_9 = (size_t) _4;
if (n_9 <= 9)
goto <bb 3>; [36.64%]
else
goto <bb 4>; [63.36%]
<bb 3> [36.64%]:
n_13 = ASSERT_EXPR <n_9, n_9 <= 9>;
_5 = __builtin_alloca (n_13);
f (_5);
and viola - now the alloca call uses n_13 which is defined at a point
dominated by if (n_9 <= 9) and thus it has an improved range:
n_13: [0, 9] EQUIVALENCES: { n_9 } (1 elements)
Yes, I remember getting much better range information when doing things
within VRP2, however since VRP2 runs after jump threading sometimes I'd
get more false positives.
Tell you what, for GCC8 I volunteer to rewrite gimple-ssa-warn-alloca.c
within VRP (or using whatever range information we decide upon or is
ready for GCC8). In the meantime, can we fix the PR with my patch (or a
similar approach)?
Aldy
