Follow up to D25691, this sets up the plumbing necessary to support vector demanded elements support in known bits calculations in target nodes.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
include/llvm/Target/TargetLowering.h | ||
---|---|---|
2416 | Do we need to describe the DemandedElts parameter here? One thing is that the SelectionDAG::computeKnownBits method currently does the check if DemandedElts is empty. So the method is not supposed to be called with an empty DemandedElts. Is this something that all the target implementations should rely on? Otherwise I guess you need to add the same kind of early-out for each target implementation? (Or is it correct to answer with a set of known bits even if the request was for no element?) | |
lib/Target/AMDGPU/AMDGPUISelLowering.cpp | ||
3556 | It would have been a smaller diff to review if only adding the line with DemandedElts. Now I needed to understand if something else changed as well here due to mixing cleanup and adding functionality in the same patch. Anyway, the change looks OK so this is just a minor remark. |
Updated after clang-format'ing AMDGPUISelLowering.cpp and adding DemandedElts to doxygen comment.
I can add an assertion to the start of each computeKnownBitsForTargetNode if you think its worth it?
No, I think we can skip the asserts. I might actually have been thinking wrong about it, since it should be allowed to ignore DemandedElts (and return som known bits) even if DemandedElts is empty.
Do we need to describe the DemandedElts parameter here?
One thing is that the SelectionDAG::computeKnownBits method currently does the check if DemandedElts is empty. So the method is not supposed to be called with an empty DemandedElts. Is this something that all the target implementations should rely on? Otherwise I guess you need to add the same kind of early-out for each target implementation? (Or is it correct to answer with a set of known bits even if the request was for no element?)
Not really sure if it helps to add a comment about that here, but I guess we have no way to really encode that DemandedElts should be non-empty. Another approach might be to add asserts in every target implementation that DemandedElts isn't empty.