HomePhabricator

[SCEV] isHighCostExpansionHelper(): use correct TTI hooks

Authored by lebedev.ri on Mar 12 2020, 1:33 AM.

Description

[SCEV] isHighCostExpansionHelper(): use correct TTI hooks

Summary:
Cost modelling strikes again.
In PR44668 https://bugs.llvm.org/show_bug.cgi?id=44668 patch series,
i've made the same mistake of always using generic getOperationCost()
that i missed in reviewing D73480/D74495 which was later fixed
in 62dd44d76da9aa596fb199bda8b1e8768bb41033.

We should be using more specific hooks instead - getCastInstrCost(),
getArithmeticInstrCost(), getCmpSelInstrCost().

Evidently, this does not have an effect on the existing testcases,
with unchanged default cost budget. But if it *does* have an effect
on some target, we'll have to segregate tests that use this function
per-target, much like we already do with other TTI-aware transform tests.

There's also an issue that @samparker has brought up in post-commit-review:

Hi,
Did you get performance numbers for these patches? We track the performance
of our (Arm) open source DSP library and the cost model fixes were generally
a notable improvement, so many thanks for that! But the final patch
for rewriting exit values has generally been bad, especially considering
the gains from the modelling improvements. I need to look into it further,
but on my current test case I'm seeing +30% increase in stack accesses
with a similar decrease in performance.
I'm just wondering if you observed any negative effects yourself?

I don't know if this addresses that, or we need D66450 for that.

Reviewers: samparker, spatel, mkazantsev, reames, wmi

Reviewed By: reames

Subscribers: kristof.beyls, hiraditya, danielkiss, llvm-commits, samparker

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D75908