Explicitly change the type of a method with implicit RetBuf to BYREF for

System V.

The System V ABI requires the address implicit RetBuf to be returned in
RAX.
Instead of moving the address of the buffer to RAX in codegenxarch.cpp,
change such methods return type to TYP_BYREF and generate the necessary
code to put the address in RAX.

Added/cleaned some comments.

Fixes issue dotnet/coreclr#3299.

Commit migrated from https://github.com/dotnet/coreclr/commit/9f6baf04de71fd6fb795786181dd872c2e5edbb8

diff --git a/src/coreclr/src/jit/codegenxarch.cpp b/src/coreclr/src/jit/codegenxarch.cpp
index 3025675..1dec8ba 100644 (file)
--- a/src/coreclr/src/jit/codegenxarch.cpp
+++ b/src/coreclr/src/jit/codegenxarch.cpp
@@ -1419,8 +1419,7 @@ void CodeGen::genCodeForBinary(GenTree* treeNode)
 //    treeNode - The tree node to evaluate whether is a struct return.
 //
 // Return Value:
-//    For AMD64 *nix: returns true if the 'treeNode" is of type GT_RETURN and the
-//                    return type is a struct or it is an implicit retBuf struct return.
+//    For AMD64 *nix: returns true if the 'treeNode" is a GT_RETURN node, of type struct.
 //                    Otherwise returns false.
 //    For other platforms always returns false.
 //
@@ -1437,8 +1436,7 @@ CodeGen::isStructReturn(GenTreePtr treeNode)
     }
 
 #ifdef FEATURE_UNIX_AMD64_STRUCT_PASSING
-    return varTypeIsStruct(treeNode) ||
-           (treeNode->TypeGet() == TYP_VOID && compiler->info.compRetBuffArg != BAD_VAR_NUM);
+    return varTypeIsStruct(treeNode);
 #else // !FEATURE_UNIX_AMD64_STRUCT_PASSING
     assert(!varTypeIsStruct(treeNode));
     return false;
@@ -1462,63 +1460,50 @@ CodeGen::genStructReturn(GenTreePtr treeNode)
     var_types targetType = treeNode->TypeGet();
 
 #ifdef FEATURE_UNIX_AMD64_STRUCT_PASSING
-    if (targetType == TYP_VOID)
-    {
-        assert(op1 == nullptr);
-        if (compiler->info.compRetBuffArg != BAD_VAR_NUM)
-        {
-            // System V AMD64 spec requires that when a struct is returned by a hidden
-            // argument the RAX should contain the value of the hidden retbuf arg.
-            getEmitter()->emitIns_R_S(INS_mov, EA_BYREF, REG_RAX, compiler->info.compRetBuffArg, 0);
-        }        
-    }
-    else
-    {
-        noway_assert((op1->OperGet() == GT_LCL_VAR) ||
-                     (op1->OperGet() == GT_CALL));
+    noway_assert((op1->OperGet() == GT_LCL_VAR) ||
+                 (op1->OperGet() == GT_CALL));
 
-        if (op1->OperGet() == GT_LCL_VAR)
-        {
-            assert(op1->isContained());
+    if (op1->OperGet() == GT_LCL_VAR)
+    {
+        assert(op1->isContained());
 
-            GenTreeLclVarCommon* lclVarPtr = op1->AsLclVarCommon();
-            LclVarDsc* varDsc = &(compiler->lvaTable[lclVarPtr->gtLclNum]);
-            assert(varDsc->lvIsMultiRegArgOrRet);
+        GenTreeLclVarCommon* lclVarPtr = op1->AsLclVarCommon();
+        LclVarDsc* varDsc = &(compiler->lvaTable[lclVarPtr->gtLclNum]);
+        assert(varDsc->lvIsMultiRegArgOrRet);
 
-            CORINFO_CLASS_HANDLE typeHnd = varDsc->lvVerTypeInfo.GetClassHandle();
-            assert(typeHnd != nullptr);
-
-            SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc;
-            compiler->eeGetSystemVAmd64PassStructInRegisterDescriptor(typeHnd, &structDesc);
-            assert(structDesc.passedInRegisters);
-            assert(structDesc.eightByteCount == CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS);
+        CORINFO_CLASS_HANDLE typeHnd = varDsc->lvVerTypeInfo.GetClassHandle();
+        assert(typeHnd != nullptr);
 
-            regNumber retReg0 = REG_NA;
-            unsigned __int8 offset0 = 0;
-            regNumber retReg1 = REG_NA;
-            unsigned __int8 offset1 = 0;
+        SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc;
+        compiler->eeGetSystemVAmd64PassStructInRegisterDescriptor(typeHnd, &structDesc);
+        assert(structDesc.passedInRegisters);
+        assert(structDesc.eightByteCount == CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS);
 
-            var_types type0 = TYP_UNKNOWN;
-            var_types type1 = TYP_UNKNOWN;
+        regNumber retReg0 = REG_NA;
+        unsigned __int8 offset0 = 0;
+        regNumber retReg1 = REG_NA;
+        unsigned __int8 offset1 = 0;
 
-            getStructTypeOffset(structDesc, &type0, &type1, &offset0, &offset1);
-            getStructReturnRegisters(type0, type1, &retReg0, &retReg1);
+        var_types type0 = TYP_UNKNOWN;
+        var_types type1 = TYP_UNKNOWN;
 
-            // Move the values into the return registers.
-            // 
+        getStructTypeOffset(structDesc, &type0, &type1, &offset0, &offset1);
+        getStructReturnRegisters(type0, type1, &retReg0, &retReg1);
 
-            assert(retReg0 != REG_NA && retReg1 != REG_NA);
+        // Move the values into the return registers.
+        // 
 
-            getEmitter()->emitIns_R_S(ins_Load(type0), emitTypeSize(type0), retReg0, lclVarPtr->gtLclNum, offset0);
-            getEmitter()->emitIns_R_S(ins_Load(type1), emitTypeSize(type1), retReg1, lclVarPtr->gtLclNum, offset1);
-        }
+        assert(retReg0 != REG_NA && retReg1 != REG_NA);
 
-        // Nothing to do if the op1 of the return statement is a GT_CALL. The call already has the return
-        // registers in the proper return registers. 
-        // This assumes that registers never get spilled. There is an Issue 2966 created to track the need 
-        // for handling the GT_CALL case of two register returns and handle it properly for stress modes 
-        // and potential other changes that may break this assumption.
+        getEmitter()->emitIns_R_S(ins_Load(type0), emitTypeSize(type0), retReg0, lclVarPtr->gtLclNum, offset0);
+        getEmitter()->emitIns_R_S(ins_Load(type1), emitTypeSize(type1), retReg1, lclVarPtr->gtLclNum, offset1);
     }
+
+    // Nothing to do if the op1 of the return statement is a GT_CALL. The call already has the return
+    // registers in the proper return registers. 
+    // This assumes that registers never get spilled. There is an Issue 2966 created to track the need 
+    // for handling the GT_CALL case of two register returns and handle it properly for stress modes 
+    // and potential other changes that may break this assumption.
 #else
     assert("!unreached");
 #endif   
@@ -2654,6 +2639,12 @@ CodeGen::genStoreRegisterReturnInLclVar(GenTreePtr treeNode)
 
         assert(retReg0 != REG_NA && retReg1 != REG_NA);
 
+        // Nothing to do if the op1 of the return statement is a GT_CALL. The call already has the return
+        // registers in the proper return registers.
+        // This assumes that registers never get spilled. There is an Issue 2966 created to track the need   
+        // for handling the GT_CALL case of two register returns and handle it properly for stress modes   
+        // and potential other changes that may break this assumption.  
+
         getEmitter()->emitIns_S_R(ins_Store(type0), emitTypeSize(type0), retReg0, lclVarPtr->gtLclNum, offset0);
         getEmitter()->emitIns_S_R(ins_Store(type1), emitTypeSize(type1), retReg1, lclVarPtr->gtLclNum, offset1);
 

diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h
index 6befe44..511fc45 100644 (file)
--- a/src/coreclr/src/jit/compiler.h
+++ b/src/coreclr/src/jit/compiler.h
@@ -7785,17 +7785,22 @@ public :
     // Returns true if the method being compiled returns RetBuf addr as its return value
     bool                compMethodReturnsRetBufAddr() 
     {
-        // Profiler Leave calllback expects the address of retbuf as return value for 
-        // methods with hidden RetBuf argument.  impReturnInstruction() when profiler
-        // callbacks are needed creates GT_RETURN(TYP_BYREF, op1 = Addr of RetBuf) for
-        // methods with hidden RetBufArg.
-        //
-        // TODO-AMD64-Unix - As per this ABI, addr of RetBuf needs to be returned by
-        // methods with hidden RetBufArg.  Right now we are special casing GT_RETURN
-        // of TYP_VOID in codegenxarch.cpp to generate "mov rax, addr of RetBuf".
-        // Instead we should consider explicitly materializing GT_RETURN of TYP_BYREF
-        // return addr of RetBuf in IR.
-        return compIsProfilerHookNeeded() && (info.compRetBuffArg != BAD_VAR_NUM);
+        // There are cases where implicit RetBuf argument should be explicitly returned in a register.  
+        // In such cases the return type is changed to TYP_BYREF and appropriate IR is generated.  
+        // These cases are:  
+        // 1. Profiler Leave calllback expects the address of retbuf as return value for   
+        //    methods with hidden RetBuf argument.  impReturnInstruction() when profiler  
+        //    callbacks are needed creates GT_RETURN(TYP_BYREF, op1 = Addr of RetBuf) for  
+        //    methods with hidden RetBufArg.  
+        //  
+        // 2. As per the System V ABI, the address of RetBuf needs to be returned by  
+        //    methods with hidden RetBufArg in RAX. In such case GT_RETURN is of TYP_BYREF,  
+        //    returning the address of RetBuf.  
+#ifdef FEATURE_UNIX_AMD64_STRUCT_PASSING
+        return (info.compRetBuffArg != BAD_VAR_NUM);
+#else // FEATURE_UNIX_AMD64_STRUCT_PASSING  
+        return (compIsProfilerHookNeeded()) && (info.compRetBuffArg != BAD_VAR_NUM);
+#endif // !FEATURE_UNIX_AMD64_STRUCT_PASSING  
     }
 
     // Returns true if the method returns a value in more than one return register

diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp
index 270aeb9..c4f2bac 100644 (file)
--- a/src/coreclr/src/jit/importer.cpp
+++ b/src/coreclr/src/jit/importer.cpp
@@ -13907,16 +13907,26 @@ bool Compiler::impReturnInstruction(BasicBlock *block, int prefixFlags, OPCODE &
 
         op2 = impAssignStructPtr(retBuffAddr, op2, retClsHnd, (unsigned)CHECK_SPILL_ALL);
         impAppendTree(op2, (unsigned)CHECK_SPILL_NONE, impCurStmtOffs);
+
+        // There are cases where the address of the implicit RetBuf should be returned explicitly (in RAX).  
+#if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
+        // System V ABI requires to return the implicit return buffer explicitly (in RAX).
+        // Change the return type to be BYREF.  
+        op1 = gtNewOperNode(GT_RETURN, TYP_BYREF, gtNewLclvNode(info.compRetBuffArg, TYP_BYREF));
+#else // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
+        // In case of Windows AMD64 the profiler hook requires to return the implicit RetBuf explicitly (in RAX).  
+        // In such case the return value of the function is changed to BYREF.  
+        // If profiler hook is not needed the return type of the function is TYP_VOID.  
         if (compIsProfilerHookNeeded())
         {
-            // The profiler callback expects the address of the return buffer in eax
             op1 = gtNewOperNode(GT_RETURN, TYP_BYREF, gtNewLclvNode(info.compRetBuffArg, TYP_BYREF));
         }
         else
         {
-            // return void
+            // return void  
             op1 = new (this, GT_RETURN) GenTreeOp(GT_RETURN, TYP_VOID);
         }
+#endif // !defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)  
     }
     else if (varTypeIsStruct(info.compRetType))
     {