Re: [Patch ifcvt costs 0/3] Introduce a new target hook for ifcvt costs.

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, Sep 25, 2015 at 5:04 PM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
> Hi,
>
> In relation to the patch I put up for review a few weeks ago to teach
> RTL if-convert to handle multiple sets in a basic block [1], I was
> asking about a sensible cost model to use. There was some consensus at
> Cauldron that what should be done in this situation is to introduce a
> target hook that delegates answering the question to the target.

Err - the consensus was to _not_ add gazillion of special target hooks
but instead enhance what we have with rtx_cost so that passes can
rely on comparing before and after costs of a sequence of insns.

Richard.

> This patch series introduces that new target hook to provide cost
> decisions for the RTL ifcvt pass.
>
> The idea is to give the target full visibility of the proposed
> transformation, and allow it to respond as to whether if-conversion in that
> way is profitable.
>
> In order to preserve current behaviour across targets, we will need the
> default implementation to keep to the strategy of simply comparing branch
> cost against a magic number. Patch 1/3 performs this refactoring, which is
> a bit hairy in some corner cases.
>
> Patch 2/3 is a simple code move, pulling the definition of the if_info
> structure used by RTL if-convert in to ifcvt.h where it can be included
> by targets.
>
> Patch 3/3 then introduces the new target hook, with the same default
> behaviour as was previously in noce_is_profitable_p.
>
> The series has been bootstrapped on ARM, AArch64 and x86_64 targets, and
> I've verified with Spec2000 and Spec2006 runs that there are no code
> generation differences for any of these three targets after the patch.
>
> I also gave ultrasparc3 a quick go, from what I could see, I changed the
> register allocation for the floating-point condition code registers.
> Presumably this is a side effect of first constructing RTXen that I then
> discard. I didn't see anything which looked like more frequent reloads or
> substantial code generation changes, though I'm not familiar with the
> intricacies of the Sparc condition registers :).
>
> I've included a patch 4/3, to give an example of what a target might want
> to do with this hook. It needs work for tuning and deciding how the function
> should actually behave, but works if it is thought of as more of a
> strawman/prototype than a patch submission.
>
> Are parts 1, 2 and 3 OK?
>
> Thanks,
> James
>
> [1]: https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00781.html
>
> ---
> [Patch ifcvt 1/3] Factor out cost calculations from noce cases
>
> 2015-09-26  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * ifcvt.c (noce_if_info): Add a magic_number field :-(.
>         (noce_is_profitable_p): New.
>         (noce_try_store_flag_constants): Move cost calculation
>         to after sequence generation, factor it out to noce_is_profitable_p.
>         (noce_try_addcc): Likewise.
>         (noce_try_store_flag_mask): Likewise.
>         (noce_try_cmove): Likewise.
>         (noce_try_cmove_arith): Likewise.
>         (noce_try_sign_mask): Add comment regarding cost calculations.
>
> [Patch ifcvt 2/3] Move noce_if_info in to ifcvt.h
>
> 2015-09-26  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * ifcvt.c (noce_if_info): Move to...
>         * ifcvt.h (noce_if_info): ...Here.
>
> [Patch ifcvt 3/3] Create a new target hook for deciding profitability
>     of noce if-conversion
>
> 2015-09-26  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * target.def (costs): New hook vector.
>         (ifcvt_noce_profitable_p): New hook.
>         * doc/tm.texi.in: Document it.
>         * doc/tm.texi: Regenerate.
>         * targhooks.h (default_ifcvt_noce_profitable_p): New.
>         * targhooks.c (default_ifcvt_noce_profitable_p): New.
>         * ifcvt.c (noce_profitable_p): Use new target hook.
>
> [Patch Prototype AArch64 ifcvt 4/3] Wire up the new if-convert costs
>     hook for AArch64
>
> 2015-09-26  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * config/aarch64/aarch64.c
>         (aarch64_additional_branch_cost_for_probability): New.
>         (aarch64_ifcvt_noce_profitable_p): Likewise.
>         (TARGET_COSTS_IFCVT_NOCE_PROFITABLE_P): Likewise.