[PATCH] D53258: [LegalizeDAG] Add generic vector CTPOP expansion (PR32655) (WIP)

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1091
+      TLI.isOperationLegalOrCustom(ISD::SRL, VT) &&
+      (NumBitsPerElt == 8 || TLI.isOperationLegalOrCustom(ISD::MUL, VT)) &&
+      TLI.isOperationLegalOrCustomOrPromote(ISD::OR, VT) &&
----------------
efriedma wrote:
> It should be possible to avoid using MUL if it isn't legal. Not sure how much work that would be to fix.
> 
> It's generally not a good idea to push vector legalization work to LegalizeDAG; there's a separate legalization stage for vectors precisely because LegalizeDAG can't unroll arbitrary vectors.  (This is the issue you mentioned with ARM support for v2i64 multiply.)  It would be better to factor out the expansion routine into a separate function, and call it from both VectorLegalizer and LegalizeDAG.
Sounds good (and should help D52965 as well) - do we have existing examples of this?


Repository:
  rL LLVM

https://reviews.llvm.org/D53258