On 1/25/22 04:15, Richard Biener wrote:
On Mon, Jan 24, 2022 at 11:55 PM Martin Sebor <mse...@gmail.com> wrote:
The pointer_query algorithm fails when it's unable to determine
the provenance of an pointer SSA variable. A failure prevents it
from making a record of the variable in the cache. Although this
doesn't happen often, one common cause of failures is PHI nodes:
e.g., because one of its arguments references the PHI itself, or
an undefined variable. Because a failed SSA_NAME isn't added to
the cache, the computation that led up to its failure is repeated
on each interesting use of the variable. This is computationally
wasteful and can cause excessive CPU usage in pathological cases
like in the test case in PR 104203.
To avoid the problem the attached patch changes the logic for PHI
arguments and nodes to cache the most conservative result on such
a failure. It treats such a pointer as pointing into the middle
of an object of maximum size (same as under some other conditions).
In addition, the patch also adds a new timer variable to
the warn_access pass to make these problems easier in the future
to track down.
LGTM.
Committed in r12-6869.
There are a number of other conditions under which point_query
fails. Even though the same approach could be used to replace
all those, I did not make the change in this patch.
It's probably worth fixing them. Btw, I wonder whether caching
fails as true fails would work as well though caching a conservative
answer of course is fine.
That's another approach: add a bitset and set a bit for each SSA
variable for which the lookup failed (or didn't find anything
interesting). It would save space in the cache. I didn't use
it because it would require more churn in the code, but it's
something to consider in the future (the cache itself could
stand to be further split up, with pointer offsets in one and
sizes in another).
Tested on x86_64-linux.
Martin
PS This solution relies on the pointer_query instance having caching
enabled. Two warning passes run without a cache: -Warray-bounds in
VRP and -Wrestrict in the wrestrict pass. I can look into enabling
it for GCC 12 if there's concern that they might be impacted.
Please. There is IMHO no reason to not use the cache - for PHIs
you can run into the same PHI arg def from multiple edges and thus
you'll do unnecessary work even when you just do a single query
for the toplevel API.
Let me post a patch with that.
Martin
Richard.
