On Tue, May 18, 2021 at 6:35 PM Andrew MacLeod <amacl...@redhat.com> wrote: > > 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.
Guess I'm confused - in classical terms an optimistic lattice propagator only allows downward transitions UNDEF -> CONSTANT -> VARYING while a non-optimistic one doesn't need to iterate and thus by definition has no lattice "transition" other than from the initial VARYING to the possibly non-VARYING but final state. > 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 > > > > > >
