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

[Sander de Smalen via Phabricator via lldb-commits]

sdesmalen added a comment.

Thanks for this patch @ctetreau! Overall looks great to me, just two little nits.



================
Comment at: llvm/include/llvm/IR/DerivedTypes.h:436
+
+  static VectorType *get(Type *ElementType, const VectorType *Other) {
+    return VectorType::get(ElementType, Other->getElementCount());
----------------
Do you need the method on line 432 if you have this one (that takes a `const VectorType *Other`) ?


================
Comment at: llvm/lib/ExecutionEngine/ExecutionEngine.cpp:1109
       for (unsigned i = 0; i < numElems; ++i)
         Result.AggregateVal[i].FloatVal = *((float*)Ptr+i);
     }
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77587