Re: [PATCH] PR tree-optimization/108356 - Ranger cache - always use range_from_dom when updating.

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Jan 31, 2023 at 9:11 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This turned out to be a more interesting problem than I wanted.
>
> the situation boils down to:
>
> <bb 10>
> # g_5 = PHI <0(2), 2(8)>
>    if (g_5 <= 1)
>      goto <bb 4>; [INV]
>
> <bb 4> :
>    if (g_5 != 0)
>      goto <bb 7>; [INV]
>    else
>      goto <bb 8>; [INV]
>
>    <bb 7> :
>    c = 0;
>
>    <bb 8> :
>      goto <bb 10>; [INV]
>
>   We globally know that g_5 is [0,0][2,2]
> we also know that 10->4 , g_5 will be [0,0]
> Which means we also know that that the branch in bb_4 will never be
> taken, and will always go to bb 8.
> THis is all processed in EVRP, the branch is changed, and the next time
> VRP is called, we blow the block with c = 0 like we want...
>
> Unfortunately it doesnt happen within EVRP because when this updated
> range for g_5 is propagated in the cache, it was tripping over a shotcut
> which tried to avoid using lookups when it thinks it didnt matter, and
> would occasionally drop back to the global range.
>
> In particular, the cache had originally propagated [0,0][2,2] as the on
> entry range to bb8. when we rewrite the branch, we mark 4->7 and 7->8
> as unexecutable edges, then propagate the new range for g_5..  This
> requires recalculating the existing range on entry to bb8.
>
> It properly picked up [0,0] from 4->8, but when the cache query checked
> the range from 7->8, it discovered that no value was yet set, so instead
> of looking it up, it fell back to the global range of [0,0][2,2].  If it
> properly calculates that edge instead, it comes up with UNDEFINED (as it
> is unexecutable) and results in [0,0]... and EVRP then also removes the
> block is c = 0 in.
>
> By picking up the global value, it still thought 2 was a possibility
> later on, and a single VRP pass couldn't eliminate the branch ultimately
> leading to the store... it required a second one with the adjusted CFG
> to catch it.
>
> This patch tells the cache to always do a read-only scan of the
> dominator tree to find the nearest actual value and use that instead.
> This may solve other lingering weird propagation issues.
>
> I also ran a performance run on this change. It does slow VRP by down
> about 1%, but the overall change is nominal at around 0.05%.
>
> Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK?

OK.

Thanks,
Richard.

> Andrew
>