Re: [PATCH] Fix PR24574

[Richard Biener <rguenther at suse dot de>]

On Wed, 13 Jul 2016, Alexander Monakov wrote:

> Hi,
> 
> On Wed, 13 Jul 2016, Richard Biener wrote:
> > The following adds the ability to transform
> > 
> >  if (x != 0)
> >    x = x / 10;
> > 
> > to
> > 
> >  x = x / 10;
> > 
> > as requested by PR.  Plus it adds some more ops where such transform
> > is possible.
> 
> In the bugzilla, you said,
> 
> > Only for -Os, it's better to avoid the expensive division otherwise.
> 
> But the patch seems to apply the transform irrespective of -Os; is that
> deliberate?  (fwiw I agree with your bugzilla comment above)

I said that but I decided I was wrong as the conditional block will
be predicted as more likely executed.  Also we were doing the transform
for other ops already.  The most simple testcase has

;;   basic block 2, loop depth 0, count 0, freq 10000, maybe hot
;;    prev block 0, next block 3, flags: (NEW, REACHABLE)
;;    pred:       ENTRY [100.0%]  (FALLTHRU,EXECUTABLE)
  if (xD.1750_2(D) != 0)
    goto <bb 3>;
  else
    goto <bb 4>;
;;    succ:       3 [54.0%]  (TRUE_VALUE,EXECUTABLE)
;;                4 [46.0%]  (FALSE_VALUE,EXECUTABLE)

;;   basic block 3, loop depth 0, count 0, freq 5400, maybe hot
;;    prev block 2, next block 4, flags: (NEW, REACHABLE)
;;    pred:       2 [54.0%]  (TRUE_VALUE,EXECUTABLE)
  xD.1750_3 = xD.1750_2(D) / 10;

so not exactly a 50/50 prediction.  It is predicted by early return
predictor it seems.  If I make the predictor not apply we predict
it as 50/50 chance.  Hmm.  Honza?

Anyway, the phiopt transform is guarded with

  /* Size-wise, this is always profitable.  */
  if (optimize_bb_for_speed_p (cond_bb)
      /* The special case is useless if it has a low probability.  */
      && profile_status_for_fn (cfun) != PROFILE_ABSENT
      && EDGE_PRED (middle_bb, 0)->probability < PROB_EVEN
      /* If assign is cheap, there is no point avoiding it.  */
      && estimate_num_insns (assign, &eni_time_weights)
         >= 3 * estimate_num_insns (cond, &eni_time_weights))
    return 0;

and thus it should be handled correctly already (division and modulo
have higher time weights).

Richard.