From 518875661611f6e94499470f5ba325cf37baae52 Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Mon, 7 May 2018 16:27:30 -0700 Subject: [PATCH 1/3] Arm64: Pass promoted struct using GT_FIELD_LIST --- src/jit/compiler.h | 1 + src/jit/morph.cpp | 95 +++++++++++++----- .../JitBlue/GitHub_16377/GitHub_16377.cs | 97 +++++++++++++++++++ .../JitBlue/GitHub_16377/GitHub_16377.csproj | 38 ++++++++ 4 files changed, 207 insertions(+), 24 deletions(-) create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.csproj diff --git a/src/jit/compiler.h b/src/jit/compiler.h index b9b256c60d4c..64f54c0bad16 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -4919,6 +4919,7 @@ class Compiler GenTree* fgMorphArrayIndex(GenTree* tree); GenTree* fgMorphCast(GenTree* tree); GenTree* fgUnwrapProxy(GenTree* objRef); + GenTreeFieldList* fgMorphLclArgToFieldlist(GenTreeLclVarCommon* lcl); GenTreeCall* fgMorphArgs(GenTreeCall* call); void fgMakeOutgoingStructArgCopy(GenTreeCall* call, diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 4514bcd6a6ec..5478efd358e7 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -907,7 +907,20 @@ void fgArgTabEntry::Dump() printf(" %d.%s", node->gtTreeID, GenTree::OpName(node->gtOper)); if (regNum != REG_STK) { - printf(", %s, regs=%u", getRegName(regNum), numRegs); + printf(", %u reg%s:", numRegs, numRegs == 1 ? "" : "s"); + printf(" %s", getRegName(regNum)); +#if defined(UNIX_AMD64_ABI) + if (numRegs > 1) + { + printf(" %s", getRegName(otherRegNum)); + } +#else // !UNIX_AMD64_ABI + // Note that for all other targets, we rely on the fact that arg regs are sequential. + for (unsigned i = 1; i < numRegs; i++) + { + printf(" %s", getRegName((regNumber)(regNum + i))); + } +#endif // !UNIX_AMD64_ABI } if (numSlots > 0) { @@ -4944,6 +4957,9 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry #ifdef _TARGET_ARM_ if ((fgEntryPtr->isSplit && fgEntryPtr->numSlots + fgEntryPtr->numRegs > 4) || (!fgEntryPtr->isSplit && fgEntryPtr->regNum == REG_STK)) +#else + if (fgEntryPtr->regNum == REG_STK) +#endif { GenTreeLclVarCommon* lcl = nullptr; @@ -4962,21 +4978,29 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry // We need to construct a `GT_OBJ` node for the argmuent, // so we need to get the address of the lclVar. lcl = arg->AsLclVarCommon(); - - arg = gtNewOperNode(GT_ADDR, TYP_I_IMPL, arg); - - // Create an Obj of the temp to use it as a call argument. - arg = gtNewObjNode(lvaGetStruct(lcl->gtLclNum), arg); } if (lcl != nullptr) { - // Its fields will need to accessed by address. - lvaSetVarDoNotEnregister(lcl->gtLclNum DEBUG_ARG(DNER_IsStructArg)); + unsigned lclNum = lcl->gtLclNum; + if (lvaGetPromotionType(lclNum) == PROMOTION_TYPE_INDEPENDENT) + { + arg = fgMorphLclArgToFieldlist(lcl); + } + else + { + if (!arg->OperIs(GT_OBJ)) + { + // Create an Obj of the temp to use it as a call argument. + arg = gtNewOperNode(GT_ADDR, TYP_I_IMPL, arg); + arg = gtNewObjNode(lvaGetStruct(lcl->gtLclNum), arg); + } + // Its fields will need to accessed by address. + lvaSetVarDoNotEnregister(lcl->gtLclNum DEBUG_ARG(DNER_IsStructArg)); + } } return arg; } -#endif #if FEATURE_MULTIREG_ARGS // Examine 'arg' and setup argValue objClass and structSize @@ -5270,21 +5294,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry if (!varIsFloat) { - unsigned offset = 0; - GenTreeFieldList* listEntry = nullptr; - // We can use the struct promoted field as arguments - for (unsigned inx = 0; inx < elemCount; inx++) - { - GenTree* lclVar = gtNewLclvNode(varNums[inx], varType[inx], varNums[inx]); - // Create a new tree for 'arg' - // replace the existing LDOBJ(ADDR(LCLVAR)) - listEntry = new (this, GT_FIELD_LIST) GenTreeFieldList(lclVar, offset, varType[inx], listEntry); - if (newArg == nullptr) - { - newArg = listEntry; - } - offset += TARGET_POINTER_SIZE; - } + newArg = fgMorphLclArgToFieldlist(varNode); } } } @@ -5474,6 +5484,43 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry return arg; } +//------------------------------------------------------------------------ +// fgMorphLclArgToFieldlist: Morph a GT_LCL_VAR node to a GT_FIELD_LIST of its promoted fields +// +// Arguments: +// lcl - The GT_LCL_VAR node we will transform +// +// Return value: +// The new GT_FIELD_LIST that we have created. +// +GenTreeFieldList* Compiler::fgMorphLclArgToFieldlist(GenTreeLclVarCommon* lcl) +{ + LclVarDsc* varDsc = &(lvaTable[lcl->gtLclNum]); + assert(varDsc->lvPromoted == true); + + unsigned lclNum = lcl->gtLclNum; + unsigned offset = 0; + unsigned fieldCount = varDsc->lvFieldCnt; + GenTreeFieldList* listEntry = nullptr; + GenTreeFieldList* newArg = nullptr; + unsigned fieldLclNum = varDsc->lvFieldLclStart; + + // We can use the struct promoted field as arguments + for (unsigned i = 0; i < fieldCount; i++) + { + LclVarDsc* fieldVarDsc = &lvaTable[fieldLclNum]; + GenTree* lclVar = gtNewLclvNode(fieldLclNum, fieldVarDsc->lvType); + listEntry = new (this, GT_FIELD_LIST) GenTreeFieldList(lclVar, offset, fieldVarDsc->lvType, listEntry); + if (newArg == nullptr) + { + newArg = listEntry; + } + offset += TARGET_POINTER_SIZE; + fieldLclNum++; + } + return newArg; +} + // Make a copy of a struct variable if necessary, to pass to a callee. // returns: tree that computes address of the outgoing arg void Compiler::fgMakeOutgoingStructArgCopy( diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.cs b/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.cs new file mode 100644 index 000000000000..069eff401337 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.cs @@ -0,0 +1,97 @@ +using System; + +class GitHub_16377 +{ + public static ValueTuple CreateLong(T1 item1, T2 item2, T3 item3, T4 item4, T5 item5, T6 item6, T7 item7, TRest rest) where TRest : struct + { + return new ValueTuple(item1, item2, item3, item4, item5, item6, item7, rest); + } + + public static Tuple CreateLongRef(T1 item1, T2 item2, T3 item3, T4 item4, T5 item5, T6 item6, T7 item7, TRest rest) + { + return new Tuple(item1, item2, item3, item4, item5, item6, item7, rest); + } + + public static void AssertEqual(string s1, string s2) + { + if (!s1.Equals(s2)) + { + throw new Exception(); + } + } + + static void Test() + { + { + var vtWithNull = CreateLong(1, 2, 3, 4, 5, 6, 7, new ValueTuple(null)); + var tupleWithNull = CreateLongRef(1, 2, 3, 4, 5, 6, 7, new Tuple(null)); + AssertEqual("(1, 2, 3, 4, 5, 6, 7, )", vtWithNull.ToString()); + AssertEqual(tupleWithNull.ToString(), vtWithNull.ToString()); + } + + { + var vtWithNull = CreateLong(1, 2, 3, 4, 5, 6, 7, new ValueTuple(null)); + var tupleWithNull = CreateLongRef(1, 2, 3, 4, 5, 6, 7, new Tuple(null)); + AssertEqual("(1, 2, 3, 4, 5, 6, 7, )", vtWithNull.ToString()); + AssertEqual(tupleWithNull.ToString(), vtWithNull.ToString()); + } + + { + var vtWithNull = CreateLong(1, 2, 3, 4, 5, 6, 7, new ValueTuple(null, null)); + var tupleWithNull = CreateLongRef(1, 2, 3, 4, 5, 6, 7, new Tuple(null, null)); + AssertEqual("(1, 2, 3, 4, 5, 6, 7, , )", vtWithNull.ToString()); + AssertEqual(tupleWithNull.ToString(), vtWithNull.ToString()); + } + + { + var vtWithNull = CreateLong(1, 2, 3, 4, 5, 6, 7, new ValueTuple(null, null, null)); + var tupleWithNull = CreateLongRef(1, 2, 3, 4, 5, 6, 7, new Tuple(null, null, null)); + AssertEqual("(1, 2, 3, 4, 5, 6, 7, , , )", vtWithNull.ToString()); + AssertEqual(tupleWithNull.ToString(), vtWithNull.ToString()); + } + + { + var vtWithNull = CreateLong(1, 2, 3, 4, 5, 6, 7, new ValueTuple(null, null, null, null)); + var tupleWithNull = CreateLongRef(1, 2, 3, 4, 5, 6, 7, new Tuple(null, null, null, null)); + AssertEqual("(1, 2, 3, 4, 5, 6, 7, , , , )", vtWithNull.ToString()); + AssertEqual(tupleWithNull.ToString(), vtWithNull.ToString()); + } + + { + var vtWithNull = CreateLong(1, 2, 3, 4, 5, 6, 7, new ValueTuple(null, null, null, null, null)); + var tupleWithNull = CreateLongRef(1, 2, 3, 4, 5, 6, 7, new Tuple(null, null, null, null, null)); + AssertEqual("(1, 2, 3, 4, 5, 6, 7, , , , , )", vtWithNull.ToString()); + AssertEqual(tupleWithNull.ToString(), vtWithNull.ToString()); + } + + { + var vtWithNull = CreateLong(1, 2, 3, 4, 5, 6, 7, new ValueTuple(null, null, null, null, null, null)); + var tupleWithNull = CreateLongRef(1, 2, 3, 4, 5, 6, 7, new Tuple(null, null, null, null, null, null)); + AssertEqual("(1, 2, 3, 4, 5, 6, 7, , , , , , )", vtWithNull.ToString()); + AssertEqual(tupleWithNull.ToString(), vtWithNull.ToString()); + } + + { + var vtWithNull = CreateLong(1, 2, 3, 4, 5, 6, 7, new ValueTuple(null, null, null, null, null, null, null)); + var tupleWithNull = CreateLongRef(1, 2, 3, 4, 5, 6, 7, new Tuple(null, null, null, null, null, null, null)); + AssertEqual("(1, 2, 3, 4, 5, 6, 7, , , , , , , )", vtWithNull.ToString()); + AssertEqual(tupleWithNull.ToString(), vtWithNull.ToString()); + } + } + + public static int Main() + { + int result = 0; + try + { + Test(); + result = 100; + } + catch (Exception) + { + result = -1; + } + + return result; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.csproj new file mode 100644 index 000000000000..5bd417ae6c76 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.csproj @@ -0,0 +1,38 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-B8089FFA8D79} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + + + + + + + False + + + + + True + + + + + + + + + + \ No newline at end of file From 16d9adf520fa434c3a9da71fd75330ade7302702 Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Tue, 8 May 2018 08:50:49 -0700 Subject: [PATCH 2/3] PR feedback --- src/jit/morph.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 5478efd358e7..78c3953c74e6 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -4981,8 +4981,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry } if (lcl != nullptr) { - unsigned lclNum = lcl->gtLclNum; - if (lvaGetPromotionType(lclNum) == PROMOTION_TYPE_INDEPENDENT) + if (lvaGetPromotionType(lcl->gtLclNum) == PROMOTION_TYPE_INDEPENDENT) { arg = fgMorphLclArgToFieldlist(lcl); } @@ -4994,7 +4993,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry arg = gtNewOperNode(GT_ADDR, TYP_I_IMPL, arg); arg = gtNewObjNode(lvaGetStruct(lcl->gtLclNum), arg); } - // Its fields will need to accessed by address. + // Its fields will need to be accessed by address. lvaSetVarDoNotEnregister(lcl->gtLclNum DEBUG_ARG(DNER_IsStructArg)); } } @@ -5498,7 +5497,6 @@ GenTreeFieldList* Compiler::fgMorphLclArgToFieldlist(GenTreeLclVarCommon* lcl) LclVarDsc* varDsc = &(lvaTable[lcl->gtLclNum]); assert(varDsc->lvPromoted == true); - unsigned lclNum = lcl->gtLclNum; unsigned offset = 0; unsigned fieldCount = varDsc->lvFieldCnt; GenTreeFieldList* listEntry = nullptr; From 687da67b16ac9f09810aa1fb546ec25785980c97 Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Tue, 8 May 2018 17:20:04 -0700 Subject: [PATCH 3/3] Handle SIMD field of GT_FIELD_LIST in codegen. Add header to test case. --- src/jit/codegenarmarch.cpp | 24 +++++++++---------- .../JitBlue/GitHub_16377/GitHub_16377.cs | 4 ++++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/jit/codegenarmarch.cpp b/src/jit/codegenarmarch.cpp index 99826f766a02..90b802d67b08 100644 --- a/src/jit/codegenarmarch.cpp +++ b/src/jit/codegenarmarch.cpp @@ -592,24 +592,24 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode) bool isStruct = (targetType == TYP_STRUCT) || (source->OperGet() == GT_FIELD_LIST); - if (varTypeIsSIMD(targetType)) + if (!isStruct) // a normal non-Struct argument { - assert(!source->isContained()); + if (varTypeIsSIMD(targetType)) + { + assert(!source->isContained()); - regNumber srcReg = genConsumeReg(source); + regNumber srcReg = genConsumeReg(source); - emitAttr storeAttr = emitTypeSize(targetType); + emitAttr storeAttr = emitTypeSize(targetType); - assert((srcReg != REG_NA) && (genIsValidFloatReg(srcReg))); - emit->emitIns_S_R(INS_str, storeAttr, srcReg, varNumOut, argOffsetOut); + assert((srcReg != REG_NA) && (genIsValidFloatReg(srcReg))); + emit->emitIns_S_R(INS_str, storeAttr, srcReg, varNumOut, argOffsetOut); - argOffsetOut += EA_SIZE_IN_BYTES(storeAttr); - assert(argOffsetOut <= argOffsetMax); // We can't write beyound the outgoing area area - return; - } + argOffsetOut += EA_SIZE_IN_BYTES(storeAttr); + assert(argOffsetOut <= argOffsetMax); // We can't write beyound the outgoing area area + return; + } - if (!isStruct) // a normal non-Struct argument - { instruction storeIns = ins_Store(targetType); emitAttr storeAttr = emitTypeSize(targetType); diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.cs b/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.cs index 069eff401337..1f78cd0dcc5b 100644 --- a/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.cs +++ b/tests/src/JIT/Regression/JitBlue/GitHub_16377/GitHub_16377.cs @@ -1,3 +1,7 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + using System; class GitHub_16377