[PATCH] D132750: [SLP]Fix PR57322: vectorize constant float stores.

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie accepted this revision.
vdmitrie added a comment.
This revision is now accepted and ready to land.

Looks acceptable.



================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5835
+    auto *CFlt = dyn_cast<ConstantFP>(Inst->getOperand(OpIdx));
+    if (!CInt && (!EnableFP || !CFlt)) {
       VK = TTI::OK_AnyValue;
----------------
As I said earlier we should never have a situation of mixed types. So I do not quite understand why you are still sticking with packing the two flows into the single loop. This does not save anything but makes the code to be error prone. Just look: the condition at 5840 should probably be if (EnableFP && CFlt). You also do not use CFlt to access the object but only for null pointer check.
Technically yes, this code will work. But I appreciate if you address maintenance concern as well.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132750