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

[rnk]

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.

[majnemer]

LGTM

lib/Target/X86/X86ISelLowering.cpp

if (Subtarget->is64Bit()) {
       }
     }
 
     Chain = DAG.getCopyToReg(Chain, dl, VA.getLocReg(), ValToCopy, Flag);
     Flag = Chain.getValue(1);
     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.
   //
   // 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()) ?
           X86::RAX : X86::EAX;
     Chain = DAG.getCopyToReg(Chain, dl, RetValReg, Val, Flag);
     Flag = Chain.getValue(1);
 
for (unsigned i = 0, e = ArgLocs.size(); i != e; ++i) {
     // If value is passed via pointer - do a load.
     if (VA.getLocInfo() == CCValAssign::Indirect)
       ArgValue = DAG.getLoad(VA.getValVT(), dl, Chain, ArgValue,
                              MachinePointerInfo(), false, false, false, 0);
 
     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;
-      }
-    }
-  }
+    // 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.
 
   unsigned StackSize = CCInfo.getNextStackOffset();
   // Align stack specially for tail calls.
   if (FuncIsMadeTailCallSafe(CallConv,
                              MF.getTarget().Options.GuaranteedTailCallOpt))
     StackSize = GetAlignedArgumentStackSize(StackSize, DAG);
 
   // If the function takes variable number of arguments, make a frame index for

test/CodeGen/X86/sret-implicit.ll

-; 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