[all-commits] [llvm/llvm-project] 8737dc: [SCEV] isHighCostExpansionHelper(): use correct TT...
Roman Lebedev via All-commits
all-commits at lists.llvm.org
Thu Mar 12 01:55:05 PDT 2020
Branch: refs/heads/master
Home: https://github.com/llvm/llvm-project
Commit: 8737dc2d32e6b76cfffa03cddabc8b5b1dd6f911
https://github.com/llvm/llvm-project/commit/8737dc2d32e6b76cfffa03cddabc8b5b1dd6f911
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-03-12 (Thu, 12 Mar 2020)
Changed paths:
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
Log Message:
-----------
[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:
>>! In D73501#1905171, @samparker wrote:
> 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
More information about the All-commits
mailing list