From cca33f1cf778f27b706fa11c3387d2bed9859b8b Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 8 Sep 2021 17:34:06 +0200 Subject: [PATCH 1/3] Fix reporting of live var ranges after callf+always pairs We were basing the decision on whether a home update was needed on the live out variables of the previous block, but that block is skipped for callf+finally pairs. Change the logic to use the callf block instead. Fix #57752 --- src/coreclr/jit/lsra.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index b2e8dadd6cf516..b0c3de04af1468 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -1348,7 +1348,17 @@ void LinearScan::recordVarLocationsAtStartOfBB(BasicBlock* bb) count++; #ifdef USING_VARIABLE_LIVE_RANGE - if (bb->bbPrev != nullptr && VarSetOps::IsMember(compiler, bb->bbPrev->bbLiveOut, varIndex)) + BasicBlock* prevReportedBlock = bb->bbPrev; + if (bb->bbPrev != nullptr && bb->bbPrev->isBBCallAlwaysPairTail()) + { + // For callf+always pair we skip over reporting anything for + // the always block (which is not going to change any liveness + // anyway). So whether we need to rehome or not depends on what + // we reported at the end of the callf block. + prevReportedBlock = bb->bbPrev->bbPrev; + } + + if (prevReportedBlock && VarSetOps::IsMember(compiler, prevReportedBlock->bbLiveOut, varIndex)) { // varDsc was alive on previous block end ("bb->bbPrev->bbLiveOut"), so it has an open // "VariableLiveRange" which should change to be according "getInVarToRegMap" From 83145053d9025a731190b63c3b18a45597416fc1 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 8 Sep 2021 17:48:51 +0200 Subject: [PATCH 2/3] Add tests --- .../JitBlue/Runtime_57752/Runtime_57752_1.cs | 95 +++++++++++++++++++ .../Runtime_57752/Runtime_57752_1.csproj | 13 +++ .../JitBlue/Runtime_57752/Runtime_57752_2.cs | 57 +++++++++++ .../Runtime_57752/Runtime_57752_2.csproj | 13 +++ 4 files changed, 178 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_1.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_1.csproj create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_2.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_2.csproj diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_1.cs b/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_1.cs new file mode 100644 index 00000000000000..7855b2288bc6f5 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_1.cs @@ -0,0 +1,95 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Generated by Fuzzlyn v1.3 on 2021-08-19 15:47:06 +// Run on .NET 6.0.0-dev on X64 Windows +// Seed: 16489483397161801783 +// Reduced from 251.9 KiB to 1.7 KiB in 02:46:36 +// +// Assert failure(PID 27840 [0x00006cc0], Thread: 1464 [0x05b8]): Assertion failed '!m_VariableLiveRanges->back().m_EndEmitLocation.Valid()' in 'Program:M51(System.Boolean[],System.UInt16[],System.Boolean[],long,System.Int32[],byref)' during 'Generate code' (IL size 140) +// +// File: D:\dev\dotnet\runtime\src\coreclr\jit\codegencommon.cpp Line: 11990 +// Image: D:\dev\Fuzzlyn\Fuzzlyn\publish\windows-x64\Fuzzlyn.exe +// +// +public class Runtime_57752_1 +{ + internal static I s_rt; + internal static long s_10; + internal static bool[] s_27 = new[]{true}; + internal static short s_28; + internal static bool s_33; + internal static bool s_53; + internal static int[][] s_56; + public static int Main() + { + s_rt = new C(); + var vr9 = new ushort[]{0}; + var vr10 = new bool[]{true}; + var vr11 = new int[]{0}; + M51(s_27, vr9, vr10, 0, vr11, ref s_28); + return 100; + } + + internal static void M51(bool[] arg0, ushort[] arg5, bool[] arg9, ulong arg10, int[] arg11, ref short arg12) + { + if (arg0[0]) + { + if (arg0[0]) + { + arg11[0] = arg11[0]; + } + else + { + try + { + } + finally + { + arg9[0] = arg9[0]; + } + + try + { + arg0[0] = true; + } + finally + { + for (int var4 = 0; var4 < 2; var4++) + { + s_53 = arg0[0]; + arg10 = 0; + ushort var6 = arg5[0]; + long var8 = 0; + s_rt.Write(s_56[0][0]); + s_rt.Write(var8); + } + + s_33 = false; + try + { + s_10 = s_10; + } + finally + { + arg12 = ref arg12; + } + } + } + + bool vr1 = arg9[0]; + } + } +} + +public interface I +{ + void Write(T val); +} + +public class C : I +{ + public void Write(T val) + { + } +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_1.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_1.csproj new file mode 100644 index 00000000000000..e7284d26ab2f18 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_1.csproj @@ -0,0 +1,13 @@ + + + Exe + True + + + None + True + + + + + diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_2.cs b/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_2.cs new file mode 100644 index 00000000000000..1ed318676bf2bb --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_2.cs @@ -0,0 +1,57 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Generated by Fuzzlyn v1.3 on 2021-08-23 02:35:43 +// Run on .NET 6.0.0-dev on X64 Windows +// Seed: 13788434105727734599 +// Reduced from 365.1 KiB to 0.9 KiB in 02:25:35 +// Exits with error: +// +// Assert failure(PID 11828 [0x00002e34], Thread: 24792 [0x60d8]): Assertion failed 'm_VariableLiveRanges != nullptr && !m_VariableLiveRanges->empty()' in 'Program:M53(byref,byref)' during 'Generate code' (IL size 177) +// +// File: D:\dev\dotnet\runtime\src\coreclr\jit\codegencommon.cpp Line: 11987 +// Image: D:\dev\Fuzzlyn\Fuzzlyn\publish\windows-x64\Fuzzlyn.exe +// +// +public class Runtime_57752_2 +{ + internal static ulong s_46; + public static int Main() + { + M53(ref s_46, ref s_46); + return 100; + } + + internal static void M53(ref ulong arg0, ref ulong arg1) + { + int var11 = default(int); + for (int var0 = 0; var0 < 1; var0++) + { + return; + } + + try + { + bool vr0 = arg1 >= 0; + bool vr6 = vr0; + } + finally + { + for (int var2 = 0; var2 < -1; var2++) + { + } + + try + { + } + finally + { + System.Console.WriteLine(var11); + } + + var vr7 = new uint[][]{new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}}; + } + + System.Console.WriteLine(arg0); + } +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_2.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_2.csproj new file mode 100644 index 00000000000000..e7284d26ab2f18 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57752/Runtime_57752_2.csproj @@ -0,0 +1,13 @@ + + + Exe + True + + + None + True + + + + + From 9c604859febf93892a8961b2a32e7d7cca17c8c0 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 8 Sep 2021 17:52:18 +0200 Subject: [PATCH 3/3] Fix comment --- src/coreclr/jit/lsra.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index b0c3de04af1468..4ff069b8c4fafa 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -1351,17 +1351,19 @@ void LinearScan::recordVarLocationsAtStartOfBB(BasicBlock* bb) BasicBlock* prevReportedBlock = bb->bbPrev; if (bb->bbPrev != nullptr && bb->bbPrev->isBBCallAlwaysPairTail()) { - // For callf+always pair we skip over reporting anything for - // the always block (which is not going to change any liveness - // anyway). So whether we need to rehome or not depends on what - // we reported at the end of the callf block. + // For callf+always pair we generate the code for the always + // block in genCallFinally and skip it, so we don't report + // anything for it (it has only trivial instructions, so that + // does not matter much). So whether we need to rehome or not + // depends on what we reported at the end of the callf block. prevReportedBlock = bb->bbPrev->bbPrev; } - if (prevReportedBlock && VarSetOps::IsMember(compiler, prevReportedBlock->bbLiveOut, varIndex)) + if (prevReportedBlock != nullptr && VarSetOps::IsMember(compiler, prevReportedBlock->bbLiveOut, varIndex)) { - // varDsc was alive on previous block end ("bb->bbPrev->bbLiveOut"), so it has an open - // "VariableLiveRange" which should change to be according "getInVarToRegMap" + // varDsc was alive on previous block end so it has an open + // "VariableLiveRange" which should change to be according to + // "getInVarToRegMap" compiler->codeGen->getVariableLiveKeeper()->siUpdateVariableLiveRange(varDsc, varNum); } #endif // USING_VARIABLE_LIVE_RANGE