pushed as 74ee12ff68243bb177fb8653474dff80c3792139
fyi, the 2 testcases depending on the VRP flag were:
c-c++-common/Warray-bounds-2.c (-warray-bounds -fno-tree-vrp :-P)
and
g++.dg/warn/string1.C (-O1 -Wall)
Andrew
On 6/10/24 16:12, Jeff Law wrote:
On 6/10/24 1:24 PM, Andrew MacLeod wrote:
The array bounds warning pass was originally attached to the VRP pass
because it wanted to leverage the context sensitive ranges available
there.
With ranger, we can make it a pass of its own for very little cost.
This patch does that. It removes the array_bounds_checker from VRP
and makes it a solo pass that runs immediately after VRP1.
The original version had VRP add any un-executable edge flags it
found, but I could not find a case where after VRP cleans up the CFG
the new pass needed that. I also did not find a case where
activating SCEV again for the warning pass made a difference after
VRP had run. So this patch does neither of those things.
It simple enough to later add SCEV and loop analysis again if it
turns out to be important.
My primary motivation for removing it was to remove the second DOM
walk the checker performs which depends on on-demand ranges
pre-cached by ranger. This prevented VRP from choosing an
alternative VRP solution when basic block counts are very high (PR
114855). I also know Siddesh want to experiment with moving the pass
later in the pipeline as well, which will make that task much simpler
as a secondary rationale.
I didn't want to mess with the internal code much. For a multitude of
reasons. I did change it so that it always uses the current
range_query object instead of passing one in to the constructor. And
then I cleaned up the VRP code ot no longer take a flag on whether to
invoke the warning code or not.
The final bit is the pass is set to only run when flag_tree_vrp is
on.. I did this primarily to preserve existing functionality, and
some tests depended on it. ie, would turn on -warray-bounds and
disables tree-vrp pass (which means the bounds checker doesnt run)
... which changes the expected warnings from the strlen pass. I'm
not going there. there are also tests which run at -O1 and -Wall
that do not expect the bounds checker to run either. So this
dependence on the vrp flag is documented in the code an preserves
existing behavior.
Does anyone have any issues with any of this?
No, in fact, quite the opposite. I think we very much want the
warning out of VRP into its own little pass that we can put wherever
it makes sense in the pipeline rather than having it be tied to VRP.
I'd probably look at the -O1 vs -Wall stuff independently so that we
could (in theory) eventually remove the dependence on flag_vrp.
jeff
