[PATCH] D25691: [DAGCombiner] Add vector demanded elements support to computeKnownBits
Bjorn Pettersson via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 24 12:50:19 PDT 2016
bjope added inline comments.
================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:2057
+ // Known bits are the values that are shared by every demanded element.
// TODO: support per-element known bits.
KnownOne &= KnownOne2;
----------------
Shouldn't we remove this TODO now? Or is that a different idea about the possibility to fetch a set of individual known bits for each element in one request?
If originating value is a vector, then the caller could call computeKnownBits for one demanded element at a time. Doing it for all elements in one go will be compilcated since the recursion could be different for different elements.
================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:2559
+ } else {
+ computeKnownBits(InVec, KnownZero, KnownOne, Depth + 1);
+ }
----------------
I suggest that you explicitly show that a decision was made to not track a specific element here. And maybe more importantly show that we can not use the original DemandedElts in the recursive call by selecting a new bit mask.
This could be done by making the recursion like this:
computeKnownBits(InVec, KnownZero, KnownOne, APInt::getAllOnesValue(VecVT.getVectorNumElements()), Depth + 1);
================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:2568
case ISD::BSWAP: {
computeKnownBits(Op.getOperand(0), KnownZero2, KnownOne2, Depth+1);
KnownZero = KnownZero2.byteSwap();
----------------
Any reason for not providing the DemandedElts in this recursive call?
================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:2539
case ISD::EXTRACT_VECTOR_ELT: {
// At the moment we keep this simple and skip tracking the specific
// element. This way we get the lowest common denominator for all elements
----------------
I guess this comment needs to be revised, or maybe removed.
And at least the TODO should be removed.
Repository:
rL LLVM
https://reviews.llvm.org/D25691
More information about the llvm-commits
mailing list