[PATCH] D87651: [AArch64][SVE] Implement extractelement of i1 vectors.

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:9050-9056
+    SDLoc DL(Op);
+    SDValue Extend = DAG.getNode(ISD::SIGN_EXTEND, DL, VectorVT,
+                                 Op.getOperand(0));
+    MVT ExtractTy = ScalarVT == MVT::i64 ? MVT::i64 : MVT::i32;
+    SDValue Extract = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, ExtractTy,
+                                  Extend, Op.getOperand(1));
+    return DAG.getSExtOrTrunc(Extract, DL, Op.getValueType());
----------------
efriedma wrote:
> paulwalker-arm wrote:
> > This is a common route when expanding i1 based operations so downstream we did it in common code, for example:
> > ```
> > setOperationAction(ISD::EXTRACT_VECTOR_ELT, MVT::nxv2i1, Promote);
> > AddPromotedToType(ISD::EXTRACT_VECTOR_ELT, MVT::nxv2i1, MVT::nxv2i64);
> > ```
> > and updated SelectionDAGLegalize::PromoteNode accordingly.  Is this route something we can do upstream? instead of having custom lowering.
> > 
> The problem is that "promote" is currently interpreted to mean "bitcast the vector operands" by vector legalization, not integer promotion, so it would be confusing to use it like this.  Maybe worth messing with LegalizeAction to distingush between the two kinds of "promotion", though.
Within AArch64ISelLowering.cpp I can see:
```
setOperationAction(ISD::FADD, MVT::v4f16, Promote);
AddPromotedToType(ISD::FADD, MVT::v4f16, MVT::v4f32);
```
With SelectionDAGLegalize::PromoteNode applying the appropriate extension.  I can see similar usages like this for other targets. To me it looks like "Promote" can mean promotion or bitcast, with nodes having support for whichever has been required in the past. Based on this it seems safe to extend PromoteNode's handling of EXTRACT_VECTOR_ELT to include real promotion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87651