On 01/04/2017 11:01 AM, Jakub Jelinek wrote:
On Wed, Jan 04, 2017 at 11:19:46AM +0100, Richard Biener wrote:
For the SSA_NAME + INTEGER_CST case restrict it to the case

  if (x_1 > 5)
    tem_2 = (char) x_1;
  # tem_3 = PHI <tem_2, 5>

that is, (char) x_1 uses x_1 and that also appears in the controlling
GIMPLE_COND.  That's what enables followup minmax replacement
(gcc.dg/tree-ssa/pr66726.c).  Another case where it might be
profitable is if the BB is empty after the conversion is sunk
(may enable other value-replacement cases).

Do we actually have any testcases where we need the SSA_NAME + INTEGER_CST
case?
The only testcase that has been added that actually relied on the
factor_out_* is since r242750 handled during gimple folding (already in
*.gimple dump).

If I modify the testcase so that it isn't recognized by the minmax folding,
extern unsigned short y[];

int
foo (int x)
{
  return 64 < y[x] ? 68 : y[x];
}

then vanilla gcc vs. gcc with factor_out* INTEGER_CST case removed gives:
-       cmpw    $65, %ax
-       cmovnb  %edx, %eax
+       cmpw    $64, %ax
+       cmova   %edx, %eax
so not worse or better.  And, even when the conversion is the only stmt
in the basic block, that still doesn't allow the bb to be removed, we need
the empty bb for the PHIs.
My recollection is this was a piece of the puzzle towards solving 45397. Given that we haven't solved 45397 yet, I won't object if we want to reject SSA_NAME + INTEGER_CST at this time and come back to it when we dig into 45397 again.

jeff

Reply via email to