[llvm] r228321 - X86 ABI fix for return values > 24 bytes.

Andrew Trick atrick at apple.com
Thu Feb 5 11:20:29 PST 2015


On Feb 5, 2015, at 11:08 AM, Reid Kleckner <rnk at google.com> wrote:
> 
> Is it possible for Clang generated IR to hit this? I would prefer if Clang avoided relying on CanLowerReturn. So far as I can tell, it is designed to make large struct return work without attempting to respect the local ABI.

Not as far as I know. Clang adjusts argument and return types to match the target ABI, including adding an explicit “sret” argument. I’m not really the expert here though.

This is a case that the backend was clearly trying to handle but was incomplete on X86 and not well tested because clang doesn’t run into it.

Andy

> 
> On Thu, Feb 5, 2015 at 10:09 AM, Andrew Trick <atrick at apple.com <mailto:atrick at apple.com>> wrote:
> Author: atrick
> Date: Thu Feb  5 12:09:05 2015
> New Revision: 228321
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=228321&view=rev <http://llvm.org/viewvc/llvm-project?rev=228321&view=rev>
> Log:
> X86 ABI fix for return values > 24 bytes.
> 
> The return value's address must be returned in %rax.
> i.e. the callee needs to copy the sret argument (%rdi)
> into the return value (%rax).
> 
> This probably won't manifest as a bug when the caller is LLVM-compiled
> code. But it is an ABI guarantee and tools expect it.
> 
> Added:
>     llvm/trunk/test/CodeGen/X86/sret-implicit.ll
> Modified:
>     llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>     llvm/trunk/test/CodeGen/X86/vselect.ll
> 
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=228321&r1=228320&r2=228321&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=228321&r1=228320&r2=228321&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Thu Feb  5 12:09:05 2015
> @@ -2107,14 +2107,15 @@ X86TargetLowering::LowerReturn(SDValue C
>    // Win32 requires us to put the sret argument to %eax as well.
>    // We saved the argument into a virtual register in the entry block,
>    // so now we copy the value out and into %rax/%eax.
> -  if (DAG.getMachineFunction().getFunction()->hasStructRetAttr() &&
> -      (Subtarget->is64Bit() || Subtarget->isTargetKnownWindowsMSVC())) {
> -    MachineFunction &MF = DAG.getMachineFunction();
> -    X86MachineFunctionInfo *FuncInfo = MF.getInfo<X86MachineFunctionInfo>();
> -    unsigned Reg = FuncInfo->getSRetReturnReg();
> -    assert(Reg &&
> -           "SRetReturnReg should have been set in LowerFormalArguments().");
> -    SDValue Val = DAG.getCopyFromReg(Chain, dl, Reg, getPointerTy());
> +  //
> +  // Checking Function.hasStructRetAttr() here is insufficient because the IR
> +  // may not have an explicit sret argument. If FuncInfo.CanLowerReturn is
> +  // false, then an sret argument may be implicitly inserted in the SelDAG. In
> +  // either case FuncInfo->setSRetReturnReg() will have been called.
> +  if (unsigned SRetReg = FuncInfo->getSRetReturnReg()) {
> +    assert((Subtarget->is64Bit() || Subtarget->isTargetKnownWindowsMSVC()) &&
> +           "No need for an sret register");
> +    SDValue Val = DAG.getCopyFromReg(Chain, dl, SRetReg, getPointerTy());
> 
>      unsigned RetValReg
>          = (Subtarget->is64Bit() && !Subtarget->isTarget64BitILP32()) ?
> 
> Added: llvm/trunk/test/CodeGen/X86/sret-implicit.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sret-implicit.ll?rev=228321&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sret-implicit.ll?rev=228321&view=auto>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/sret-implicit.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/sret-implicit.ll Thu Feb  5 12:09:05 2015
> @@ -0,0 +1,10 @@
> +; RUN: llc -mtriple=x86_64-apple-darwin8 < %s | FileCheck %s
> +; RUN: llc -mtriple=x86_64-pc-linux < %s | FileCheck %s
> +
> +; CHECK-LABEL: return32
> +; CHECK-DAG: movq      $0, (%rdi)
> +; CHECK-DAG: movq      %rdi, %rax
> +; CHECK: retq
> +define i256 @return32() {
> +  ret i256 0
> +}
> 
> Modified: llvm/trunk/test/CodeGen/X86/vselect.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/vselect.ll?rev=228321&r1=228320&r2=228321&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/vselect.ll?rev=228321&r1=228320&r2=228321&view=diff>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/vselect.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/vselect.ll Thu Feb  5 12:09:05 2015
> @@ -275,6 +275,7 @@ define <16 x double> @select_illegal(<16
>  ; CHECK-NEXT:    movaps %xmm2, 32(%rdi)
>  ; CHECK-NEXT:    movaps %xmm1, 16(%rdi)
>  ; CHECK-NEXT:    movaps %xmm0, (%rdi)
> +; CHECK-NEXT:    movq %rdi, %rax
>  ; CHECK-NEXT:    retq
>    %sel = select <16 x i1> <i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 false, i1 false, i1 false, i1 false, i1 false, i1 false, i1 false, i1 false>, <16 x double> %a, <16 x double> %b
>    ret <16 x double> %sel
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150205/ead2f7aa/attachment.html>


More information about the llvm-commits mailing list