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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

Cheers @lebedev.ri, the premise seems fine, and the costs are a lot more sensible



================
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)) ==
----------------
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100099