[PATCH] D29261: [X86][GlobalISel] Add limited ret lowering support to the IRTranslator.

[Diana Picus via Phabricator via llvm-commits]

rovka added a comment.

I don't see anything wrong with this patch, but I'll let Zvi or someone else that knows the x86 ABI approve if the tests cover all interesting cases.



================
Comment at: lib/Target/X86/X86CallLowering.cpp:38
 
+void X86CallLowering::splitToValueTypes(const ArgInfo &OrigArg,
+                                        SmallVectorImpl<ArgInfo> &SplitArgs,
----------------
igorb wrote:
> rovka wrote:
> > I was wondering if it's possible to use just one of these for all backends.
> > 
> > In particular, I see you need to split values based on TLI.getNumRegisters/getRegisterType. I was looking into that myself for the ARM port. For ARM, I think it would be enough to extract the function from AArch64 and then add this splitting logic in the loop on the SplitVTs (and also not exit if we only have one VT - I think that doesn't do the right thing for AArch64 either if we have i128 arguments). Some generalization in the handling of bit offsets would be needed too, but that probably wouldn't be too exotic.
> > 
> > Would something like that work for x86 as well? I see that in the arg handling in SelectionDAG there are some special cases for some x86 calling conventions (vector call, intr) and obviously we don't want to special-case in the common code anymore. Do you see any way in which we could share some of this code across all backends, without doing anything awkward? Like I said, ARM and AArch64 are pretty similar, so some x86 perspective would definitely help here :)
> I think it is possible to use just one ( like void SelectionDAGISel::LowerArguments) with additional target specific hooks to handle special cases, most probably X86 back-end is the only one that need it for now.
> I don't know if it is a good long-term solution.
> May be we can break it to a few generic help factions , so very target can construct his own solution?
Sure, breaking it into several helper functions is a good idea. We can leave it for later if you think it's too soon to know what the best helpers would be.


Repository:
  rL LLVM

https://reviews.llvm.org/D29261