[PATCH] D79197: [SVE] Fix invalid usage of getNumElements() in InstCombineMulDivRem

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

In D79197#2017388 <https://reviews.llvm.org/D79197#2017388>, @ctetreau wrote:

> > I am thinking at the situation in which a scalable vector is the splat of a power of 2, say `[2^c, 2^c, ..., 2^c]`. I think in this case returning `nullptr` might lead to a missed optimization opportunity? For example, I have highlighted a place in the code where the power of 2 is used to simplify `udiv`. I think we want to have that optimization for scalable vectors too: surely we can retrieve the scalable vector "splat of C" from the scalable vector "splat of 2^C"?
>
> My current mandate is to fix all the misuses of getNumElements(). Right now we're in a situation where llvm does some transformations that maybe speeds things up sometimes, but other times causes a correctness issue or crash. I'm more concerned with fixing the correctness issues and crashes; the optimizations can be reintroduced after the compiler works. Step 1 in "make it work, then make if fast" is "make it work" after all. For many of these fixes that I make, there's something the code can do to take the optimization opportunity with scalable vectors. However, if I spend the time for each one to find all of these I'll never get this done.


Sure, I understand. My comment was mostly raised by the message "this cannot be done for scalable vectors" in the summary. I think that you should amend the commit message and add a comment in the code before `if (!isa<FixedVectorType>(Ty))` saying something along the lines of `FIXME: we can extract pow of 2 of splat constant for scalable vectors.`

Thank you!

Francesco


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79197/new/

https://reviews.llvm.org/D79197