[PATCH] D97299: [IR][SVE] Add new llvm.experimental.stepvector intrinsic

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:8961-8962
+  EVT ElemVT = VT.getScalarType();
+  if (ElemVT != MVT::i1)
+    return SDValue();
+
----------------
paulwalker-arm wrote:
> I'm not a fan of the pattern duplication within AArch64SVEInstrInfo.td so I'm thinking we should lower all ISD::STEP_VECTORs to AArch64ISD::INDEX_VECTOR.
> 
> Regardless of whether you do that I would expect the predicate handling to be more generic, for example:
> `return ISD::TRUNC(ISD::STEP_VECTOR(Op.getConstantOperandVal())`
Hi @paulwalker-arm, me too for i1 case, however I tried the generic route first exactly as you suggest and it didn't work. The problem was that if I created the ISD::TRUNC node we never ended up custom lowering as you'd expect. If you look at AArch64TargetLowering::LowerTRUNCATE for i1 element types we return this:

```return DAG.getSetCC(dl, VT, And, Zero, ISD::SETNE);```

which then never gets into our custom lowering code, despite the action being set to Custom for precisely those types. We then crash in isel trying to match a `setcc` operation. However I'd love to do it this way if you've got any suggestions for how I can solve this problem? I suspect there are just too many levels of custom lowering involved, i.e. custom lower STEP_VECTOR, followed by custom lower TRUNCATE, followed by custom lower SETCC.

So in general your preference is to custom lower *all* types and go through this function to create AArch64ISD::INDEX_VECTOR?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97299