[PATCH] D78130: [SVE] Fixup calls to VectorType::getNumElements() in IR

Christopher Tetreault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 15 13:47:11 PDT 2020


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


================
Comment at: llvm/lib/IR/Instructions.cpp:1977
   if (const auto *CDS = dyn_cast<ConstantDataSequential>(Mask)) {
-    unsigned V1Size = cast<VectorType>(V1->getType())->getNumElements();
-    for (unsigned i = 0, e = MaskTy->getNumElements(); i != e; ++i)
+    unsigned V1Size = cast<FixedVectorType>(V1->getType())->getNumElements();
+    for (unsigned i = 0, e = cast<FixedVectorType>(MaskTy)->getNumElements();
----------------
efriedma wrote:
> ctetreau wrote:
> > By the way, I'm doing this to preserve the original behavior. The code used to enter this branch for scalable vectors. I don't know enough about this code to judge what it should actually do for scalable vectors so I'm making it loudly fail rather than silently be buggy.
> ConstantDataSequential is never scalable.  (The only valid scalable constants are zero, undef, and ConstantExprs.)
ugh. Seems I misread the code. What I meant was, for cases like:

```
if (auto *VTy = dyn_cast<VectorType>(Ty)) {
   auto *FVTy = cast<FixedVectorType>(VTy); // immediately cast my VectorType unconditionally
   ... // stuff
}
```

... that I'm doing it on purpose because trying to dyn_cast to FixedVectorType would be a behavior change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78130





More information about the llvm-commits mailing list