RE: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

[Michael Matz <matz at suse dot de>]

Hi,

On Wed, 26 Oct 2011, Jiangning Liu wrote:

> > > >> -
> > > >> -      if (LOGICAL_OP_NON_SHORT_CIRCUIT)
> > > >> -     {
> > > >> -       if (code != orig_code || lhs != orig_lhs || rhs !=
> > orig_rhs)
> > > >> -         return build2_loc (loc, code, truth_type, lhs, rhs);
> > > >> -       return NULL_TREE;
> > > >> -     }
> > > >
> > > > Why do you remove this hunk?  Shouldn't you instead move the hunk
> > you
> > > > added to fold_truth_andor() here.  I realize this needs some TLC to
> > > > fold_truth_andor_1, because right now it early-outs for non-
> > comparisons,
> > > > but it seems the better place.  I.e. somehow move the below code
> > into the
> > > > above branch, with the associated diddling on fold_truth_andor_1
> > that it
> > > > gets called.
> > >
> > > This hunk is removed, as it is vain to do here.
> > 
> > There is a fallthrough now, that wasn't there before.  I don't know if
> > it's harmless, I just wanted to mention it.
> > 
> 
> Yes, this part introduced different behavior for this small case,
> 
>   D.4710 = *i;
>   D.4711 = D.4710 != 0;
>   D.4712 = j != 2;
>   D.4713 = D.4711 & D.4712;
>   if (D.4713 != 0) goto <D.4714>; else goto <D.4715>;
> 
> After the fix, we have
> 
>   D.4711 = *i;
>   if (D.4711 != 0) goto <D.4712>; else goto <D.4710>;
>   <D.4712>:
>   if (j != 2) goto <D.4713>; else goto <D.4710>;

So, we have one more jump than originally, when the point of the patch 
was to emit less on targets with high branch costs.  So, as speculated, 
the hunk was not useless.  (It's nice that it caused a benchmark to 
improve significantly, but that should be done via a proper analysis and 
patch, not as a side effect of a supposed non-change).


Ciao,
Michael.