[PATCH] D82211: [SVE] Remove calls to VectorType::getNumElements from ExecutionEngine

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/ExecutionEngine/ExecutionEngine.cpp:630
         // if the whole vector is 'undef' just reserve memory for the value.
-        auto *VTy = cast<VectorType>(C->getType());
+        auto *VTy = cast<FixedVectorType>(C->getType());
         Type *ElemTy = VTy->getElementType();
----------------
fpetrogalli wrote:
> Like you have done below, it would be useful to have the following comment before the explicit cast:
> 
> ```
> // FIXME: this needs to handle scalable vectors correctly
> ```
> 
> On a side note - haven't you changed the compiler behavior here? What was resulting in a warning (taking this case for a scalable vector) would be resulting in a crash (if I understand correctly).
> 
> Given that the scalable case will look different from the fixed width one, why not just duplicate the code for each of the two case statements, making sure that the scalable one uses scalable vectors (with the current behavior) and the fixed vector one uses fixed vectors (with the warning removed). Something like:
> 
> ```
>       case Type::FixedVectorTyID:
>         // if the whole vector is 'undef' just reserve memory for the value.
>         auto *VTy = cast<FixedVectorType>(C->getType());
>         Type *ElemTy = VTy->getElementType();
>         unsigned int elemNum = VTy->getNumElements();
>         Result.AggregateVal.resize(elemNum);
>         if (ElemTy->isIntegerTy())
>           for (unsigned int i = 0; i < elemNum; ++i)
>             Result.AggregateVal[i].IntVal =
>                 APInt(ElemTy->getPrimitiveSizeInBits(), 0);
>         break;
>       case Type::ScalableVectorTyID:
>         // if the whole vector is 'undef' just reserve memory for the value.
>         auto *VTy = cast<ScalableVectorType>(C->getType());
>         Type *ElemTy = VTy->getElementType();
>         // FIX ME: fix implementation for scalable vectors
>         unsigned int elemNum = VTy->getElementCount().Min;
>         Result.AggregateVal.resize(elemNum);
>         if (ElemTy->isIntegerTy())
>           for (unsigned int i = 0; i < elemNum; ++i)
>             Result.AggregateVal[i].IntVal =
>                 APInt(ElemTy->getPrimitiveSizeInBits(), 0);
>         break;
> ```
Would this not be sufficient:
```case Type::ScalableVectorTyID:
  // FIXME: Add ExecutionEngine support for scalable vectors
  report_fatal_error("Scalable vector support not yet implemented in ExecutionEngine");```
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82211