[llvm] [RISCV][GISel] Support G_MERGE_VALUES/G_UNMERGE_VALUES with Zfa. (PR #120379)

Amara Emerson via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 19 20:47:34 PST 2024


aemerson wrote:

> > > I'm not sure if its a good idea to make GINodeEquiv between a target independent generic opcode and a target dependent SelectionDAG opcode. Similar is done on Mips. And I saw some G_LOAD/G_STORE equivalents in AMDGPU so maybe it's ok?
> > 
> > 
> > Is the general case of merge/unmerge really a 1-1 semantic mapping with your target node? If not I wouldn't advise going down this route, maybe lowering them into something more specific (like G_RISCV_MERGE) and then specifying the node equivalence would be a more precise route.
> 
> Its the only case we have of G_MERGE_VALUES/UNMERGE_VALUES right now. Not sure if we will need more in the future. Looking at tablegen it looks like the mapping is from SelectionDAG node to GISelEquiv so the same GISel opcode can be mapped to multiple SelectionDAG opcodes?

What I meant by 1-1 was that G_MERGE_VALUES takes any number of scalars and merges into a larger scalar. If your target node exactly implements the legal G_MERGE_VALUES variants for RISC-V then I guess it's ok. But if not you may run into issues later where the selector can't handle some of the edge cases.
> 
> Where should I do the "lowering" if I were going to add G_RISCV_MERGE?
If you don't want to implement a PostLegalizerLowering pass like AArch64 then I guess you could do it in `preISelLower()` instead? It's your call though, if you think this is ok for now go ahead. It's fairly easy to fix later if needed.



https://github.com/llvm/llvm-project/pull/120379


More information about the llvm-commits mailing list