[PATCH] D112175: [NFC] Add llvm::StaticVector ADT

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:27958
 
-  SmallVector<SDValue, 64> LUTVec;
+  StaticVector<SDValue, 64> LUTVec;
   for (int i = 0; i < NumBytes; ++i)
----------------
I'm really not convinced that this is a good use of this data structure: the invariant that makes 64 value enough is non trivial and hard to validate.
The code above is:
```
  int NumElts = VT.getVectorNumElements();
  int NumBytes = NumElts * (VT.getScalarSizeInBits() / 8);
```

It isn't clear from there where the 64 comes from and what guarantee we have it'll be enough "always".
More importantly: I'm not convinced that this is the kind of situation where we would want "by construction" to never exceed the SmallVector or if instead it just isn't best to continue to "just" have SmallVector as an optimization without change in the overall "behavior contract" that usual with vectors.

On the other hand, I find the YAML example more convincing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112175