From c36ba2d8183ce0d7a36b74a226ffdf3d68721675 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Sun, 8 Aug 2021 14:48:39 -0700 Subject: [PATCH] JIT: fix morph tail call copy opt alias analysis (#57009) If there are non-call references to an implicit byref local, disqualify that local from morph tail call copy optimization. The fix is conservative in that "harmless" non-call references like field accesses will also disqualify the opt. This can be improved on with extra analysis in local morph. Closes #56734. --- src/coreclr/jit/lclmorph.cpp | 2 +- src/coreclr/jit/morph.cpp | 18 +++++++++++- .../JitBlue/Runtime_56743/Runtime_56743_0.cs | 34 ++++++++++++++++++++++ .../JitBlue/Runtime_56743/Runtime_56743_0.csproj | 12 ++++++++ .../JitBlue/Runtime_56743/Runtime_56743_1.cs | 34 ++++++++++++++++++++++ .../JitBlue/Runtime_56743/Runtime_56743_1.csproj | 13 +++++++++ .../JitBlue/Runtime_56743/Runtime_56743_2.cs | 34 ++++++++++++++++++++++ .../JitBlue/Runtime_56743/Runtime_56743_2.csproj | 13 +++++++++ 8 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_0.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_0.csproj create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_1.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_1.csproj create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_2.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_2.csproj diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 349c5ef..caa8a1b 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1184,7 +1184,7 @@ private: if (isArgToCall) { - JITDUMP("LocalAddressVisitor incrementing weighted ref count from %d to %d" + JITDUMP("LocalAddressVisitor incrementing weighted ref count from " FMT_WT " to " FMT_WT " for implicit by-ref V%02d arg passed to call\n", varDsc->lvRefCntWtd(RCS_EARLY), varDsc->lvRefCntWtd(RCS_EARLY) + 1, lclNum); varDsc->incLvRefCntWtd(1, RCS_EARLY); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d23fea5..a00b4bb 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -7118,13 +7118,29 @@ bool Compiler::fgCallHasMustCopyByrefParameter(GenTreeCall* callee) // We only check this for calls with small numbers of arguments, // as the analysis cost will be quadratic. // - if (varDsc->lvRefCnt(RCS_EARLY) == 1) + const unsigned totalAppearances = varDsc->lvRefCnt(RCS_EARLY); + const unsigned callAppearances = (unsigned)varDsc->lvRefCntWtd(RCS_EARLY); + assert(totalAppearances >= callAppearances); + + if (totalAppearances == 1) { JITDUMP("... yes, arg is the only appearance of V%02u\n", lclNum); hasMustCopyByrefParameter = false; } + else if (totalAppearances > callAppearances) + { + // lvRefCntWtd tracks the number of appearances of the arg at call sites. + // If this number doesn't match the regular ref count, there is + // a non-call appearance, and we must be conservative. + // + JITDUMP("... no, arg has %u non-call appearance(s)\n", + totalAppearances - callAppearances); + } else if (argInfo->ArgCount() <= argLimit) { + JITDUMP("... all %u appearance(s) are as implicit byref args to calls.\n" + "... Running alias analysis on this call's args\n", + totalAppearances); GenTree* interferingArg = nullptr; for (unsigned index2 = 0; index2 < argInfo->ArgCount(); ++index2) { diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_0.cs b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_0.cs new file mode 100644 index 0000000..8140262 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_0.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +class Runtime_56743_0 +{ + [MethodImpl(MethodImplOptions.NoOptimization)] + static int Main() + { + int result = Foo(default, default); + return result == 0 ? 100 : -1; + } + + static int Foo(S s, Span span) + { + span = MemoryMarshal.CreateSpan(ref s, 1); + return Bar(s, span); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Bar(S h, Span s) + { + s[0].A = 10; + return h.A; + } + + struct S + { + public int A, B, C, D; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_0.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_0.csproj new file mode 100644 index 0000000..f3e1cbd --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_0.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + + diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_1.cs b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_1.cs new file mode 100644 index 0000000..32bfb85 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_1.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; + +unsafe class Runtime_56743_1 +{ + [MethodImpl(MethodImplOptions.NoOptimization)] + static int Main() + { + int result = Foo(default); + return result == 0 ? 100 : -1; + } + + static S* s_s; + static int Foo(S s) + { + s_s = &s; + return Bar(s); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Bar(S h) + { + s_s->A = 10; + return h.A; + } + + struct S + { + public int A, B, C, D; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_1.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_1.csproj new file mode 100644 index 0000000..bf6f589 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_1.csproj @@ -0,0 +1,13 @@ + + + Exe + + + None + True + True + + + + + diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_2.cs b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_2.cs new file mode 100644 index 0000000..b258e9e --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_2.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; + +unsafe class Runtime_56743_2 +{ + [MethodImpl(MethodImplOptions.NoOptimization)] + static int Main() + { + int result = Foo(default); + return result == 0 ? 100 : -1; + } + + static int Foo(S h) + { + h.H = &h; + return Bar(h); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Bar(S h) + { + h.H->A = 10; + return h.A; + } + + unsafe struct S + { + public int A, B; + public S* H; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_2.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_2.csproj new file mode 100644 index 0000000..bf6f589 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56743/Runtime_56743_2.csproj @@ -0,0 +1,13 @@ + + + Exe + + + None + True + True + + + + + -- 2.7.4