Re: [Patch 2/4] Hack out a use of MOVE_RATIO in tree-inline.c

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

On Thu, Sep 25, 2014 at 4:57 PM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
>
> Hi,
>
> This patch hookizes the use of MOVE_RATIO in
> tree-inline.c:estimate_move_cost as TARGET_ESTIMATE_BLOCK_COPY_NINSNS.
> This hook should return an estimate for the number of instructions
> which will be emitted to copy a block of memory.
>
> tree-inline.c uses this in inlining heuristics to estimate the cost of
> moving an object. The implementation is lacking, and will likely
> underestimate the size of most copies.
>
> An initial iteration of this patch migrated tree-inline.c to use
> move_by_pieces_profitable_p and move_by_pieces_ninsns, but this
> proved painful for performance on ARM.
>
> This patch puts the control in the hands of the backend, and uses
> the existing logic as a default.
>
> Bootstrapped on x86_64, ARM, AArch64.
>
> Ok?

Note that if you are here then one issue is that the inliner uses
this very same function to estimate cost of function parameters/returns
that are eventually passed/returned in registers.  That's of course
a pre-existing issue.

+ "This target hook should return an estimate of the number of\n\
+instructions which will be emitted when copying an object with a size\n\
+in units @var{size}.\n\

I'm confused by this sentence.  Doesn't it mean to say
"when copying an object with size @var{size} in units of word_mode."?

It's always difficult when transforming a heuristic using existing
target macros to a new hook.  It would be best to think about the
heuristic itself again and make the hook more closely match
the uses of the heuristic.  In this case it would mean splitting
this up into the load/store and the function parameter case.

Note that estimate_move_cost is used elsewhere as well.

Richard.

> Thanks,
> James
>
> ---
> 2014-09-25  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * target.def (estimate_block_copy_ninsns): New.
>         * targhooks.h (default_estimate_block_copy_ninsns): New.
>         * targhooks.c (default_estimate_block_copy_ninsns): New.
>         * tree-inline.c (estimate_move_cost): Use new target hook.
>         * doc/tm.texi.in (TARGET_ESTIMATE_BLOCK_COPY_NINSNS): New.
>         * doc/tm.texi: Regenerate.