[PATCH] D102765: [SelectionDAG] Add stub implementation of ReplaceInsertSubVectorResults

[Joe Ellis via Phabricator via llvm-commits]

joechrisellis added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16728-16730
+  case ISD::INSERT_SUBVECTOR:
+    ReplaceInsertSubVectorResults(N, Results, DAG);
+    return;
----------------
paulwalker-arm wrote:
> I'm not saying this is not the correct solution but you typically opt into having custom handling via `ReplaceNodeResults` so perhaps we've requested (via `setOperationAction()`) something we didn't want?
That's a good point. I reckon we're hitting the unreachable because of the following code block in `AArch64ISelLowering.cpp`:

```
// Illegal unpacked integer vector types.
for (auto VT : {MVT::nxv8i8, MVT::nxv4i16, MVT::nxv2i32}) {
  setOperationAction(ISD::EXTRACT_SUBVECTOR, VT, Custom);
  setOperationAction(ISD::INSERT_SUBVECTOR, VT, Custom); // <--
}
```

I haven't done much digging, but I think that's important. Reverting this patch locally, then removing that line fixes the issues that this patch resolves, but means that the following tests fail:

```
llvm/test/CodeGen/AArch64/sve-masked-gather-legalize.ll
llvm/test/CodeGen/AArch64/sve-split-fcvt.ll
llvm/test/CodeGen/AArch64/sve-split-trunc.ll
```

with `Do not know how to promote this operator's operand!`. This makes me think we probably need to keep those lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102765