[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 62dd44d7. 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
Loading
Please register or sign in to comment