[PATCH] D79053: [SVE] Fix invalid use of VectorType::getNumElements() in Value Tracking

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau marked an inline comment as done.
ctetreau added a comment.

In D79053#2010799 <https://reviews.llvm.org/D79053#2010799>, @efriedma wrote:

> It's not obvious what DemandedElts should actually mean for a scalable vector.  How do you decide whether elements past the minimum length are actually demanded?  I can think of the following possibilities:
>
> 1. The elements beyond the minimum are always demanded
> 2. The demanded elements is a repeating pattern
> 3. We shouldn't use DemandedElts for scalable vectors at all.
>
>   It probably makes sense to just choose the last one until we actually have some real-world testscases that would benefit.  Also saves some implementation work short-term.
>
>   Given that, it probably makes sense to only construct DemandedElts for FixedVectorTypes, and just use `APInt(1, 1)` for scalable vectors.


So your proposal is that for scalable vectors, DemandedElts can have any value?



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1946
+  if (isa<ConstantVector>(V) && !EC->Scalable) {
+    const auto *CV = cast<ConstantVector>(V);
+    assert((CV->getNumOperands() == EC->Min) &&
----------------
efriedma wrote:
> No reason to check Scalable; a ConstantVector is never scalable anyway.
ConstantVector::getSplat() can return a scalable ConstantVector


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79053