From ba39b8985fd6eac64c306acd175a15cbbb98c67e Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 17 Jan 2023 12:21:14 +0100 Subject: [PATCH 1/6] JIT: Account for GT_JMP implicit uses in local morph ref counting Fix #80731 --- src/coreclr/jit/lclmorph.cpp | 9 ++ .../JitBlue/Runtime_80731/Runtime_80731.il | 147 ++++++++++++++++++ .../Runtime_80731/Runtime_80731.ilproj | 10 ++ .../Runtime_80731/Runtime_80731_source.cs | 75 +++++++++ .../Regression/JitBlue/Runtime_80731/il.il | 147 ++++++++++++++++++ 5 files changed, 388 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731.il create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731.ilproj create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731_source.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_80731/il.il diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 8b3119d1cfd76..d8f73bcde954e 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -606,6 +606,15 @@ class LocalAddressVisitor final : public GenTreeVisitor } } + if (node->OperIs(GT_JMP)) + { + // GT_JMP has implicit uses of all arguments. + for (unsigned lclNum = 0; lclNum < m_compiler->info.compArgsCount; lclNum++) + { + UpdateEarlyRefCount(lclNum); + } + } + PushValue(use); return Compiler::WALK_CONTINUE; diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731.il b/src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731.il new file mode 100644 index 0000000000000..65e797ac5ce78 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731.il @@ -0,0 +1,147 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Runtime {} +.assembly extern System.Console {} +.assembly Runtime_80731 {} + +.class private auto ansi beforefieldinit Runtime_80731 + extends [System.Runtime]System.Object +{ + .class sequential ansi sealed nested private beforefieldinit S16 + extends [System.Runtime]System.ValueType + { + .field public int32 A + .field public int32 B + .field public int32 C + .field public int32 D + } // end of class S16 + + .method private hidebysig static int32 + Main(string[] args) cil managed + { + .entrypoint + // Code size 99 (0x63) + .maxstack 2 + .locals init (int32 V_0, + int32 V_1, + int32 V_2, + valuetype Runtime_80731/S16 V_3) + IL_0000: ldc.i4.s 100 + IL_0002: stloc.0 + IL_0003: ldloca.s V_3 + IL_0005: initobj Runtime_80731/S16 + IL_000b: ldloca.s V_3 + IL_000d: ldc.i4 0x162e + IL_0012: stfld int32 Runtime_80731/S16::A + IL_0017: ldloc.3 + IL_0018: call int32 Runtime_80731::ImplicitByref(valuetype Runtime_80731/S16) + IL_001d: stloc.1 + IL_001e: ldloc.1 + IL_001f: ldc.i4 0x162e + IL_0024: beq.s IL_003a + + IL_0026: ldstr "FAIL: ImplicitByref returned {0}" + IL_002b: ldloc.1 + IL_002c: box [System.Runtime]System.Int32 + IL_0031: call void [System.Console]System.Console::WriteLine(string, + object) + IL_0036: ldloc.0 + IL_0037: ldc.i4.1 + IL_0038: or + IL_0039: stloc.0 + IL_003a: ldc.i4 0x4d2 + IL_003f: call int32 Runtime_80731::ForwardSub(int32) + IL_0044: stloc.2 + IL_0045: ldloc.2 + IL_0046: ldc.i4 0x162e + IL_004b: beq.s IL_0061 + + IL_004d: ldstr "FAIL: ForwardSub returned {0}" + IL_0052: ldloc.2 + IL_0053: box [System.Runtime]System.Int32 + IL_0058: call void [System.Console]System.Console::WriteLine(string, + object) + IL_005d: ldloc.0 + IL_005e: ldc.i4.2 + IL_005f: or + IL_0060: stloc.0 + IL_0061: ldloc.0 + IL_0062: ret + } // end of method Runtime_80731::Main + + .method private hidebysig static int32 + ImplicitByref(valuetype Runtime_80731/S16 s) cil managed noinlining + { + // Code size 11 (0xb) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call void Runtime_80731::Modify(valuetype Runtime_80731/S16) + IL_0006: jmp int32 Runtime_80731::ImplicitByrefCallee(valuetype Runtime_80731/S16) + } // end of method Runtime_80731::ImplicitByref + + .method private hidebysig static void Modify(valuetype Runtime_80731/S16 s) cil managed noinlining + { + // Code size 19 (0x13) + .maxstack 8 + IL_0000: ldarga.s s + IL_0002: ldc.i4 0x4d2 + IL_0007: stfld int32 Runtime_80731/S16::A + IL_000c: ldarg.0 + IL_000d: call void Runtime_80731::Consume(!!0) + IL_0012: ret + } // end of method Runtime_80731::Modify + + .method private hidebysig static void Consume(!!T val) cil managed noinlining + { + // Code size 1 (0x1) + .maxstack 8 + IL_0000: ret + } // end of method Runtime_80731::Consume + + .method private hidebysig static int32 + ImplicitByrefCallee(valuetype Runtime_80731/S16 s) cil managed noinlining + { + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld int32 Runtime_80731/S16::A + IL_0006: ret + } // end of method Runtime_80731::ImplicitByrefCallee + + .method private hidebysig static int32 + ForwardSub(int32 a) cil managed noinlining + { + // Code size 20 (0x14) + .maxstack 8 + IL_0000: ldc.i4 0x162e + IL_0005: starg a + IL_0009: ldarg.0 + IL_000a: call void Runtime_80731::Consume(!!0) + IL_000f: jmp int32 Runtime_80731::ForwardSubCallee(int32) + } // end of method Runtime_80731::ForwardSub + + .method private hidebysig static int32 + ForwardSubCallee(int32 a) cil managed noinlining + { + // Code size 2 (0x2) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ret + } // end of method Runtime_80731::ForwardSubCallee + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [System.Runtime]System.Object::.ctor() + IL_0006: ret + } // end of method Runtime_80731::.ctor + +} // end of class Runtime_80731 + +// ============================================================= + +// *********** DISASSEMBLY COMPLETE *********************** diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731.ilproj b/src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731.ilproj new file mode 100644 index 0000000000000..7dab57fe6d225 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731.ilproj @@ -0,0 +1,10 @@ + + + Exe + None + True + + + + + diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731_source.cs b/src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731_source.cs new file mode 100644 index 0000000000000..e500ba9159cc6 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731_source.cs @@ -0,0 +1,75 @@ +using InlineIL; +using System; +using System.Runtime.CompilerServices; + +// Tests that we properly account for local uses of GT_JMP nodes when omitting +// copies for implicit byrefs and when forward substituting. +class Runtime_80731 +{ + static int Main(string[] args) + { + int code = 100; + + int implicitByrefResult = ImplicitByref(new S16 { A = 5678 }); + if (implicitByrefResult != 5678) + { + Console.WriteLine("FAIL: ImplicitByref returned {0}", implicitByrefResult); + code |= 1; + } + + int forwardSubResult = ForwardSub(1234); + if (forwardSubResult != 5678) + { + Console.WriteLine("FAIL: ForwardSub returned {0}", forwardSubResult); + code |= 2; + } + + return code; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int ImplicitByref(S16 s) + { + Modify(s); + IL.Emit.Jmp(new MethodRef(typeof(Runtime_80731), nameof(ImplicitByrefCallee))); + throw IL.Unreachable(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Modify(S16 s) + { + s.A = 1234; + Consume(s); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Consume(T val) + { + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int ImplicitByrefCallee(S16 s) + { + return s.A; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int ForwardSub(int a) + { + a = 5678; + Consume(a); + IL.Emit.Jmp(new MethodRef(typeof(Runtime_80731), nameof(ForwardSubCallee))); + throw IL.Unreachable(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int ForwardSubCallee(int a) + { + return a; + } + + private struct S16 + { + public int A, B, C, D; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_80731/il.il b/src/tests/JIT/Regression/JitBlue/Runtime_80731/il.il new file mode 100644 index 0000000000000..65e797ac5ce78 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_80731/il.il @@ -0,0 +1,147 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Runtime {} +.assembly extern System.Console {} +.assembly Runtime_80731 {} + +.class private auto ansi beforefieldinit Runtime_80731 + extends [System.Runtime]System.Object +{ + .class sequential ansi sealed nested private beforefieldinit S16 + extends [System.Runtime]System.ValueType + { + .field public int32 A + .field public int32 B + .field public int32 C + .field public int32 D + } // end of class S16 + + .method private hidebysig static int32 + Main(string[] args) cil managed + { + .entrypoint + // Code size 99 (0x63) + .maxstack 2 + .locals init (int32 V_0, + int32 V_1, + int32 V_2, + valuetype Runtime_80731/S16 V_3) + IL_0000: ldc.i4.s 100 + IL_0002: stloc.0 + IL_0003: ldloca.s V_3 + IL_0005: initobj Runtime_80731/S16 + IL_000b: ldloca.s V_3 + IL_000d: ldc.i4 0x162e + IL_0012: stfld int32 Runtime_80731/S16::A + IL_0017: ldloc.3 + IL_0018: call int32 Runtime_80731::ImplicitByref(valuetype Runtime_80731/S16) + IL_001d: stloc.1 + IL_001e: ldloc.1 + IL_001f: ldc.i4 0x162e + IL_0024: beq.s IL_003a + + IL_0026: ldstr "FAIL: ImplicitByref returned {0}" + IL_002b: ldloc.1 + IL_002c: box [System.Runtime]System.Int32 + IL_0031: call void [System.Console]System.Console::WriteLine(string, + object) + IL_0036: ldloc.0 + IL_0037: ldc.i4.1 + IL_0038: or + IL_0039: stloc.0 + IL_003a: ldc.i4 0x4d2 + IL_003f: call int32 Runtime_80731::ForwardSub(int32) + IL_0044: stloc.2 + IL_0045: ldloc.2 + IL_0046: ldc.i4 0x162e + IL_004b: beq.s IL_0061 + + IL_004d: ldstr "FAIL: ForwardSub returned {0}" + IL_0052: ldloc.2 + IL_0053: box [System.Runtime]System.Int32 + IL_0058: call void [System.Console]System.Console::WriteLine(string, + object) + IL_005d: ldloc.0 + IL_005e: ldc.i4.2 + IL_005f: or + IL_0060: stloc.0 + IL_0061: ldloc.0 + IL_0062: ret + } // end of method Runtime_80731::Main + + .method private hidebysig static int32 + ImplicitByref(valuetype Runtime_80731/S16 s) cil managed noinlining + { + // Code size 11 (0xb) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call void Runtime_80731::Modify(valuetype Runtime_80731/S16) + IL_0006: jmp int32 Runtime_80731::ImplicitByrefCallee(valuetype Runtime_80731/S16) + } // end of method Runtime_80731::ImplicitByref + + .method private hidebysig static void Modify(valuetype Runtime_80731/S16 s) cil managed noinlining + { + // Code size 19 (0x13) + .maxstack 8 + IL_0000: ldarga.s s + IL_0002: ldc.i4 0x4d2 + IL_0007: stfld int32 Runtime_80731/S16::A + IL_000c: ldarg.0 + IL_000d: call void Runtime_80731::Consume(!!0) + IL_0012: ret + } // end of method Runtime_80731::Modify + + .method private hidebysig static void Consume(!!T val) cil managed noinlining + { + // Code size 1 (0x1) + .maxstack 8 + IL_0000: ret + } // end of method Runtime_80731::Consume + + .method private hidebysig static int32 + ImplicitByrefCallee(valuetype Runtime_80731/S16 s) cil managed noinlining + { + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld int32 Runtime_80731/S16::A + IL_0006: ret + } // end of method Runtime_80731::ImplicitByrefCallee + + .method private hidebysig static int32 + ForwardSub(int32 a) cil managed noinlining + { + // Code size 20 (0x14) + .maxstack 8 + IL_0000: ldc.i4 0x162e + IL_0005: starg a + IL_0009: ldarg.0 + IL_000a: call void Runtime_80731::Consume(!!0) + IL_000f: jmp int32 Runtime_80731::ForwardSubCallee(int32) + } // end of method Runtime_80731::ForwardSub + + .method private hidebysig static int32 + ForwardSubCallee(int32 a) cil managed noinlining + { + // Code size 2 (0x2) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ret + } // end of method Runtime_80731::ForwardSubCallee + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [System.Runtime]System.Object::.ctor() + IL_0006: ret + } // end of method Runtime_80731::.ctor + +} // end of class Runtime_80731 + +// ============================================================= + +// *********** DISASSEMBLY COMPLETE *********************** From e53319936d43ec02f9d574df41deb9df285b44f6 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 17 Jan 2023 12:24:15 +0100 Subject: [PATCH 2/6] Remove leftover file --- .../Regression/JitBlue/Runtime_80731/il.il | 147 ------------------ 1 file changed, 147 deletions(-) delete mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_80731/il.il diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_80731/il.il b/src/tests/JIT/Regression/JitBlue/Runtime_80731/il.il deleted file mode 100644 index 65e797ac5ce78..0000000000000 --- a/src/tests/JIT/Regression/JitBlue/Runtime_80731/il.il +++ /dev/null @@ -1,147 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -.assembly extern System.Runtime {} -.assembly extern System.Console {} -.assembly Runtime_80731 {} - -.class private auto ansi beforefieldinit Runtime_80731 - extends [System.Runtime]System.Object -{ - .class sequential ansi sealed nested private beforefieldinit S16 - extends [System.Runtime]System.ValueType - { - .field public int32 A - .field public int32 B - .field public int32 C - .field public int32 D - } // end of class S16 - - .method private hidebysig static int32 - Main(string[] args) cil managed - { - .entrypoint - // Code size 99 (0x63) - .maxstack 2 - .locals init (int32 V_0, - int32 V_1, - int32 V_2, - valuetype Runtime_80731/S16 V_3) - IL_0000: ldc.i4.s 100 - IL_0002: stloc.0 - IL_0003: ldloca.s V_3 - IL_0005: initobj Runtime_80731/S16 - IL_000b: ldloca.s V_3 - IL_000d: ldc.i4 0x162e - IL_0012: stfld int32 Runtime_80731/S16::A - IL_0017: ldloc.3 - IL_0018: call int32 Runtime_80731::ImplicitByref(valuetype Runtime_80731/S16) - IL_001d: stloc.1 - IL_001e: ldloc.1 - IL_001f: ldc.i4 0x162e - IL_0024: beq.s IL_003a - - IL_0026: ldstr "FAIL: ImplicitByref returned {0}" - IL_002b: ldloc.1 - IL_002c: box [System.Runtime]System.Int32 - IL_0031: call void [System.Console]System.Console::WriteLine(string, - object) - IL_0036: ldloc.0 - IL_0037: ldc.i4.1 - IL_0038: or - IL_0039: stloc.0 - IL_003a: ldc.i4 0x4d2 - IL_003f: call int32 Runtime_80731::ForwardSub(int32) - IL_0044: stloc.2 - IL_0045: ldloc.2 - IL_0046: ldc.i4 0x162e - IL_004b: beq.s IL_0061 - - IL_004d: ldstr "FAIL: ForwardSub returned {0}" - IL_0052: ldloc.2 - IL_0053: box [System.Runtime]System.Int32 - IL_0058: call void [System.Console]System.Console::WriteLine(string, - object) - IL_005d: ldloc.0 - IL_005e: ldc.i4.2 - IL_005f: or - IL_0060: stloc.0 - IL_0061: ldloc.0 - IL_0062: ret - } // end of method Runtime_80731::Main - - .method private hidebysig static int32 - ImplicitByref(valuetype Runtime_80731/S16 s) cil managed noinlining - { - // Code size 11 (0xb) - .maxstack 8 - IL_0000: ldarg.0 - IL_0001: call void Runtime_80731::Modify(valuetype Runtime_80731/S16) - IL_0006: jmp int32 Runtime_80731::ImplicitByrefCallee(valuetype Runtime_80731/S16) - } // end of method Runtime_80731::ImplicitByref - - .method private hidebysig static void Modify(valuetype Runtime_80731/S16 s) cil managed noinlining - { - // Code size 19 (0x13) - .maxstack 8 - IL_0000: ldarga.s s - IL_0002: ldc.i4 0x4d2 - IL_0007: stfld int32 Runtime_80731/S16::A - IL_000c: ldarg.0 - IL_000d: call void Runtime_80731::Consume(!!0) - IL_0012: ret - } // end of method Runtime_80731::Modify - - .method private hidebysig static void Consume(!!T val) cil managed noinlining - { - // Code size 1 (0x1) - .maxstack 8 - IL_0000: ret - } // end of method Runtime_80731::Consume - - .method private hidebysig static int32 - ImplicitByrefCallee(valuetype Runtime_80731/S16 s) cil managed noinlining - { - // Code size 7 (0x7) - .maxstack 8 - IL_0000: ldarg.0 - IL_0001: ldfld int32 Runtime_80731/S16::A - IL_0006: ret - } // end of method Runtime_80731::ImplicitByrefCallee - - .method private hidebysig static int32 - ForwardSub(int32 a) cil managed noinlining - { - // Code size 20 (0x14) - .maxstack 8 - IL_0000: ldc.i4 0x162e - IL_0005: starg a - IL_0009: ldarg.0 - IL_000a: call void Runtime_80731::Consume(!!0) - IL_000f: jmp int32 Runtime_80731::ForwardSubCallee(int32) - } // end of method Runtime_80731::ForwardSub - - .method private hidebysig static int32 - ForwardSubCallee(int32 a) cil managed noinlining - { - // Code size 2 (0x2) - .maxstack 8 - IL_0000: ldarg.0 - IL_0001: ret - } // end of method Runtime_80731::ForwardSubCallee - - .method public hidebysig specialname rtspecialname - instance void .ctor() cil managed - { - // Code size 7 (0x7) - .maxstack 8 - IL_0000: ldarg.0 - IL_0001: call instance void [System.Runtime]System.Object::.ctor() - IL_0006: ret - } // end of method Runtime_80731::.ctor - -} // end of class Runtime_80731 - -// ============================================================= - -// *********** DISASSEMBLY COMPLETE *********************** From 59f38c69d2e8cf0a0b59baf286b92d3ba02cf5cd Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 17 Jan 2023 18:11:06 +0100 Subject: [PATCH 3/6] Exclude new test under llvmfullaot --- src/tests/issues.targets | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index dadea0c4b59e3..75485f5b32b60 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2994,6 +2994,9 @@ https://github.com/dotnet/runtime/issues/57350 + + https://github.com/dotnet/runtime/issues/57350 + llvmfullaot: EH problem From b6b69937700063764fa1851275a37733c7616c9c Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 17 Jan 2023 20:27:26 +0100 Subject: [PATCH 4/6] Use a switch in LocalAddressVisitor::PreOrderVisit --- src/coreclr/jit/lclmorph.cpp | 75 ++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index d8f73bcde954e..abd5339b01dcc 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -569,49 +569,58 @@ class LocalAddressVisitor final : public GenTreeVisitor { GenTree* const node = *use; - if (node->OperIs(GT_IND, GT_FIELD, GT_FIELD_ADDR)) + switch (node->gtOper) { - MorphStructField(node, user); - } - else if (node->OperIs(GT_LCL_FLD)) - { - MorphLocalField(node, user); - } + case GT_IND: + case GT_FIELD: + case GT_FIELD_ADDR: + MorphStructField(node, user); + break; + case GT_LCL_FLD: + MorphLocalField(node, user); + assert(node->OperIsLocal()); + __fallthrough; + case GT_LCL_VAR: + case GT_LCL_VAR_ADDR: + case GT_LCL_FLD_ADDR: + { + unsigned const lclNum = node->AsLclVarCommon()->GetLclNum(); + LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum); - if (node->OperIsLocal() || node->OperIsLocalAddr()) - { - unsigned const lclNum = node->AsLclVarCommon()->GetLclNum(); - LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum); + UpdateEarlyRefCount(lclNum); - UpdateEarlyRefCount(lclNum); + if (varDsc->lvIsStructField) + { + // Promoted field, increase count for the parent lclVar. + // + assert(!m_compiler->lvaIsImplicitByRefLocal(lclNum)); + unsigned parentLclNum = varDsc->lvParentLcl; + UpdateEarlyRefCount(parentLclNum); + } - if (varDsc->lvIsStructField) - { - // Promoted field, increase count for the parent lclVar. - // - assert(!m_compiler->lvaIsImplicitByRefLocal(lclNum)); - unsigned parentLclNum = varDsc->lvParentLcl; - UpdateEarlyRefCount(parentLclNum); + if (varDsc->lvPromoted) + { + // Promoted struct, increase count for each promoted field. + // + for (unsigned childLclNum = varDsc->lvFieldLclStart; + childLclNum < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++childLclNum) + { + UpdateEarlyRefCount(childLclNum); + } + } + + break; } - if (varDsc->lvPromoted) + case GT_JMP: { - // Promoted struct, increase count for each promoted field. - // - for (unsigned childLclNum = varDsc->lvFieldLclStart; - childLclNum < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++childLclNum) + // GT_JMP has implicit uses of all arguments. + for (unsigned lclNum = 0; lclNum < m_compiler->info.compArgsCount; lclNum++) { - UpdateEarlyRefCount(childLclNum); + UpdateEarlyRefCount(lclNum); } - } - } - if (node->OperIs(GT_JMP)) - { - // GT_JMP has implicit uses of all arguments. - for (unsigned lclNum = 0; lclNum < m_compiler->info.compArgsCount; lclNum++) - { - UpdateEarlyRefCount(lclNum); + break; } } From 5ef4ef2cc8eff0e5646472827d92b06b1e4bb266 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 17 Jan 2023 20:37:19 +0100 Subject: [PATCH 5/6] Check for GT_JMP once per block instead --- src/coreclr/jit/lclmorph.cpp | 83 +++++++++++++++++------------------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index abd5339b01dcc..2645c86ab5fd1 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -569,58 +569,40 @@ class LocalAddressVisitor final : public GenTreeVisitor { GenTree* const node = *use; - switch (node->gtOper) + if (node->OperIs(GT_IND, GT_FIELD, GT_FIELD_ADDR)) { - case GT_IND: - case GT_FIELD: - case GT_FIELD_ADDR: - MorphStructField(node, user); - break; - case GT_LCL_FLD: - MorphLocalField(node, user); - assert(node->OperIsLocal()); - __fallthrough; - case GT_LCL_VAR: - case GT_LCL_VAR_ADDR: - case GT_LCL_FLD_ADDR: - { - unsigned const lclNum = node->AsLclVarCommon()->GetLclNum(); - LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum); + MorphStructField(node, user); + } + else if (node->OperIs(GT_LCL_FLD)) + { + MorphLocalField(node, user); + } - UpdateEarlyRefCount(lclNum); + if (node->OperIsLocal() || node->OperIsLocalAddr()) + { + unsigned const lclNum = node->AsLclVarCommon()->GetLclNum(); + LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum); - if (varDsc->lvIsStructField) - { - // Promoted field, increase count for the parent lclVar. - // - assert(!m_compiler->lvaIsImplicitByRefLocal(lclNum)); - unsigned parentLclNum = varDsc->lvParentLcl; - UpdateEarlyRefCount(parentLclNum); - } + UpdateEarlyRefCount(lclNum); - if (varDsc->lvPromoted) - { - // Promoted struct, increase count for each promoted field. - // - for (unsigned childLclNum = varDsc->lvFieldLclStart; - childLclNum < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++childLclNum) - { - UpdateEarlyRefCount(childLclNum); - } - } - - break; + if (varDsc->lvIsStructField) + { + // Promoted field, increase count for the parent lclVar. + // + assert(!m_compiler->lvaIsImplicitByRefLocal(lclNum)); + unsigned parentLclNum = varDsc->lvParentLcl; + UpdateEarlyRefCount(parentLclNum); } - case GT_JMP: + if (varDsc->lvPromoted) { - // GT_JMP has implicit uses of all arguments. - for (unsigned lclNum = 0; lclNum < m_compiler->info.compArgsCount; lclNum++) + // Promoted struct, increase count for each promoted field. + // + for (unsigned childLclNum = varDsc->lvFieldLclStart; + childLclNum < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++childLclNum) { - UpdateEarlyRefCount(lclNum); + UpdateEarlyRefCount(childLclNum); } - - break; } } @@ -1484,6 +1466,7 @@ class LocalAddressVisitor final : public GenTreeVisitor m_stmtModified |= node->OperIs(GT_LCL_VAR); } +public: //------------------------------------------------------------------------ // UpdateEarlyRefCount: updates the ref count for locals // @@ -1563,6 +1546,7 @@ class LocalAddressVisitor final : public GenTreeVisitor } } +private: //------------------------------------------------------------------------ // IsValidLclAddr: Can the given local address be represented as "LCL_FLD_ADDR"? // @@ -1678,6 +1662,19 @@ PhaseStatus Compiler::fgMarkAddressExposedLocals() visitor.VisitStmt(stmt); } + + // We could check for GT_JMP inside the visitor, but this node is very + // rare so keeping it here avoids pessimizing the hot code. + if (block->endsWithJmpMethod(this)) + { + // GT_JMP has implicit uses of all arguments. + for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++) + { + visitor.UpdateEarlyRefCount(lclNum); + } + + break; + } } madeChanges |= visitor.MadeChanges(); From 0e6a55d36c79c60974dd423d94251a2674e3780d Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 18 Jan 2023 00:39:49 +0100 Subject: [PATCH 6/6] Oops --- src/coreclr/jit/lclmorph.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 2645c86ab5fd1..33da0308c4f1d 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1672,8 +1672,6 @@ PhaseStatus Compiler::fgMarkAddressExposedLocals() { visitor.UpdateEarlyRefCount(lclNum); } - - break; } }