On Sat, Apr 01, 2017 at 01:43:36PM -0500, Segher Boessenkool wrote:
> Hi,
>
> On Sat, Apr 01, 2017 at 02:20:27PM +0200, Jakub Jelinek wrote:
> > As discussed in the PR, in the following testcase we don't if-convert
> > with the generic (and many other) tuning, because we default to
> > --param max-rtl-if-conversion-insns=1 in most of the tunings.
> > The problem we have is with multiple cmov instructions, but in the
> > testcase there is just one cmov and the other insn is turned into a SSE
> > max insn, which is fine.
> >
> > This patch stops artificially lowering that param, and for one_if_conv_insn
> > tuning it instead rejects the if-conversion if the resulting sequence has
> > multiple cmov instructions. The hook is passed if_info too, so it can
> > in the future do better heuristics based on predictability of the edges,
> > how far the uses of the cmov result are (I assume cmov major problem is
> > latency, right?) etc.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Does this change anything for targets that do not implement the new hook?
No. The patch changes all calls to noce_conversion_profitable_p into the
new target hook with the same parameters, and provides the old definition
of the function as the default implementation of the target hook.
So, other targets shouldn't see any change (ok, indirect vs. direct call
or inline in the compiler internals), and targets that choose to override
it can use the default hook as fallthrough or whatever else they choose.
> It isn't immediately obvious from the patch.
Jakub
