[PATCH] D56098: [ARM] Teach ComputeKnownBitsTarget to handle extract vectors

[Diogo N. Sampaio via Phabricator via llvm-commits]

dnsampaio added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:13639
+    const SDValue &SrcSV = Op.getOperand(0);
+    Known = DAG.computeKnownBits(SrcSV, Depth + 1);
+    EVT VT = Op.getValueType();
----------------
RKSimon wrote:
> dnsampaio wrote:
> > sbaranga wrote:
> > > IIUC we're computing here the known bits on the input vector operand. However, when computing the result of the VGETLANE operation we're not taking into account the lane operand, which seems wrong to me.
> > > 
> > > We should have additional tests to cover both the sext and zext cases and cases where the lane operand is not zero.
> > Perhaps I've got it wrong, but from what I've seen is that computeKnownBits tells bits that are known to be zero or one in all vector elements (lanes). So it really does not matter which lane is being extracted. Even the width is the number of bits of a single element, not the entire vector.
> > 
> > I'll add a signed extension test, to show that the behavior is the same.
> computeKnownBits can takes DemandedElts mask to indicate the vector elements that you care about - see the ISD::EXTRACT_VECTOR_ELT code in SelectionDAG::computeKnownBits for an example of how it works.
Hi, thanks for the suggestion. I just couldn't find a straightforward test to capture such behavior, as most simplifications are done before extractelement gets replaced by VGETLANE. It doesn't seems to have an intrinsic as well.


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

https://reviews.llvm.org/D56098