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
