[PATCH] D124997: [InstCombine] Fix scalable-vector bitwise select matching

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D124997#3494120 <https://reviews.llvm.org/D124997#3494120>, @frasercrmck wrote:

> Oh, right you are! Sorry, I had the wrong impression of how this worked. Thank you.

The code comments could be improved. There are many different potential patterns within this block, and scalable vectors just make it harder to keep it all straight. :)
But after mucking around in here for a long time, I don't think the fix is sufficient. 
I added a test with 7bad1d281c798929a <https://reviews.llvm.org/rG7bad1d281c798929ae1be44b8c8a1e0713151ea9> to try to uncover another potential bug.
But I can't find a case currently where we would go wrong because code before here (ComputeNumSignBits) prevents matching a scalable vector with the right combination of bitcasts to trigger that bug.

A better fix should do something like this:

  // If this is a vector, we may need to cast to match the condition's length.
  Type *SelTy = A->getType();
  if (auto *VecTy = dyn_cast<VectorType>(Cond->getType())) {
    // For a fixed or scalable vector get N from <{vscale x} N x iM>
    unsigned Elts = VecTy->getElementCount().getKnownMinValue();
    // For a fixed or scalable vector, get the value N x iM; for a scalar this is just M.
    unsigned SelEltSize = SelTy->getPrimitiveSizeInBits().getKnownMinSize();
    Type *EltTy = Builder.getIntNTy(SelEltSize / Elts);
    SelTy = VectorType::get(EltTy, VecTy->getElementCount());
  }

I had a hard time making sense of the type and size APIs, so if anyone knows that better, please correct/improve if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124997