Fixes for two JIT helpers that should not use the x8 RetBuf argument on ARM64
authorBrian Sullivan <briansul@microsoft.com>
Mon, 13 Jun 2016 21:06:14 +0000 (14:06 -0700)
committerBrian Sullivan <briansul@microsoft.com>
Wed, 15 Jun 2016 23:03:37 +0000 (16:03 -0700)
Fixes these two JIT helpers" CORINFO_HELP_GETFIELDSTRUCT and CORINFO_HELP_UNBOX_NULLABLE
We no longer set GTF_CALL_M_RETBUFFARG for the two special Jit helpers
We insert the extra byref argument in the x0 position for these two JIT helpers

Commit migrated from https://github.com/dotnet/coreclr/commit/987536accfc50a689b914f343d79fb3588cc552b

src/coreclr/src/jit/gentree.cpp
src/coreclr/src/jit/gentree.h
src/coreclr/src/jit/importer.cpp

index b600a26..e5c1c66 100644 (file)
@@ -1329,6 +1329,62 @@ GenTreeCall::GetOtherRegMask() const
     return resultMask;
 }
 
+//-------------------------------------------------------------------------
+// TreatAsHasRetBufArg:
+//
+// Arguments:
+//     compiler, the compiler instance so that we can call eeGetHelperNum
+//
+// Return Value:
+//     Returns true if we treat the call as if it has a retBuf argument
+//     This method may actually have a retBuf argument 
+//     or it could be a JIT helper that we are still transforming during 
+//     the importer phase.
+//
+// Notes:
+//     On ARM64 marking the method with the GTF_CALL_M_RETBUFFARG flag
+//     will make HasRetBufArg() return true, but will also force the 
+//     use of register x8 to pass the RetBuf argument.
+//
+//     These two Jit Helpers that we handle here by returning true
+//     aren't actually defined to return a struct, so they don't expect
+//     their RetBuf to be passed in x8, instead they  expect it in x0.
+//
+bool GenTreeCall::TreatAsHasRetBufArg(Compiler* compiler)
+{
+    if (HasRetBufArg())
+    {
+        return true;
+    }
+    else
+    {
+        // If we see a Jit helper call that returns a TYP_STRUCT we will
+        // transform it as if it has a Return Buffer Argument
+        //
+        if (IsHelperCall() && (gtCall.gtReturnType == TYP_STRUCT))
+        {
+            // There are two possible helper calls that use this path:
+            //  CORINFO_HELP_GETFIELDSTRUCT and CORINFO_HELP_UNBOX_NULLABLE
+            //
+            CorInfoHelpFunc helpFunc = compiler->eeGetHelperNum(gtCall.gtCallMethHnd);
+
+            if (helpFunc == CORINFO_HELP_GETFIELDSTRUCT)
+            {
+                return true;
+            }
+            else if (helpFunc == CORINFO_HELP_UNBOX_NULLABLE)
+            {
+                return true;
+            }
+            else
+            {
+                assert(!"Unexpected JIT helper in TreatAsHasRetBufArg");
+            }
+        }
+    }
+    return false;
+}
+
 /*****************************************************************************
  *
  *  Returns non-zero if the two trees are identical.
@@ -11628,10 +11684,6 @@ GenTreePtr          Compiler::gtNewRefCOMfield(GenTreePtr   objPtr,
         {
             if (pFieldInfo->helper == CORINFO_HELP_GETFIELDSTRUCT)
             {
-                // The helper ALWAYS takes a 'return buffer'. (really just the
-                // the first parameter is really an out parameter)
-                tree->gtCall.gtCallMoreFlags |= GTF_CALL_M_RETBUFFARG;
-
                 if (!varTypeIsStruct(lclTyp))
                 {
                     // get the result as primitive type
@@ -11639,8 +11691,7 @@ GenTreePtr          Compiler::gtNewRefCOMfield(GenTreePtr   objPtr,
                     tree = gtNewOperNode(GT_IND, lclTyp, tree);
                 }
             }
-            else
-            if (varTypeIsIntegral(lclTyp) && genTypeSize(lclTyp) < genTypeSize(TYP_INT))
+            else if (varTypeIsIntegral(lclTyp) && genTypeSize(lclTyp) < genTypeSize(TYP_INT))
             {
                 // The helper does not extend the small return types.
                 tree = gtNewCastNode(genActualType(lclTyp), tree, lclTyp);
index 48c77b5..133e504 100644 (file)
@@ -2809,15 +2809,37 @@ struct GenTreeCall final : public GenTree
     }
 #endif // !LEGACY_BACKEND
 
-    // Returns true if the call has retBuf argument
-    bool HasRetBufArg() const { return (gtCallMoreFlags & GTF_CALL_M_RETBUFFARG) != 0; }
+    // Returns true if this call uses a retBuf argument and its calling convention
+    bool HasRetBufArg() const
+    {
+         return (gtCallMoreFlags & GTF_CALL_M_RETBUFFARG) != 0;
+    }
+
+    //-------------------------------------------------------------------------
+    // TreatAsHasRetBufArg:
+    //
+    // Arguments:
+    //     compiler, the compiler instance so that we can call eeGetHelperNum
+    //
+    // Return Value:
+    //     Returns true if we treat the call as if it has a retBuf argument
+    //     This method may actually have a retBuf argument 
+    //     or it could be a JIT helper that we are still transforming during 
+    //     the importer phase.
+    //
+    // Notes:
+    //     On ARM64 marking the method with the GTF_CALL_M_RETBUFFARG flag
+    //     will make HasRetBufArg() return true, but will also force the 
+    //     use of register x8 to pass the RetBuf argument.
+    //
+    bool TreatAsHasRetBufArg(Compiler* compiler);
 
     //-----------------------------------------------------------------------------------------
     // HasMultiRegRetVal: whether the call node returns its value in multiple return registers.
     //
     // Arguments:
     //     None
-
+    //
     // Return Value:
     //     True if the call is returning a multi-reg return value. False otherwise.
     //
index df88a96..9eefa5f 100644 (file)
@@ -1104,7 +1104,7 @@ GenTreePtr Compiler::impAssignStructPtr(GenTreePtr      dest,
 
     if (src->gtOper == GT_CALL)
     {
-        if (src->AsCall()->HasRetBufArg())
+        if (src->AsCall()->TreatAsHasRetBufArg(this))
         {
             // Case of call returning a struct via hidden retbuf arg
 
@@ -7614,7 +7614,7 @@ REDO_RETURN_NODE:
     }
     else if (op->gtOper == GT_CALL)
     {
-        if (op->AsCall()->HasRetBufArg())
+        if (op->AsCall()->TreatAsHasRetBufArg(this))
         {
             // This must be one of those 'special' helpers that don't
             // really have a return buffer, but instead use it as a way
@@ -13081,14 +13081,6 @@ FIELD_DONE:
                     // Don't optimize, just call the helper and be done with it
                 args = gtNewArgList(op2, op1);
                 op1 = gtNewHelperCallNode(helper, (var_types)((helper == CORINFO_HELP_UNBOX)?TYP_BYREF:TYP_STRUCT), callFlags, args);
-
-                if (helper == CORINFO_HELP_UNBOX_NULLABLE)
-                {
-                    // NOTE!  This really isn't a return buffer. On the native side this
-                    // is the first argument, but the trees are prettier this way
-                    // so fake it as a return buffer.
-                    op1->gtCall.gtCallMoreFlags |= GTF_CALL_M_RETBUFFARG;
-                }
             }
 
             assert(helper == CORINFO_HELP_UNBOX          && op1->gtType == TYP_BYREF || // Unbox helper returns a byref.