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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

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

--- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> 
---
This may be worth filing as another PR (let me know if you
think I should), but another case of VRP stymieing phiopt is:

void bar (int);
void
foo (int a, int b)
{
  if (!b)
    bar (1);
  else
    {
      int c;
      if (a)
        c = a;
      else
        c = 0;
      if (c == b)
        bar (2);
    }
}

Before vrp1 we have:

  <bb 2> [100.0%]:
  if (b_3(D) == 0)
    goto <bb 3>; [29.6%]
  else
    goto <bb 4>; [70.4%]

  <bb 3> [29.6%]:
  bar (1);
  goto <bb 8>; [100.0%]

  <bb 4> [70.4%]:
  if (a_4(D) != 0)
    goto <bb 6>; [50.0%]
  else
    goto <bb 5>; [50.0%]

  <bb 5> [35.2%]:

  <bb 6> [70.4%]:
  # c_1 = PHI <a_4(D)(4), 0(5)>
  if (c_1 == b_3(D))
    goto <bb 7>; [22.9%]
  else
    goto <bb 8>; [77.0%]

  <bb 7> [16.2%]:
  bar (2);

  <bb 8> [100.0%]:
  return;

phiopt would tell that c_1 is equal to a_4,
and after making that substitution, cfgcleanup
would remove blocks 4 and 5, leaving two comparisons
rather than three.

However, VRP runs first and threads 4->5 to 4->8,
giving:

 <bb 2> [100.0%]:
  if (b_3(D) == 0)
    goto <bb 3>; [29.6%]
  else
    goto <bb 4>; [70.4%]

  <bb 3> [29.6%]:
  bar (1);
  goto <bb 7>; [100.0%]

  <bb 4> [70.4%]:
  if (a_4(D) != 0)
    goto <bb 5>; [50.0%]
  else
    goto <bb 8>; [50.0%]

  <bb 5> [35.2%]:
  # c_1 = PHI <a_4(D)(4)>
  if (c_1 == b_3(D))
    goto <bb 6>; [45.9%]
  else
    goto <bb 7>; [54.1%]

  <bb 6> [16.2%]:
  bar (2);

  <bb 7> [100.0%]:
  return;

  <bb 8> [35.2%]:
  # c_11 = PHI <0(4)>
  goto <bb 7>; [100.0%]

We never "recover" from that and so the three comparisons
survive into the final output.

Removing the first !b comparison leaves no jump threading
opportunities, so phiopt kicks in as expected.

This is a cut-down version of what happens with things like
the safe_as_a calls in NEXT_INSN.  E.g. in the basic-block
iterator:

#define FOR_BB_INSNS(BB, INSN)                  \
  for ((INSN) = BB_HEAD (BB);                   \
       (INSN) && (INSN) != NEXT_INSN (BB_END (BB));     \
       (INSN) = NEXT_INSN (INSN))

we never get rid of the p null test in:

template <typename T, typename U>
inline T
safe_as_a (U *p)
{
  if (p)
    {
      gcc_checking_assert (is_a <T> (p));
      return is_a_helper <T>::cast (p);
    }
  else
    return NULL;
}

even though in release builds the function should
reduce to "return p".

Reply via email to