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

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16728-16730
+  case ISD::INSERT_SUBVECTOR:
+    ReplaceInsertSubVectorResults(N, Results, DAG);
+    return;
----------------
joechrisellis wrote:
> 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.
That makes sense, thanks for checking @joechrisellis .

In which I'll just say there doesn't seem much value in creating an empty function, but rather can just add a comment here along the lines of "We've requested custom lowering but rely on common code for result type legalisation." and return directly.

Given this'll make this a two line change without any tests I think you may as well just add the code to D102766.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102765