[PATCH] D100099: [X86][CostModel] Try to fix cost computation load/stores of non-power-of-two vectors

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:3222-3230
+      SmallVector<unsigned, CHAR_BIT * sizeof(NumElem)> Factors;
+      for (unsigned Bit = 0; Bit != CHAR_BIT * sizeof(NumElem); ++Bit) {
+        unsigned Factor = unsigned(1) << Bit;
+        if (NumElem & Factor)
+          Factors.emplace_back(Factor);
+      }
+      assert(std::accumulate(Factors.begin(), Factors.end(), unsigned(0)) ==
----------------
RKSimon wrote:
> ABataev wrote:
> > Why not just something like this:
> > ```
> > unsigned Factor = 0;
> > for (; NumElem > 0; NumElem -= Factor) {
> >   Factor = PowerOf2Floor(NumElem);
> >   .....
> > }
> > ```
> +1 Having Factor updated in the condition as well as being used in increment block is difficult to grok
Note that i have addressed @ABataev's comment, it was about earlier patch version:
https://reviews.llvm.org/D100099?id=336063#change-OQEVJvBxQbDZ


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100099