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

Igor Breger via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 31 05:37:42 PST 2017

igorb added inline comments.

Comment at: lib/Target/X86/X86CallLowering.cpp:44
+  const X86TargetLowering &TLI = *getTLI<X86TargetLowering>();
+  LLVMContext &Context = OrigArg.Ty->getContext();
zvi wrote:
> Isn't there already a class member that can be casted to X86TargetLowering?
This is getter for target specific TargetLowering class.

Comment at: lib/Target/X86/X86CallLowering.cpp:195
+  MachineBasicBlock &MBB = MIRBuilder.getMBB();
+  if (!MBB.empty())
+     MIRBuilder.setInstr(*MBB.begin());
zvi wrote:
> Is there a test-case for when the condition is false?
In case of empty function  ( ret instruction only)

Comment at: lib/Target/X86/X86CallLowering.cpp:38
+void X86CallLowering::splitToValueTypes(const ArgInfo &OrigArg,
+                                        SmallVectorImpl<ArgInfo> &SplitArgs,
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?



More information about the llvm-commits mailing list