[PATCH] D105587: [Instcombine]Transform reduction+(sext/zext(<n x i1>) to <n x im>) to [-]zext/trunc(ctpop(bitcast <n x i1> to in)) to im.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D105587#2864256 <https://reviews.llvm.org/D105587#2864256>, @lebedev.ri wrote:

> In D105587#2864252 <https://reviews.llvm.org/D105587#2864252>, @ABataev wrote:
>
>> In D105587#2864251 <https://reviews.llvm.org/D105587#2864251>, @RKSimon wrote:
>>
>>> Seems to be correct: https://alive2.llvm.org/ce/z/TBZbdZ
>>
>> Yes, checked it via alive2 before uploading
>
> While not required, since you already had it, it is really good to make patch description more useful by adding such details there.

Sure.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2006
+      Value *Vect;
+      if (match(Arg, m_ZExtOrSExt(m_Value(Vect)))) {
+        if (auto *FTy = dyn_cast<FixedVectorType>(Vect->getType()))
----------------
lebedev.ri wrote:
> Extension is optional: https://alive2.llvm.org/ce/z/N6qJGz
> Please either use `m_ZExtOrSExtOrSelf()`, or if it's already handled elsewhere add a comment about that.
Did not see such a pattern but will check if it is handled already and use add `m_ZExtOrSExtOrSelf`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105587