[PATCH] D130778: [NFC] Simplify some conversions from ArrayRef to SmallVector by using to_vector and to_vector_of utilities

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp:814
   // Otherwise, create TBAA with the new Len
-  ArrayRef<MDOperand> MDOperands = MD->operands();
-  SmallVector<Metadata *, 4> NextNodes(MDOperands.begin(), MDOperands.end());
+  auto NextNodes = to_vector_of<Metadata *, 4>(MD->operands());
   ConstantInt *PreviousSize = mdconst::extract<ConstantInt>(NextNodes[3]);
----------------
yurai007 wrote:
> dblaikie wrote:
> > I'm not sure changes like this add value - if we end up with a ctor that can take `MD->operands()` as a single argument. Is that possible already/will it be possible with some of the patches you're working on?
> Taking into account parallel change which adds SmallVector(ArrayRef<U>) the answer is yes - NextNodes (SmallVector) can be created directly from MD->operands(). So yeah we can skip using to_vector_of here.
Could you remove this/changes like this from the patch then, or do you plan to wait on changes for this patch until the ArrayRef ctor is added? (you can flag this review as "changes planned" or something)


================
Comment at: llvm/unittests/Analysis/VectorUtilsTest.cpp:510
   bool validParams(ArrayRef<VFParameter> Parameters) {
-    Shape.Parameters =
-        SmallVector<VFParameter, 8>(Parameters.begin(), Parameters.end());
+    Shape.Parameters = llvm::to_vector<8>(Parameters);
     return Shape.hasValidParameterList();
----------------
yurai007 wrote:
> dblaikie wrote:
> > if there is/we end up with a ctor that takes a range, maybe there should also be an `assign` function that does the same, that would be used here? `Shape.Parameters.assign(Parameters)`
> I can think of extending SmallVector with range version of assign, but haven't checked yet how much value it would add excluding this particular usage. If you don't mind I would like to postpone it until rest of SmallVector related patches are submitted.
Fair enough - it is a bit unfortunate that this still causes copies & has to re-encode the small size (`8` in this instance) but yeah, this is still simpler/tidier than the previous code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130778