On 5/21/19 3:53 AM, Richard Biener wrote:
On Tue, May 21, 2019 at 4:13 AM Martin Sebor <mse...@gmail.com> wrote:
On 5/20/19 3:16 AM, Richard Biener wrote:
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 thought there was a preference to add new middle-end warnings
into passes of their own rather than into existing passes. Is
that not so (either in general or in this specific case)?
The preference was to add them not into optimization passes. But
of course having 10+ warning passes, each going over the whole IL
is excessive. Also each of the locally computing ranges or so.
Given the simplicity of Walloca I wonder why it's not part of another
warning pass - since it's about tracking "sizes" again there are plenty
that fit ;)
-Walloca doesn't need to track object sizes in the same sense
as objsize and strlen do. It just examines calls to allocation
functions, same as -Walloc-larger-than. It would make sense to
merge the implementation of two warnings. They don't need to
run as a pass of their own.
From my POV, the main (only?) benefit of putting warnings in their
own passes is modularity. Are there any others?
The biggest drawback I see is that it makes it hard to then share
data across multiple passes. The sharing can help not just
warnings (reduce both false positive and false negative rates) but
also optimization. That's why I'm merging the strlen and sprintf
passes, and want to eventually also look into merging
the -Wstringop-overflow warnings there (also emitted just before
RTL expansion. Did I miss any downsides?
When things fit together they are fine to merge obviously.
One may not like -Warray-bounds inside VRP but it really "fits".
It fits because it has access to data that's not (yet) available
outside VRP, right? It's not an ideal fit because by being in VRP
it doesn't have access to allocated object sizes so out-of-bounds
access to dynamically allocated objects aren't detected. To detect
those, I think the only pass it could go in is strlen. So again
an optimization pass. It doesn't fit there very well at all, but
there is no other pass that tracks sizes of all objects (objsize
does, though only in response to calls to __builtin_object_size).
I think just like a general purpose API to query value ranges that
cam be called from any pass, another API to query object sizes, and
another to query string lengths, would be useful.
OTOH making a warning part of an optimization pass naturally
limits its effect to when the specific optimization is enabled.
In theory it's possible to do -Warray-bounds at -O0 - we are in
SSA form after all - but of course you don't want to enable VRP at -O0.
True. Better warnings at -O0 would be great. The drawback would
be that the warnings would then have to implement all the same
analyses as the optimization passes they rely on. We'd end up
having to implement a static analyzer on top of basic GIMPLE.
I don't know if there's the -Walloca pass would benefit from merging
with any of the others or vice versa, but superficially it seems like
it might be worth thinking about integrating the -Walloc-larger-than
warnings into the -Walloca pass, if only to keep similar functionality
in the same place.
-Wrestrict and all the format warning stuff seems related as well.
I think so. All of strlen, sprintf, Wrestrict, -Wstringop-overflow
and -Walloca/-Walloc-larger-than, and even objsize, would benefit
from sharing the same IL traversal and the same data. As would
-Warray-bounds to detect out-of-bounds accesses to dynamically
allocated objects. There probably are other warnings and likely
also optimizations that would benefit from closer coupling.
Martin
I also see that the Og pass pipeline misses the second walloca pass
completely (and also the warn_restrict pass).
That's worth fixing.
Martin
Given code sinkings obvious effects on SSA value-range representation
it may make sense to add another instance of that pass earlier.
--
Marc Glisse
