Fix for issue 1995.

This changes containg a fix for Issue 1995.
When remorphing an argument that has been already completed (in case of
CSE rewriting for example, as in this case) the classification code was
not run properly and it is triggering an assertion. As the Issue 1995
states the assert failure is benign since the generated code is correct -
the change of registers used during completed argument classification is
not allowed and the code is not changing them.
This changes add the necessary code to properly classify and assign
registes for completed arguments remorphing, which complies with the expectations of the failing assert.

diff --git a/src/jit/compiler.h b/src/jit/compiler.h
index 13c64b7..252a87c 100644 (file)
--- a/src/jit/compiler.h
+++ b/src/jit/compiler.h
@@ -1189,7 +1189,7 @@ struct fgArgTabEntry
     fgArgTabEntry()
     {
         otherRegNum                     = REG_NA;
-        isStruct                        = false;  // is this a struct arg
+        isStruct                        = false;            // is this a struct arg
     }
 #endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
 
@@ -1220,9 +1220,8 @@ struct fgArgTabEntry
     bool           isNonStandard:1; // True if it is an arg that is passed in a reg other than a standard arg reg
 
 #if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
-    regNumber             otherRegNum;              // The (second) register to use when passing this argument.
-    bool                  isStruct;                 // is this a struct arg
-
+    regNumber             otherRegNum;  // The (second) register to use when passing this argument.
+    bool                  isStruct;     // is this a struct arg.
     SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc;
 #endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
 

diff --git a/src/jit/lowerxarch.cpp b/src/jit/lowerxarch.cpp
index c9df42e..4c1dc1a 100644 (file)
--- a/src/jit/lowerxarch.cpp
+++ b/src/jit/lowerxarch.cpp
@@ -932,7 +932,7 @@ void Lowering::TreeNodeInfoInit(GenTree* stmt)
                     argNode = argNode->gtEffectiveVal();
                 }
 
-                // If the struct arg is wraped in CPYBLK the type of the param will beTYP_VOID.
+                // If the struct arg is wraped in CPYBLK the type of the param will be TYP_VOID.
                 // Use the curArgTabEntry's isStruct to get whether the param is a struct.
                 if (argNode->TypeGet() == TYP_STRUCT 
                     FEATURE_UNIX_AMD64_STRUCT_PASSING_ONLY(|| curArgTabEntry->isStruct))

diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp
index 053a58b..35525f7 100644 (file)
--- a/src/jit/morph.cpp
+++ b/src/jit/morph.cpp
@@ -2693,9 +2693,16 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
                    (call->gtCallObjp->gtType == TYP_I_IMPL));
 
             /* this is a register argument - put it in the table */
-            call->fgArgInfo->AddRegArg(argIndex, argx, NULL, genMapIntRegArgNumToRegNum(intArgRegNum), 1, 1
+            call->fgArgInfo->AddRegArg(argIndex,
+                                       argx, 
+                                       NULL, 
+                                       genMapIntRegArgNumToRegNum(intArgRegNum),
+                                       1, 
+                                       1
 #ifdef FEATURE_UNIX_AMD64_STRUCT_PASSING
-                , false, REG_STK, nullptr
+                                       , false,
+                                       REG_STK,
+                                       nullptr
 #endif // FEATURE_UNIX_AMD64_STRUCT_PASSING
                 );
         }
@@ -2956,6 +2963,23 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
 
         if (lateArgsComputed)
         {
+#if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
+            // Get the struct description for the already completed struct argument.
+            fgArgTabEntryPtr fgEntryPtr = gtArgEntryByNode(call, argx);
+            assert(fgEntryPtr != nullptr);
+
+            // As described in few other places, this can happen when the argx was morphed 
+            // into an arg setup node - COPYBLK. The COPYBLK has always a type of void.
+            // In such case the fgArgTabEntry keeps track of whether the original node (before morphing)
+            // was a struct and the struct classification. 
+            isStructArg = fgEntryPtr->isStruct;
+
+            if (isStructArg)
+            {
+                structDesc.CopyFrom(fgEntryPtr->structDesc);
+            }
+#endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
+
             assert(argEntry != NULL);
             if (argEntry->IsBackFilled())
             {
@@ -3095,7 +3119,13 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
                     unsigned originalSize = info.compCompHnd->getClassSize(ldObjClass);
                     originalSize = (originalSize == 0 ? TARGET_POINTER_SIZE : originalSize);
                     unsigned roundupSize  = (unsigned)roundUp(originalSize, TARGET_POINTER_SIZE);
+#ifdef FEATURE_UNIX_AMD64_STRUCT_PASSING
+                    // On System V OS-es a struct is never passed by reference.
+                    // It is either passed by value on the stack or in registers.
+                    bool     passStructInRegisters = false;
+#else // !FEATURE_UNIX_AMD64_STRUCT_PASSING
                     bool     passStructByRef = false;
+#endif // !FEATURE_UNIX_AMD64_STRUCT_PASSING
 
 #ifndef _TARGET_X86_
 #ifndef FEATURE_UNIX_AMD64_STRUCT_PASSING
@@ -3115,12 +3145,13 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
 #else // FEATURE_UNIX_AMD64_STRUCT_PASSING
                         if (!structDesc.passedInRegisters)
                         {
-                            passStructByRef = false;
+                            passStructInRegisters = false;
                             copyBlkClass = NULL;
                         }
                         else 
                         {
-                            passStructByRef = true;
+                            // The ldObjClass is used to materialize the struct on stack.
+                            passStructInRegisters = true;
                             copyBlkClass = ldObjClass;
                         }
 #endif // FEATURE_UNIX_AMD64_STRUCT_PASSING
@@ -3280,7 +3311,13 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
 
 #endif // not _TARGET_X86_
                     // We still have a TYP_STRUCT unless we converted the GT_LDOBJ into a GT_IND above...
-                    if ((structBaseType == TYP_STRUCT) && !passStructByRef)
+                    if ((structBaseType == TYP_STRUCT) &&
+#if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
+                        !passStructInRegisters
+#else // !defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
+                        !passStructByRef
+#endif // !defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
+                        )
                     {
                         // if the valuetype size is not a multiple of sizeof(void*),
                         // we must copyblk to a temp before doing the ldobj to avoid
@@ -3472,7 +3509,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
 #if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
             regNumber nextOtherRegNum = REG_STK;
 
-            if (isStructArg)
+            if (isStructArg && structDesc.passedInRegisters)
             {
                 // It is a struct passed in registers. Assign the next available register.
                 unsigned int curIntReg = intArgRegNum;
@@ -3485,10 +3522,22 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
                         if (i == 0)
                         {
                             nextRegNum = genMapIntRegArgNumToRegNum(curIntReg);
+
+                            // For non-completed args the counters are incremented already
+                            // in the !lateArgsComputed above.
+                            if (lateArgsComputed)
+                            {
+                                structIntRegs++;
+                            }
                         }
                         else if (i == 1)
                         {
                             nextOtherRegNum = genMapIntRegArgNumToRegNum(curIntReg);
+
+                            if (lateArgsComputed)
+                            {
+                                structIntRegs++;
+                            }
                         }
                         else
                         {
@@ -3502,10 +3551,20 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
                         if (i == 0)
                         {
                             nextRegNum = genMapFloatRegArgNumToRegNum(curFloatReg);
+
+                            if (lateArgsComputed)
+                            {
+                                structFloatRegs++;
+                            }
                         }
                         else if (i == 1)
                         {
                             nextOtherRegNum = genMapFloatRegArgNumToRegNum(curFloatReg);
+
+                            if (lateArgsComputed)
+                            {
+                                structFloatRegs++;
+                            }
                         }
                         else
                         {
@@ -3541,10 +3600,16 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
                 hasNonStandardArg = true;
                 if (argx == nonStandardArgs.Index(i).node)
                 {
-                    fgArgTabEntry* argEntry = call->fgArgInfo->AddRegArg(argIndex, argx,
-                        args, nonStandardArgs.Index(i).reg, size, argAlign
+                    fgArgTabEntry* argEntry = call->fgArgInfo->AddRegArg(argIndex, 
+                                                                         argx,
+                                                                         args, 
+                                                                         nonStandardArgs.Index(i).reg, 
+                                                                         size, 
+                                                                         argAlign
 #if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
-                        , isStructArg, nextOtherRegNum, &structDesc
+                                                                         , isStructArg, 
+                                                                         nextOtherRegNum, 
+                                                                         &structDesc
 #endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
                     );
                     argEntry->isNonStandard = true;
@@ -3560,10 +3625,16 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
             if (!lateArgsComputed)
             {
                 // This is a register argument - put it in the table
-                fgArgTabEntryPtr newArg = call->fgArgInfo->AddRegArg(
-                    argIndex, argx, args, nextRegNum, size, argAlign
+                fgArgTabEntryPtr newArg = call->fgArgInfo->AddRegArg(argIndex, 
+                                                                     argx, 
+                                                                     args, 
+                                                                     nextRegNum, 
+                                                                     size, 
+                                                                     argAlign
 #if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
-                    , isStructArg, nextOtherRegNum, &structDesc
+                                                                     , isStructArg, 
+                                                                     nextOtherRegNum, 
+                                                                     &structDesc
 #endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
                     );
                 (void)newArg; //prevent "unused variable" error from GCC
@@ -3598,6 +3669,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
                     if (passUsingFloatRegs)
                     {
                         fltArgRegNum += size;
+
 #if defined(_TARGET_AMD64_) && !defined(UNIX_AMD64_ABI)
                         argSkippedRegMask |= genMapArgNumToRegMask(intArgRegNum, TYP_I_IMPL);
                         intArgRegNum = min(intArgRegNum + size, MAX_REG_ARG);
@@ -3617,6 +3689,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
                     else
                     {
                         intArgRegNum += size;
+
 #if defined(_TARGET_AMD64_) && !defined(UNIX_AMD64_ABI)
                         fltArgSkippedRegMask |= genMapArgNumToRegMask(fltArgRegNum, TYP_DOUBLE);
                         fltArgRegNum = min(fltArgRegNum + size, MAX_FLOAT_REG_ARG);