[RFC: Patch 0/6] Rewrite the noce-ifcvt cost models

[James Greenhalgh <james dot greenhalgh at arm dot com>]

Hi,

When I was working in the area last year, I promised to revisit the cost
model for noce if-conversion and see if I could improve the modeling. This
turned out to be more tricky than I expected.

This patch set rewrites the cost model for noce if-conversion. The goal is
to rationalise the units used in the calculations away from BRANCH_COST,
which is only defined relative to itself.

I've introduced a new target hook "rtx_branch_cost" which is defined to return
the cost of a branch in RTX cost units, suitable for comparing directly with
the calculated cost of a conditional move, or the conditionally executed
branches.

If you're looking at that and thinking it doesn't sound much different from
our current call to BRANCH_COST, you're right. This isn't as large a
departure from the existing cost model as I had originally intended. I
started out experimenting with much larger hooks (with many
parameters/pass-specific data), or hooks that passed the whole edge to
the target asking for the cost. These ended up feeling quite silly
to write in the target, and don't match with the direction discussed at
last year's cauldron. We don't want to go around leaking pass-internal
data around to back-ends. That is a path of madness as the passes change but
find that targets have invented baroque calculations to help invent a
magic number.

I tried implementing a "replacement_cost" hook, which would take before
and after code sequences and try to guess profitability, but because you
want to take edge probabilities in to consideration while trying to calculate
the costs of an if-then-else structure, the code gets hairy quickly. Worse
is that this would need duplicating across any target implementing the
hook. I found that I was constructing lots of RTX just to throw it away
again, and sometimes we were constructing RTX that would trivially be
optimised by a future pass. As a metric for if-conversion, this hook
seemed more harmful than useful for both the quality of the decision we'd
make, and for the quality of the GCC source.

As I iterated through versions of this patch set, I realised that all we
really wanted for ifcvt was a way to estimate the cost of a branch in units
that were comparable to the cost of instructions. The trouble with BRANCH_COST
wasn't that it was returning a magic number, it was just that it was returning
a magic number which had inconsistent meanings in the compiler. Otherwise,
BRANCH_COST was a straightforward, low-complexity target hook.

So the new hook simply defines the relative units that it will use and
splits off the use in ifcvt from other BRANCH_COST calls.

Having introduced the hook, and added some framework to make use of it, the
rest of the patch set works through each of the cost models in ifcvt.c,
makes them consistent, and moves them to the new hook.

This act of making the cost models consistent will cause code generation
changes on a number of targets - most notably x86_64. On x86_64 the RTX
cost of a conditional move comes out at "20" - this is far higher than
COSTS_N_INSNS (BRANCH_COST) for the x86 targets, so they lose lots
of if-conversion. The easy fix for this would be to implement the new hook.
I measured the performance impact on Spec2000 as a smoke test, it didn't
seem to harm anything, and the result was a slight (< 3%) uplift on
Spec2000FP. I'm no expert on x86_64, so I haven't taken a closer look for
the reasons.

Having worked through the patch set, I'd say it is probably a small
improvement over what we currently do, but I'm not very happy with it. I'm
posting it for comment so we can discuss any directions for costs that I
haven't thought about or prototyped. I'm also happy to drop the costs
rewrite if this seems like complexity for no benefit.

Any thoughts?

I've bootstrapped and tested the patch set on x86_64 and aarch64, but
they probably need some further polishing if we were to decide this was a
useful direction.

Thanks,
James

James Greenhalgh (6):
  [RFC: Patch 1/6] New target hook: rtx_branch_cost
  [RFC: Patch 2/6] Factor out the comparisons against magic numbers in
    ifcvt
  [RFC: Patch 3/6] Remove if_info->branch_cost
  [RFC: Patch 4/6] Modify cost model for noce_cmove_arith
  [RFC: Patch 5/6] Improve the cost model for multiple-sets
  [RFC: Patch 6/6] Remove second cost model from
    noce_try_store_flag_mask

 gcc/doc/tm.texi    |  10 +++
 gcc/doc/tm.texi.in |   2 +
 gcc/ifcvt.c        | 204 +++++++++++++++++++++++++++++++++++++++++------------
 gcc/target.def     |  14 ++++
 gcc/targhooks.c    |  10 +++
 gcc/targhooks.h    |   2 +
 6 files changed, 197 insertions(+), 45 deletions(-)