On Tue, Oct 6, 2020 at 7:06 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I have now checked in the hybrid EVRP pass.
>
> We have resolved all the issue we are aware of with a full Fedora build,
> but if any more issues arise, please let us know. (And Im sure you will :-)
>
> I made some minor tweaks. the option to the new -fevrp-mode flag are now:
>
> legacy : classic EVRP mode
> ranger : Ranger only mode
> *legacy-first : Query ranges with EVRP first, and if that fails try
> the ranger*
> ranger-first : Query the ranger first, then evrp
> ranger-trace : Ranger-only mode plus Show range tracing info in the dump
> ranger-debug : Ranger-only mode, and also include all cache debugging info
> trace : Hybrid mode with range tracing info
> debug : Hybrid mode with cache debugging as well as tracing
>
> The default is still *legacy-first*.
We'll have to keep -fevrp-mode forever so can you instead make it a --param
since I hope it is transitional only? It certainly shouldn't be a
user-visible flag, should it?
Richard.
>
> ********
> If there is a compilation problem, and the problem goes away with
> -fevrp-mode=legacy
> the ranger is to blame, and let us know asap.
> ********
>
> Attached is the patch which was applied.
>
> -------------------------------------------
>
> These are the initial test case differences, and the detailed analysis
> is below:
>
> 1) We now XPASS analyzer/pr94851-1.c.
> 2) -disable-tree-evrp added to gcc.dg/pr81192.c
> 3) -disable-tree-evrp added to gcc.dg/tree-ssa/pr77445-2.c
> 4) -disable-tree-evrp added to tree-ssa/ssa-dom-thread-6.c
> 5) -disable-tree-evrp added to tree-ssa/ssa-dom-thread-7.c
>
>
> mostly it is one of 2 things:
>
> 1) we propagate a constant into into PHI that wasnt happening before,
> EVRP didn't handle anything other than single entry blocks well.
> 2) switches are processed in a lot more detail, which again propagate a
> lot of values into PHIs, and it then triggers more threading.
>
> Last minute update... It also turns out the analyzer xpass may be noise.
> we did change the IL, but it sounds like there are other analyzer things
> at play at the moment :-)
>
> --------------------------------------------------------------------------------------------------------------------------------------------------------
> 1) We now XPASS analyzer/pr94851-1.c.
>
>
> It was xfailing with:
> In function ‘pamark’:
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:43:13:
> warning: leak of ‘p’ [CWE-401] [-Wanalyzer-malloc-leak]
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:29:6: note:
> (1) following ‘false’ branch (when ‘p’ is NULL)...
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:32:23: note:
> (2) ...to here
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:32:23: note:
> (3) allocated here
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:32:8: note:
> (4) assuming ‘p’ is non-NULL
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:32:8: note:
> (5) following ‘false’ branch (when ‘p’ is non-NULL)...
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:35:15: note:
> (6) ...to here
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:43:13: note:
> (7) ‘p’ leaks here; was allocated at (3)
>
> now we produce:
> XPASS: gcc.dg/analyzer/pr94851-1.c bogus leak (test for bogus messages,
> line 43)
>
>
> THe reason is in the IL:
> <bb 4> :
> # p_9 = PHI <p_16(2), p_21(3)>
> # last_11 = PHI <p_16(2), p_9(3)>
> if (p_9 != 0B)
> goto <bb 5>; [INV]
> else
> goto <bb 6>; [INV] --> This outgoing edge
>
> <bb 5> :
> _3 = p_9->m_name;
> _4 = (char) _32;
> if (_3 != _4)
> goto <bb 3>; [INV]
> else
> goto <bb 6>; [INV]
>
> <bb 6> :
> # p_2 = PHI <p_9(4), p_9(5)> <<<---- THis PHI node
> # last_17 = PHI <last_11(4), last_11(5)>
> if (p_2 != 0B)
> goto <bb 7>; [INV]
> else
> goto <bb 8>; [INV]
>
> <bb 7> :
> printf ("over writing mark %c\n", _32);
> goto <bb 13>; [INV]
>
>
> The ranger propagates the p_9 == 0 from the else branch into the PHI
> argument on edge 4->6
> <bb 6> :
> # p_2 = PHI <0B(4), p_9(5)>
>
> which the threaders can then bypass the print in bb7 on one path, and
> that seems to resolve the current issue.
>
> THe IL produced by the time we get to .optimized is identical, we just
> clear it up early enough for the analyzer to use now.
>
> -------------------------------------------------------------------------------------------------------------------
>
> 2) -fdisable-tree-evrp added to gcc.dg/pr81192.c to enable the test to pass
>
> new version of evrp sees
> <bb 3> :
> if (j_8(D) != 2147483647)
> goto <bb 4>; [50.00%]
> else
> goto <bb 5>; [50.00%]
> <bb 4> :
> iftmp.2_11 = j_8(D) + 1;
> <bb 5> :
> # iftmp.2_12 = PHI <j_8(D)(3), iftmp.2_11(4)>
>
> EVRP now recognizes a constant can be propagated into the 3->5 edge and
> produces
> # iftmp.2_12 = PHI <2147483647(3), iftmp.2_11(4)>
> which causes the situation being tested to dissappear before we get to
> PRE. */
>
> -------------------------------------------------------------------------------------------------------------------
>
> 3) -disable-tree-evrp added to gcc.dg/tree-ssa/pr77445-2.c
>
> Aldy investigated this, and basically we are threading 6 more paths on
> x86_64 which is changing the IL in visible ways.
> Disabling evrp allows the threaders to test what they are looking for.
>
> -------------------------------------------------------------------------
>
> 4) and 5)
>
> along the same vein, we are threading anew opportunies in PHIs... these
> 2 tests were changes to include the new evrp result.
>
> Aldy has added comments to the test cases
>
> + # state_10 = PHI <state_11(7), 0(10), state_11(5), 1(6),
> state_11(8), 2(9), state_11(11)>
> +
> + Now with the new evrp, we get:
> +
> + # state_10 = PHI <0(7), 0(10), state_11(5), 1(6), 0(8), 2(9), 1(11)>
> +
> + Thus, we have 3 more paths that are known to be constant and can be
> + threaded. Which means that by the second threading pass, we can
> + only find one profitable path.
> +
> + For the record, all these extra constants are better paths coming
> + out of switches. For example:
> +
> + SWITCH_BB -> BBx -> BBy -> BBz -> PHI
> +
> + We now know the value of the switch index at PHI. */
>
> and
>
> +$ diff clean/a.c.105t.mergephi2 a.c.105t.mergephi2
> +252c252
> +< # s_50 = PHI <s_49(10), 5(14), s_51(18), s_51(22), 1(26), 1(29),
> 1(31), s_51(5), 4(12), 1(15), 5(17), 1(19), 3(21), 1(23), 6(25), 7(28),
> s_51(30)>
> +---
> +> # s_50 = PHI <s_49(10), 5(14), 4(18), 5(22), 1(26), 1(29), 1(31),
> s_51(5), 4(12), 1(15), 5(17), 1(19), 3(21), 1(23), 6(25), 7(28), 7(30)>
> +272a273
> +
> + I spot checked a few and they all have the same pattern. We are
> + basically tracking the switch index better through multiple
> + paths. */
>
>
>
> Andrew
