On 5/18/21 3:22 AM, Richard Biener wrote:
On Tue, May 18, 2021 at 1:23 AM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
The code in PR 100512 triggers an interaction between ranger and the
propagation engine related to undefined values.
I put the detailed analysis in the PR, but it boils down to the early
VRP pass has concluded that something is a constant and can be replaced,
and removes the definition expecting the constant to be propagated
everywhere.
If the code is in an undefined region that the CFG is going to remove,
we can find impossible situations,a nd ranger then changes that value ot
UNDEFINED.. because, well, it is. But then the propagation engine
panics because it doesnt have a constant any more, so odesnt replace it,
and now we have a used but not defined value.
Once we get to a globally constant range where further refinements can
only end up in an UNDEFINED state, stop further evaluating the range.
This is typically in places which are about to be removed by CFG cleanup
anyway, and it will make the propagation engine happy with no surprises.
Yeah, the propagation engine and EVRP as I know it relies on not visiting
"unexecutable" (as figured by anaysis) paths in the CFG and thus considering
edges coming from such regions not contributing conditions/values/etc. that
would cause such "undefinedness" to appear. Not sure how it works with
ranger, maybe that can as well get a mode where it does only traverse
EDGE_EXECUTABLE edges. Might be a bit difficult since IIRC it works
with SSA edges and not CFG edges.
Well it does do CFG based work as well, and I do not currently check
EDGE_EXECUTABLE... I just tried checking the EXECUTABLE_EDGE flag and
not processing it, but it doesn't resolve the problem. I think its
because the edge has not been determined unexecutable until after the
pass is done.. which is too late.
Bootstraps on x86_64-pc-linux-gnu with no regressions, and fixes the PR.
So that means the lattice isn't an optimistic lattice, right? EVRPs wasn't
optimistic either, but VRPs is/was. Whatever this means in this context ;)
It is optimistic, this just tells it to stop being optimistic if we get
to a constant so we don't mess up propagation. Any further evaluation
which causes it to become undefined has to be a result of this being an
undefined hunk of code, and I *think* that it will be eliminated by the
CFG cleanup due to change elsewhere.
The only thing I can imagine where we might miss something is if the
ssa_name we are leaving as a constant now were used elsewhere in a PHI
node. That PHI node will get a constant instead of an undefined
range.. and we would no longer make the optimistic assumption that that
PHI edge no longer contributes to the result. When the block of
code/edge is then eliminated by CFG cleanup, then that constant would be
removed since the edge is gone... but it would delay finding that
situation. We'll find out when we look at replacing VRP if that
actually happens anywhere.
Eventually I hope to tweak the propagation engine (or use an
alternative) so that deciding something is a constant and eliminating
the definition doesn't cause problems if we later discover the result is
actually undefined.. that what this boils down to. Following the linear
decision making of substituting constants doesn't work quite so well in
a more optimistic iterative environment. That will make us fully
optimistic again.
Andrew
