[PATCH] D124284: [SLP]Try partial store vectorization if supported by target.

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 2 02:10:12 PDT 2022


dmgreen added a comment.

> I tried something similar already, it won't work. Plus, trunc store is not the case we're looking at here, it is different. This function just says to vectorizer that it might be worth trying this vector factor. The cost model should later inform that it is not profitable. If something is not correct in the TTI, it should be fixed in TTI.

Oh OK, that's a shame. There may be something a little off with the f16 costmodel, it is not always perfect, but I don't see anything obvious from what it is printing. There are only v2f16 values, which can't go too wrong.
The issue isn't that SLP vectorization is worse than scalar, it's that in that particular case runtime unrolling is better. The SLP that happens can get in the way of something more profitable, and no pass in llvm operates in a vacuum.

That issue isn't too important though. My worry is that this is currently an expensive way of saying "return 2". I've no strong objection if you want to go with the current method, but perhaps the default should be more "correct" and we can override the targets that want something different/more aggressive? They can choose to spend the extra compile time on factors that might not be expected to be very profitable to other archs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124284



More information about the llvm-commits mailing list