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

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

Hi @ctetreau ,

you say:

> getLogBase2 tries to walk its input vector type to do its work. This
>  cannot be done for scalable vectors

I agree with you if we imagine the input scalable vector consisting of different powers of two, say  `[2^c1, 2^c2, 2^c3, ...]`

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"?

Thank you for working on this!

Regards,

Francesco



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:847-849
   Constant *C1 = getLogBase2(Op0->getType(), cast<Constant>(Op1));
   if (!C1)
     llvm_unreachable("Failed to constant fold udiv -> logbase2");
----------------
Wouldn't your changes get to LLVM unreachable here? Would it make sense to have a test that checks that `X udiv Y` don't crash the compiler when both X and Y are scalable vector and Y is a splat of a power of 2? 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79197