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

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

dnsampaio marked 3 inline comments as done.
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();
----------------
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.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:13656
+  }
+  assert(DstSz == Known.getBitWidth());
   }
----------------
sbaranga wrote:
> Is this assert unreachable?
It is. Compiling fore release does not test it. Thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56098