From ce1d090d33b400a25620c0145046471495067cc7 Mon Sep 17 00:00:00 2001 From: dotnet-maestro-bot Date: Thu, 6 Jun 2019 16:05:35 -0700 Subject: [PATCH] Port #22775 into release/2.1 (#24858) (#24954) * Set flag in comp info to signal that a caller has >8 byte struct args (#22775) * Set flag in comp info to signal that a caller has >8 byte struct args This will be used by fgCanFastTailCall to correctly determine whether an arm64 or x64 linux caller/callee can fastTailCall. It is also a workaround to #12468 to catch early any slot shuffling that would happen in LowerFastTailCall. Which currently assumes all parameters are one slot size. * Address feedback * Apply format patch * Add comment * apply new format patch * Address feedback and add header --- src/jit/compiler.h | 5 +- src/jit/lclvars.cpp | 9 ++-- src/jit/morph.cpp | 9 +++- src/jit/register_arg_convention.h | 4 +- .../JitBlue/GitHub_22330/GitHub_22330.cs | 53 +++++++++++++++++++ .../JitBlue/GitHub_22330/GitHub_22330.csproj | 33 ++++++++++++ 6 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_22330/GitHub_22330.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_22330/GitHub_22330.csproj diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 97138fa81825..2bce77dd294c 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -8602,8 +8602,9 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX unsigned compArgsCount; // Number of arguments (incl. implicit and hidden) #if FEATURE_FASTTAILCALL - size_t compArgStackSize; // Incoming argument stack size in bytes -#endif // FEATURE_FASTTAILCALL + size_t compArgStackSize; // Incoming argument stack size in bytes + bool compHasMultiSlotArgs; // Caller has >8 byte sized struct parameter +#endif // FEATURE_FASTTAILCALL unsigned compRetBuffArg; // position of hidden return param var (0, 1) (BAD_VAR_NUM means not present); int compTypeCtxtArg; // position of hidden param for type context for generic code (CORINFO_CALLCONV_PARAMTYPE) diff --git a/src/jit/lclvars.cpp b/src/jit/lclvars.cpp index a8f6fc3040f3..330da7081b72 100644 --- a/src/jit/lclvars.cpp +++ b/src/jit/lclvars.cpp @@ -364,7 +364,8 @@ void Compiler::lvaInitArgs(InitVarDscInfo* varDscInfo) // Save the stack usage information // We can get register usage information using codeGen->intRegState and // codeGen->floatRegState - info.compArgStackSize = varDscInfo->stackArgSize; + info.compArgStackSize = varDscInfo->stackArgSize; + info.compHasMultiSlotArgs = varDscInfo->hasMultiSlotStruct; #endif // FEATURE_FASTTAILCALL // The total argument size must be aligned. @@ -801,8 +802,9 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) // If there is a second eightbyte, get a register for it too and map the arg to the reg number. if (structDesc.eightByteCount >= 2) { - secondEightByteType = GetEightByteType(structDesc, 1); - secondAllocatedRegArgNum = varDscInfo->allocRegArg(secondEightByteType, 1); + secondEightByteType = GetEightByteType(structDesc, 1); + secondAllocatedRegArgNum = varDscInfo->allocRegArg(secondEightByteType, 1); + varDscInfo->hasMultiSlotStruct = true; } if (secondEightByteType != TYP_UNDEF) @@ -817,6 +819,7 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) { varDsc->lvOtherArgReg = genMapRegArgNumToRegNum(firstAllocatedRegArgNum + 1, TYP_I_IMPL); varDsc->addPrefReg(genRegMask(varDsc->lvOtherArgReg), this); + varDscInfo->hasMultiSlotStruct = true; } #endif // _TARGET_ARM64_ #endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING) diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index ce201ee41900..9dcd6e07f537 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -7672,6 +7672,7 @@ bool Compiler::fgCanFastTailCall(GenTreeCall* callee) } const unsigned maxRegArgs = MAX_REG_ARG; + hasTwoSlotSizedStruct = hasTwoSlotSizedStruct || info.compHasMultiSlotArgs; // If we reached here means that callee has only those argument types which can be passed in // a register and if passed on stack will occupy exactly one stack slot in out-going arg area. @@ -7746,8 +7747,12 @@ bool Compiler::fgCanFastTailCall(GenTreeCall* callee) return false; } - // Callee has a >8 and <=16 byte struct and arguments that has to go on the stack. Do not fastTailCall. - if (calleeStackSize > 0 && hasTwoSlotSizedStruct) + // Either the caller or callee has a >8 and <=16 byte struct and arguments that has to go on the stack. Do not + // fastTailCall. + // + // When either the caller or callee have multi-stlot stack arguments we cannot safely + // shuffle arguments in LowerFastTailCall. See https://github.com/dotnet/coreclr/issues/12468. + if (hasStackArgs && hasTwoSlotSizedStruct) { reportFastTailCallDecision("Will not fastTailCall calleeStackSize > 0 && hasTwoSlotSizedStruct", callerStackSize, calleeStackSize); diff --git a/src/jit/register_arg_convention.h b/src/jit/register_arg_convention.h index 758c764ea3c8..f948e45ebae0 100644 --- a/src/jit/register_arg_convention.h +++ b/src/jit/register_arg_convention.h @@ -29,6 +29,7 @@ struct InitVarDscInfo #if FEATURE_FASTTAILCALL // It is used to calculate argument stack size information in byte unsigned stackArgSize; + bool hasMultiSlotStruct; #endif // FEATURE_FASTTAILCALL public: @@ -49,7 +50,8 @@ struct InitVarDscInfo #endif // _TARGET_ARM_ #if FEATURE_FASTTAILCALL - stackArgSize = 0; + stackArgSize = 0; + hasMultiSlotStruct = false; #endif // FEATURE_FASTTAILCALL } diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_22330/GitHub_22330.cs b/tests/src/JIT/Regression/JitBlue/GitHub_22330/GitHub_22330.cs new file mode 100644 index 000000000000..0933a4c6419e --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_22330/GitHub_22330.cs @@ -0,0 +1,53 @@ +// 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; +using System.Runtime.CompilerServices; + +class X +{ + // a -> rdi + // b -> rsi + // c -> rdx + // -> rcx + // d -> r8 + // e -> r9 + // f -> s[0] + // g -> s[1] + public static int F(int a, int b, Guid c, int d, int e, int f, int g) + { + Guid[] z = new Guid[] { c }; + // Bug is here passing params to G: f gets trashed by g. + return G(a, b, z, d, e, f, g); + } + + // loop here is just to make this method too big to inline + // if we set [noinline] to effect this, we won't tail call it either. + // + // a -> rdi + // b -> rsi + // c -> rdx + // d -> rcx + // e -> r8 + // f -> r9 + // g -> s[0] + public static int G(int a, int b, Guid[] c, int d, int e, int f, int g) + { + int r = 0; + for (int i = 0; i < 10; i++) + { + r += f + g; + } + return r / 10; + } + + // No-opt to stop F from being inlined without marking it noinline + [MethodImpl(MethodImplOptions.NoOptimization)] + public static int Main() + { + int result = F(0, 1, Guid.Empty, 3, 4, 33, 67); + Console.WriteLine($"Result={result}"); + return result; + } +} \ No newline at end of file diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_22330/GitHub_22330.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_22330/GitHub_22330.csproj new file mode 100644 index 000000000000..42f8a01f3919 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_22330/GitHub_22330.csproj @@ -0,0 +1,33 @@ + + + + + Debug + AnyCPU + 2.0 + {2649FAFE-07BF-4F93-8120-BA9A69285ABB} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + None + True + + + + False + + + + + + + + + + + \ No newline at end of file