This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] On x86 allow if-conversion of more than one insn as long as there is at most one cmov (PR tree-optimization/79390)
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jeff Law <law at redhat dot com>, Bernd Schmidt <bschmidt at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Sun, 2 Apr 2017 20:44:03 +0200
- Subject: Re: [PATCH] On x86 allow if-conversion of more than one insn as long as there is at most one cmov (PR tree-optimization/79390)
- Authentication-results: sourceware.org; auth=none
- References: <20170401122027.GT17461@tucnak>
On Sat, Apr 1, 2017 at 2:20 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> 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?
>
> 2017-04-01 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/79390
> * target.h (struct noce_if_info): Declare.
> * targhooks.h (default_noce_conversion_profitable_p): Declare.
> * target.def (noce_conversion_profitable_p): New target hook.
> * ifcvt.h (struct noce_if_info): New type, moved from ...
> * ifcvt.c (struct noce_if_info): ... here.
> (noce_conversion_profitable_p): Renamed to ...
> (default_noce_conversion_profitable_p): ... this. No longer
> static nor inline.
> (noce_try_store_flag_constants, noce_try_addcc,
> noce_try_store_flag_mask, noce_try_cmove, noce_try_cmove_arith,
> noce_convert_multiple_sets): Use targetm.noce_conversion_profitable_p
> instead of noce_conversion_profitable_p.
> * config/i386/i386.c: Include ifcvt.h.
> (ix86_option_override_internal): Don't override
> PARAM_MAX_RTL_IF_CONVERSION_INSNS default.
> (ix86_noce_conversion_profitable_p): New function.
> (TARGET_NOCE_CONVERSION_PROFITABLE_P): Redefine.
> * config/i386/x86-tune.def (X86_TUNE_ONE_IF_CONV_INSN): Adjust comment.
> * doc/tm.texi.in (TARGET_NOCE_CONVERSION_PROFITABLE_P): Add.
> * doc/tm.texi: Regenerated.
>
> * gcc.target/i386/pr79390.c: New test.
> * gcc.dg/ifcvt-4.c: Use -mtune-ctrl=^one_if_conv_insn for i?86/x86_64.
x86 part LGTM.
Hopefully, this infrastructure will allow us to fix (or it already
fixes) PR 56309 [1].
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56309
Thanks,
Uros.