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

[silviu.baranga@arm.com via Phabricator via llvm-commits]

sbaranga accepted this revision.
sbaranga added a comment.
This revision is now accepted and ready to land.

LGTM! (with one nit)



================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:13643
+    ConstantSDNode *Pos = dyn_cast<ConstantSDNode>(Op.getOperand(1).getNode());
+    if (Pos && Pos->getAPIntValue().ult(NumSrcElts)) {
+      // If we know the element index, just demand that vector element.
----------------
dnsampaio wrote:
> dnsampaio wrote:
> > sbaranga wrote:
> > > I think this test should always evaluate to true. Maybe it should be an assert?
> > Don't know. Can't Pos be nullptr? That is, the position is not a constant value? (vector[variableX] ?)
> > if (Pos >= NumSrcElts), perhaps I could just consider it a poison and return as a value zero.
> Oh, I see that when creating these operands we only allow constant values. And trying llvm-mc it confirms it so. Sorry, did not get the suggestion. :)
Yeah, sorry, I should have given a justification for that.

These nodes are also used to only select instructions that have the lane encoded as an immediate.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:13642
+    const unsigned NumSrcElts = VecVT.getVectorNumElements();
+    ConstantSDNode *Pos = dyn_cast<ConstantSDNode>(Op.getOperand(1).getNode());
+    assert(Pos && "VGETGLANE index must be a constant value");
----------------
Nit: you can use a cast<> instead of dyn_cast here and remove the assert.


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

https://reviews.llvm.org/D56098