[PATCH] D102765: [SelectionDAG] Add stub implementation of ReplaceInsertSubVectorResults
Paul Walker via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 21 06:31:02 PDT 2021
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
More information about the llvm-commits
mailing list