[llvm] [RISCV] Enable TTI::shouldDropLSRSolutionIfLessProfitable by default (PR #89927)

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Wed May 15 03:11:11 PDT 2024


asb wrote:

Thank you for taking a look. I've gone ahead and rebased to check that the changes remain similar.

My main goal of posting this quickly was to see if there was any memory on previous efforts in this direction given the original LSR flag came from RISC-V contributors. It sounds like there's not!

I agree that the changes to the in-tree vector code are a minor degradation - multiple cases where we're we have an additional scalar instruction in the loop. Not a big regression, but it's something and probably worth understanding. This patch also misses the test/Transforms/LoopStrengthReduce/RISCV changes that merit a closer look.

"Noise" is a good description and roughly what I would expect outside of the case I found or those like it.  The heuristics for LSR seem very rough and potentially misleading in some cases, so although I think dropping solutions we view as less profitable than the starting point is a sensible way to go and a better basis on which to build any further LSR improvements, it's likely to cause minor variations that aren't much better or worse in many cases.

I think based on the discussion here the next sensible steps are:
* Precommit the test case in the PR
  * It seems a useful example to have coverage for regardless of whether this ends up landing or not.
* Look more closely at why we see a minor regression in some of the changed vector test cases
* Run across a wider range of external inputs to look for surprises
  * I'd wondered if people had already done this, but it seems not

https://github.com/llvm/llvm-project/pull/89927


More information about the llvm-commits mailing list