[Lldb-commits] [PATCH] D77587: [SVE] Add new VectorType subclasses

[Christopher Tetreault via Phabricator via lldb-commits]

ctetreau marked 2 inline comments as done.
ctetreau added inline comments.


================
Comment at: llvm/include/llvm/IR/DerivedTypes.h:436
+
+  static VectorType *get(Type *ElementType, const VectorType *Other) {
+    return VectorType::get(ElementType, Other->getElementCount());
----------------
sdesmalen wrote:
> Do you need the method on line 432 if you have this one (that takes a `const VectorType *Other`) ?
It shouldn't be needed, I'll remove it.


================
Comment at: llvm/lib/ExecutionEngine/ExecutionEngine.cpp:1109
       for (unsigned i = 0; i < numElems; ++i)
         Result.AggregateVal[i].FloatVal = *((float*)Ptr+i);
     }
----------------
sdesmalen wrote:
> This code doesn't really work for scalable vectors. Assuming you don't want to change that in this patch, Is it worth putting a FIXME here?
This will have to be changed when getNumElements() moves into FixedVectorType. I'll get it then.

Likely, the plan will be to just have the scalable vector branch assert. My overall strategy is to just assume all calls to getNumElements are correct, cast to FixedVectorType instead of VectorType, and just fix enough to make the tests pass.

There's a lot of code that calls getNumElements, and the overall goal is to force everybody to clean their own houses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77587