[Bug tree-optimization/88542] Optimize symmetric range check

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88542

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
On GIMPLE I see

  <bb 2> [local count: 1073741825]:
  if (d_3(D) < max_4(D))
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 3> [local count: 536870912]:
  _1 = -max_4(D);
  _6 = _1 < d_3(D);

  <bb 4> [local count: 1073741825]:
  # iftmp.0_2 = PHI <_6(3), 0(2)>

generated by late phiopt which means phiopt could iteratively arrive at
the solution if it handled the above.

phiopt first value-replaces

  if (_1 < d_3(D))
    goto <bb 5>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 4> [local count: 268435456]:

  <bb 5> [local count: 1073741825]:
  # iftmp.0_2 = PHI <0(4), 1(3), 0(2)>

not sure why that only happens in the last phiopt pass.  Ah, because
the main phiopt pass sees the following:

  <bb 2> [local count: 1073741825]:
  if (d_3(D) < max_4(D))
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 3> [local count: 536870912]:
  _1 = -max_4(D);
  if (_1 < d_3(D))
    goto <bb 5>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 4> [local count: 805306368]:

  <bb 5> [local count: 1073741825]:
  # iftmp.0_2 = PHI <1(3), 0(4)>

oddly enough the forwarder is removed by veclower?!  Ah, I guess this
is the first cfg-cleanup run that happens since nobody else touches
the code and veclower runs it unconditionally.  Heh.  No, it looks like
the forwarding is somehow triggered by PRE splitting critical edges
and _then_ only veclower runs cfg-cleanup.

Hmm.  CCP runs CFG cleanup which removes one forwarder but then
has to keep the other one because we need at least one forwarder
due to the different PHI arg values.  So it looks like we should
somehow prefer to eliminate the other forwarder (for phiopts sake
only?).  That's sth to investigate - make CFG cleanup apply a
more consistent decision here rather than eliminating the forwarders
simply in BB index order.

The course of action is probably to detect the

remove_forwarder_block (basic_block bb)
{
...
  /* If there are phi nodes in DEST, and some of the blocks that are
     predecessors of BB are also predecessors of DEST, check that the
     phi node arguments match.  */
  if (!gimple_seq_empty_p (phi_nodes (dest)))
    {
      FOR_EACH_EDGE (e, ei, bb->preds)
        {
          s = find_edge (e->src, dest);
          if (!s)
            continue;

          if (!phi_alternatives_equal (dest, succ, s))
            return false;

situation and then decide whether to re-create a forwarder on the
other edge to create a more canonical CFG (when BB has multiple
predecessors, that is, though in that case we're introducing another
copy).