[llvm] r237175 - [X86] Always return the sret parameter in eax/rax, even on 32-bit

Quentin Colombet qcolombet at apple.com
Tue May 12 14:56:53 PDT 2015


Hi Reid,

Looks like this commit broke at least a bot:
http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/6959/consoleFull#10105486868254eaf0-7326-4999-85b0-388101f2d404 <http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/6959/consoleFull#10105486868254eaf0-7326-4999-85b0-388101f2d404>

Could you investigate please?

Thanks,
-Quentin
> On May 12, 2015, at 1:56 PM, Reid Kleckner <reid at kleckner.net> wrote:
> 
> Author: rnk
> Date: Tue May 12 15:56:32 2015
> New Revision: 237175
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=237175&view=rev
> Log:
> [X86] Always return the sret parameter in eax/rax, even on 32-bit
> 
> Summary:
> This rule was always in the old SysV i386 ABI docs and the new ones that
> H.J. Lu has put together, but we never noticed:
> 
>  EAX   scratch register; also used to return integer and pointer values
>        from functions; also stores the address of a returned struct or union
> 
> Fixes PR23491.
> 
> Reviewers: majnemer
> 
> Subscribers: llvm-commits
> 
> Differential Revision: http://reviews.llvm.org/D9715
> 
> Modified:
>    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>    llvm/trunk/test/CodeGen/X86/cmovcmov.ll
>    llvm/trunk/test/CodeGen/X86/sret-implicit.ll
> 
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=237175&r1=237174&r2=237175&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Tue May 12 15:56:32 2015
> @@ -2000,9 +2000,8 @@ X86TargetLowering::LowerReturn(SDValue C
>     RetOps.push_back(DAG.getRegister(VA.getLocReg(), VA.getLocVT()));
>   }
> 
> -  // The x86-64 ABIs require that for returning structs by value we copy
> +  // All x86 ABIs require that for returning structs by value we copy
>   // the sret argument into %rax/%eax (depending on ABI) for the return.
> -  // 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.
>   //
> @@ -2011,8 +2010,6 @@ X86TargetLowering::LowerReturn(SDValue C
>   // 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
> @@ -2434,24 +2431,21 @@ X86TargetLowering::LowerFormalArguments(
>     InVals.push_back(ArgValue);
>   }
> 
> -  if (Subtarget->is64Bit() || Subtarget->isTargetKnownWindowsMSVC()) {
> -    for (unsigned i = 0, e = ArgLocs.size(); i != e; ++i) {
> -      // The x86-64 ABIs require that for returning structs by value we copy
> -      // the sret argument into %rax/%eax (depending on ABI) for the return.
> -      // Win32 requires us to put the sret argument to %eax as well.
> -      // Save the argument into a virtual register so that we can access it
> -      // from the return points.
> -      if (Ins[i].Flags.isSRet()) {
> -        unsigned Reg = FuncInfo->getSRetReturnReg();
> -        if (!Reg) {
> -          MVT PtrTy = getPointerTy();
> -          Reg = MF.getRegInfo().createVirtualRegister(getRegClassFor(PtrTy));
> -          FuncInfo->setSRetReturnReg(Reg);
> -        }
> -        SDValue Copy = DAG.getCopyToReg(DAG.getEntryNode(), dl, Reg, InVals[i]);
> -        Chain = DAG.getNode(ISD::TokenFactor, dl, MVT::Other, Copy, Chain);
> -        break;
> +  for (unsigned i = 0, e = ArgLocs.size(); i != e; ++i) {
> +    // All x86 ABIs require that for returning structs by value we copy the
> +    // sret argument into %rax/%eax (depending on ABI) for the return. Save
> +    // the argument into a virtual register so that we can access it from the
> +    // return points.
> +    if (Ins[i].Flags.isSRet()) {
> +      unsigned Reg = FuncInfo->getSRetReturnReg();
> +      if (!Reg) {
> +        MVT PtrTy = getPointerTy();
> +        Reg = MF.getRegInfo().createVirtualRegister(getRegClassFor(PtrTy));
> +        FuncInfo->setSRetReturnReg(Reg);
>       }
> +      SDValue Copy = DAG.getCopyToReg(DAG.getEntryNode(), dl, Reg, InVals[i]);
> +      Chain = DAG.getNode(ISD::TokenFactor, dl, MVT::Other, Copy, Chain);
> +      break;
>     }
>   }
> 
> 
> Modified: llvm/trunk/test/CodeGen/X86/cmovcmov.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/cmovcmov.ll?rev=237175&r1=237174&r2=237175&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/cmovcmov.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/cmovcmov.ll Tue May 12 15:56:32 2015
> @@ -143,19 +143,19 @@ entry:
> ; NOCMOV-NEXT:   jp  [[TBB]]
> ; NOCMOV-NEXT:   leal  24(%esp), %eax
> ; NOCMOV-NEXT: [[TBB]]:
> -; NOCMOV-NEXT:   movl  (%eax), %eax
> -; NOCMOV-NEXT:   leal  44(%esp), %ecx
> +; NOCMOV-NEXT:   movl  (%eax), %ecx
> +; NOCMOV-NEXT:   leal  44(%esp), %edx
> ; NOCMOV-NEXT:   jne  [[TBB:.LBB[0-9_]+]]
> ; NOCMOV-NEXT:   jp  [[TBB]]
> -; NOCMOV-NEXT:   leal  28(%esp), %ecx
> +; NOCMOV-NEXT:   leal  28(%esp), %edx
> ; NOCMOV-NEXT: [[TBB]]:
> -; NOCMOV-NEXT:   movl  (%ecx), %ecx
> +; NOCMOV-NEXT:   movl  12(%esp), %eax
> +; NOCMOV-NEXT:   movl  (%edx), %edx
> ; NOCMOV-NEXT:   leal  48(%esp), %esi
> ; NOCMOV-NEXT:   jne  [[TBB:.LBB[0-9_]+]]
> ; NOCMOV-NEXT:   jp  [[TBB]]
> ; NOCMOV-NEXT:   leal  32(%esp), %esi
> ; NOCMOV-NEXT: [[TBB]]:
> -; NOCMOV-NEXT:   movl  12(%esp), %edx
> ; NOCMOV-NEXT:   movl  (%esi), %esi
> ; NOCMOV-NEXT:   leal  52(%esp), %edi
> ; NOCMOV-NEXT:   jne  [[TBB:.LBB[0-9_]+]]
> @@ -163,10 +163,10 @@ entry:
> ; NOCMOV-NEXT:   leal  36(%esp), %edi
> ; NOCMOV-NEXT: [[TBB]]:
> ; NOCMOV-NEXT:   movl  (%edi), %edi
> -; NOCMOV-NEXT:   movl  %edi, 12(%edx)
> -; NOCMOV-NEXT:   movl  %esi, 8(%edx)
> -; NOCMOV-NEXT:   movl  %ecx, 4(%edx)
> -; NOCMOV-NEXT:   movl  %eax, (%edx)
> +; NOCMOV-NEXT:   movl  %edi, 12(%eax)
> +; NOCMOV-NEXT:   movl  %esi, 8(%eax)
> +; NOCMOV-NEXT:   movl  %edx, 4(%eax)
> +; NOCMOV-NEXT:   movl  %ecx, (%eax)
> ; NOCMOV-NEXT:   popl  %esi
> ; NOCMOV-NEXT:   popl  %edi
> ; NOCMOV-NEXT:   retl  $4
> 
> Modified: llvm/trunk/test/CodeGen/X86/sret-implicit.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sret-implicit.ll?rev=237175&r1=237174&r2=237175&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/sret-implicit.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/sret-implicit.ll Tue May 12 15:56:32 2015
> @@ -1,12 +1,34 @@
> -; RUN: llc -mtriple=x86_64-apple-darwin8 < %s | FileCheck %s
> -; RUN: llc -mtriple=x86_64-pc-linux < %s | FileCheck %s
> -; RUN: llc -mtriple=x86_64-apple-darwin8 -terminal-rule < %s | FileCheck %s
> -; RUN: llc -mtriple=x86_64-pc-linux -terminal-rule < %s | FileCheck %s
> -
> -; CHECK-LABEL: return32
> -; CHECK-DAG: movq	$0, (%rdi)
> -; CHECK-DAG: movq	%rdi, %rax
> -; CHECK: retq
> -define i256 @return32() {
> +; RUN: llc -mtriple=x86_64-apple-darwin8 < %s | FileCheck %s --check-prefix=X64
> +; RUN: llc -mtriple=x86_64-pc-linux < %s | FileCheck %s --check-prefix=X64
> +; RUN: llc -mtriple=i686-pc-linux < %s | FileCheck %s --check-prefix=X86
> +; RUN: llc -mtriple=x86_64-apple-darwin8 -terminal-rule < %s | FileCheck %s --check-prefix=X64
> +; RUN: llc -mtriple=x86_64-pc-linux -terminal-rule < %s | FileCheck %s --check-prefix=X64
> +
> +define void @sret_void(i32* sret %p) {
> +  store i32 0, i32* %p
> +  ret void
> +}
> +
> +; X64-LABEL: sret_void
> +; X64-DAG: movl $0, (%rdi)
> +; X64-DAG: movq %rdi, %rax
> +; X64: retq
> +
> +; X86-LABEL: sret_void
> +; X86: movl 4(%esp), %eax
> +; X86: movl $0, (%eax)
> +; X86: retl
> +
> +define i256 @sret_demoted() {
>   ret i256 0
> }
> +
> +; X64-LABEL: sret_demoted
> +; X64-DAG: movq $0, (%rdi)
> +; X64-DAG: movq %rdi, %rax
> +; X64: retq
> +
> +; X86-LABEL: sret_demoted
> +; X86: movl 4(%esp), %eax
> +; X86: movl $0, (%eax)
> +; X86: retl
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> 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/20150512/6bea62e9/attachment.html>


More information about the llvm-commits mailing list