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.
