From b85d02a790537f6201cc6354f25912ee6df7e19c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 16 Jan 2020 17:27:04 +0300 Subject: [PATCH 1/9] Optimize `box + isinst + unbox.any` as `nop` --- src/coreclr/src/jit/importer.cpp | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 132cf2bf082e47..1f3c1404c14bc2 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -5902,13 +5902,13 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, const B break; case CEE_ISINST: - // box + isinst + br_true/false if (codeAddr + 1 + sizeof(mdToken) + 1 <= codeEndp) { const BYTE* nextCodeAddr = codeAddr + 1 + sizeof(mdToken); switch (nextCodeAddr[0]) { + // box + isinst + br_true/false case CEE_BRTRUE: case CEE_BRTRUE_S: case CEE_BRFALSE: @@ -5941,6 +5941,27 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, const B } } } + break; + + // box + isinst + unbox.any + case CEE_UNBOX_ANY: + if ((nextCodeAddr + 5) <= codeEndp) + { + CORINFO_RESOLVED_TOKEN unboxResolvedToken = {}; + impResolveToken(codeAddr + 1, &unboxResolvedToken, CORINFO_TOKENKIND_Class); + + // See if the resolved tokens describe types that are equal. + const TypeCompareState compare = + info.compCompHnd->compareTypesForEquality(unboxResolvedToken.hClass, pResolvedToken->hClass); + + // If so, box/unbox.any is a nop. + if (compare == TypeCompareState::Must) + { + JITDUMP("\n Importing BOX; ISINST, UNBOX.ANY as NOP\n"); + return 6 + sizeof(mdToken); + } + } + break; } } break; From 017eb8c4b7349e07f9d478078badd7c5a5e1f109 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 16 Jan 2020 20:11:54 +0300 Subject: [PATCH 2/9] Formatting --- src/coreclr/src/jit/importer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 1f3c1404c14bc2..0e3fe11dbc40d4 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -5952,7 +5952,8 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, const B // See if the resolved tokens describe types that are equal. const TypeCompareState compare = - info.compCompHnd->compareTypesForEquality(unboxResolvedToken.hClass, pResolvedToken->hClass); + info.compCompHnd->compareTypesForEquality(unboxResolvedToken.hClass, + pResolvedToken->hClass); // If so, box/unbox.any is a nop. if (compare == TypeCompareState::Must) From 8eb9f803042bf4a097d7a18cf0b1347931a2c0b0 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 16 Jan 2020 22:27:21 +0300 Subject: [PATCH 3/9] Address feedback --- src/coreclr/src/jit/importer.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 0e3fe11dbc40d4..3e4b67ee0a3fe9 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -5945,21 +5945,28 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, const B // box + isinst + unbox.any case CEE_UNBOX_ANY: - if ((nextCodeAddr + 5) <= codeEndp) + if ((nextCodeAddr + 1 + sizeof(mdToken)) <= codeEndp) { + CORINFO_RESOLVED_TOKEN isinstResolvedToken = {}; + impResolveToken(codeAddr + 1, &isinstResolvedToken, CORINFO_TOKENKIND_Class); + CORINFO_RESOLVED_TOKEN unboxResolvedToken = {}; - impResolveToken(codeAddr + 1, &unboxResolvedToken, CORINFO_TOKENKIND_Class); + impResolveToken(nextCodeAddr + 1, &unboxResolvedToken, CORINFO_TOKENKIND_Class); // See if the resolved tokens describe types that are equal. - const TypeCompareState compare = + const TypeCompareState compareBoxIsInst = + info.compCompHnd->compareTypesForEquality(isinstResolvedToken.hClass, + pResolvedToken->hClass); + const TypeCompareState compareBoxUnbox = info.compCompHnd->compareTypesForEquality(unboxResolvedToken.hClass, pResolvedToken->hClass); // If so, box/unbox.any is a nop. - if (compare == TypeCompareState::Must) + if (compareBoxIsInst == TypeCompareState::Must && + compareBoxUnbox == TypeCompareState::Must) { JITDUMP("\n Importing BOX; ISINST, UNBOX.ANY as NOP\n"); - return 6 + sizeof(mdToken); + return 2 + sizeof(mdToken) * 2; } } break; From ffdc2e1674067afafcdd08796ad2ca9b8d22dc7e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 17 Jan 2020 00:52:10 +0300 Subject: [PATCH 4/9] Add tests --- src/coreclr/src/jit/importer.cpp | 3 +- .../Boxing/box_isinst_unbox-noinline.csproj | 20 ++ .../Conversions/Boxing/box_isinst_unbox.cs | 237 ++++++++++++++++++ .../Boxing/box_isinst_unbox.csproj | 10 + 4 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj create mode 100644 src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.cs create mode 100644 src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.csproj diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 3e4b67ee0a3fe9..73e43cafda77c6 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -5962,8 +5962,7 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, const B pResolvedToken->hClass); // If so, box/unbox.any is a nop. - if (compareBoxIsInst == TypeCompareState::Must && - compareBoxUnbox == TypeCompareState::Must) + if (compareBoxIsInst == TypeCompareState::Must && compareBoxUnbox == TypeCompareState::Must) { JITDUMP("\n Importing BOX; ISINST, UNBOX.ANY as NOP\n"); return 2 + sizeof(mdToken) * 2; diff --git a/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj new file mode 100644 index 00000000000000..758180f61372b8 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj @@ -0,0 +1,20 @@ + + + Exe + None + True + + + + + + + + + diff --git a/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.cs b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.cs new file mode 100644 index 00000000000000..ef56f931667dd0 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.cs @@ -0,0 +1,237 @@ +// 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.Collections.Generic; +using System.Runtime.CompilerServices; + +public static class Tests +{ + private static int returnCode = 100; + + public static int BoxIsInstUnbox1(T t) => t is int n ? n : -1; + public static int BoxIsInstUnbox2(T t) => t is string n ? n.Length : -1; + public static int BoxIsInstUnbox3(T t) => t is Struct1 n ? n.a : -1; + public static int BoxIsInstUnbox4(T t) => t is Struct1 n ? n.GetHashCode() : -1; + public static int BoxIsInstUnbox5(T t) => t is Class1 n ? n.a : -1; + public static int BoxIsInstUnbox6(T t) => t is RefBase n ? n.a : -1; + public static int BoxIsInstUnbox7(T t) => t is object[] n ? n.Length : -1; + + public static void Expect(this int actual, int expected, [CallerLineNumber] int line = 0) + { + if (expected != actual) + { + Console.WriteLine($"{actual} != {expected}, line {line}."); + returnCode++; + } + } + + public static int Main() + { + BoxIsInstUnbox1(1).Expect(1); + BoxIsInstUnbox1(1).Expect(-1); + BoxIsInstUnbox1(1).Expect(-1); + BoxIsInstUnbox1(1).Expect(-1); + BoxIsInstUnbox1(1).Expect(-1); + BoxIsInstUnbox1(1).Expect(1); + BoxIsInstUnbox1(null).Expect(-1); + BoxIsInstUnbox1(1).Expect(-1); + BoxIsInstUnbox1("1").Expect(-1); + BoxIsInstUnbox1(1.ToString()).Expect(-1); + BoxIsInstUnbox1(null).Expect(-1); + BoxIsInstUnbox1>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1?>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1>(new Struct2()).Expect(-1); + BoxIsInstUnbox1>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox1(new string[1]).Expect(-1); + BoxIsInstUnbox1(new string[1]).Expect(-1); + BoxIsInstUnbox1>(new string[1]).Expect(-1); + + BoxIsInstUnbox2(1).Expect(-1); + BoxIsInstUnbox2(1).Expect(-1); + BoxIsInstUnbox2(1).Expect(-1); + BoxIsInstUnbox2(1).Expect(-1); + BoxIsInstUnbox2(1).Expect(-1); + BoxIsInstUnbox2(1).Expect(-1); + BoxIsInstUnbox2(null).Expect(-1); + BoxIsInstUnbox2(1).Expect(-1); + BoxIsInstUnbox2("1").Expect(1); + BoxIsInstUnbox2(1.ToString()).Expect(1); + BoxIsInstUnbox2(null).Expect(-1); + BoxIsInstUnbox2>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2?>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2>(new Struct2()).Expect(-1); + BoxIsInstUnbox2>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox2(new string[1]).Expect(-1); + BoxIsInstUnbox2(new string[1]).Expect(-1); + BoxIsInstUnbox2>(new string[1]).Expect(-1); + + BoxIsInstUnbox3(1).Expect(-1); + BoxIsInstUnbox3(1).Expect(-1); + BoxIsInstUnbox3(1).Expect(-1); + BoxIsInstUnbox3(1).Expect(-1); + BoxIsInstUnbox3(1).Expect(-1); + BoxIsInstUnbox3(1).Expect(-1); + BoxIsInstUnbox3(null).Expect(-1); + BoxIsInstUnbox3(1).Expect(-1); + BoxIsInstUnbox3("1").Expect(-1); + BoxIsInstUnbox3(1.ToString()).Expect(-1); + BoxIsInstUnbox3(null).Expect(-1); + BoxIsInstUnbox3>(new Struct1 { a = 1 }).Expect(1); + BoxIsInstUnbox3?>(new Struct1 { a = 1 }).Expect(1); + BoxIsInstUnbox3>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox3>(new Struct2()).Expect(-1); + BoxIsInstUnbox3>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox3>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox3>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox3>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox3>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox3>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox3>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox3(new string[1]).Expect(-1); + BoxIsInstUnbox3(new string[1]).Expect(-1); + BoxIsInstUnbox3>(new string[1]).Expect(-1); + + BoxIsInstUnbox4(1).Expect(-1); + BoxIsInstUnbox4(1).Expect(-1); + BoxIsInstUnbox4(1).Expect(-1); + BoxIsInstUnbox4(1).Expect(-1); + BoxIsInstUnbox4(1).Expect(-1); + BoxIsInstUnbox4(1).Expect(-1); + BoxIsInstUnbox4(null).Expect(-1); + BoxIsInstUnbox4(1).Expect(-1); + BoxIsInstUnbox4("1").Expect(-1); + BoxIsInstUnbox4(1.ToString()).Expect(-1); + BoxIsInstUnbox4(null).Expect(-1); + BoxIsInstUnbox4>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4?>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4>(new Struct2()).Expect(-1); + BoxIsInstUnbox4>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox4(new string[1]).Expect(-1); + BoxIsInstUnbox4(new string[1]).Expect(-1); + BoxIsInstUnbox4>(new string[1]).Expect(-1); + + BoxIsInstUnbox5(1).Expect(-1); + BoxIsInstUnbox5(1).Expect(-1); + BoxIsInstUnbox5(1).Expect(-1); + BoxIsInstUnbox5(1).Expect(-1); + BoxIsInstUnbox5(1).Expect(-1); + BoxIsInstUnbox5(1).Expect(-1); + BoxIsInstUnbox5(null).Expect(-1); + BoxIsInstUnbox5(1).Expect(-1); + BoxIsInstUnbox5("1").Expect(-1); + BoxIsInstUnbox5(1.ToString()).Expect(-1); + BoxIsInstUnbox5(null).Expect(-1); + BoxIsInstUnbox5>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox5?>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox5>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox5>(new Struct2()).Expect(-1); + BoxIsInstUnbox5>(new Class1 { a = 1 }).Expect(1); + BoxIsInstUnbox5>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox5>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox5>(new Class1 { a = 1 }).Expect(1); + BoxIsInstUnbox5>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox5>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox5>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox5(new string[1]).Expect(-1); + BoxIsInstUnbox5(new string[1]).Expect(-1); + BoxIsInstUnbox5>(new string[1]).Expect(-1); + + BoxIsInstUnbox6(1).Expect(-1); + BoxIsInstUnbox6(1).Expect(-1); + BoxIsInstUnbox6(1).Expect(-1); + BoxIsInstUnbox6(1).Expect(-1); + BoxIsInstUnbox6(1).Expect(-1); + BoxIsInstUnbox6(1).Expect(-1); + BoxIsInstUnbox6(null).Expect(-1); + BoxIsInstUnbox6(1).Expect(-1); + BoxIsInstUnbox6("1").Expect(-1); + BoxIsInstUnbox6(1.ToString()).Expect(-1); + BoxIsInstUnbox6(null).Expect(-1); + BoxIsInstUnbox6>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox6?>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox6>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox6>(new Struct2()).Expect(-1); + BoxIsInstUnbox6>(new Class1 { a = 1 }).Expect(1); + BoxIsInstUnbox6>(new Class1 { a = 1 }).Expect(1); + BoxIsInstUnbox6>(new Class1 { a = 1 }).Expect(1); + BoxIsInstUnbox6>(new Class1 { a = 1 }).Expect(1); + BoxIsInstUnbox6>(new Class1 { a = 1 }).Expect(1); + BoxIsInstUnbox6>(new Class1 { a = 1 }).Expect(1); + BoxIsInstUnbox6>(new Class1 { a = 1 }).Expect(1); + BoxIsInstUnbox6(new string[1]).Expect(-1); + BoxIsInstUnbox6(new string[1]).Expect(-1); + BoxIsInstUnbox6>(new string[1]).Expect(-1); + + BoxIsInstUnbox7(1).Expect(-1); + BoxIsInstUnbox7(1).Expect(-1); + BoxIsInstUnbox7(1).Expect(-1); + BoxIsInstUnbox7(1).Expect(-1); + BoxIsInstUnbox7(1).Expect(-1); + BoxIsInstUnbox7(1).Expect(-1); + BoxIsInstUnbox7(null).Expect(-1); + BoxIsInstUnbox7(1).Expect(-1); + BoxIsInstUnbox7("1").Expect(-1); + BoxIsInstUnbox7(1.ToString()).Expect(-1); + BoxIsInstUnbox7(null).Expect(-1); + BoxIsInstUnbox7>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7?>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7>(new Struct1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7>(new Struct2()).Expect(-1); + BoxIsInstUnbox7>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7>(new Class1 { a = 1 }).Expect(-1); + BoxIsInstUnbox7(new string[1]).Expect(1); + BoxIsInstUnbox7(new string[1]).Expect(1); + BoxIsInstUnbox7>(new string[1]).Expect(1); + + return returnCode; + } +} + +public struct Struct1 +{ + public T a; +} + +public struct Struct2 +{ + public T a; +} + +public class RefBase : IDisposable +{ + public int a; + public void Dispose() { } +} + +public class Class1 : RefBase +{ + public T b; +} diff --git a/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.csproj b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.csproj new file mode 100644 index 00000000000000..1100f420532dc8 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.csproj @@ -0,0 +1,10 @@ + + + Exe + None + True + + + + + From 8c801146088ce84ddaf8b4bd0100780976c45942 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 17 Jan 2020 01:58:17 +0300 Subject: [PATCH 5/9] Fix CI errors --- .../Conversions/Boxing/box_isinst_unbox-noinline.csproj | 2 +- .../src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.csproj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj index 758180f61372b8..82b3f06cd0a5d0 100644 --- a/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj +++ b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj @@ -5,7 +5,7 @@ True - + True - + From 3cf7d5c32c0eedd77e79453d628c2e8a58791ee9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 18 Jan 2020 01:37:10 +0300 Subject: [PATCH 6/9] Address feedback --- src/coreclr/src/jit/importer.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 73e43cafda77c6..7bb4cd5b01ec8f 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -5947,21 +5947,21 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, const B case CEE_UNBOX_ANY: if ((nextCodeAddr + 1 + sizeof(mdToken)) <= codeEndp) { + // See if the resolved tokens in box, isinst and unbox.any describe types that are equal. CORINFO_RESOLVED_TOKEN isinstResolvedToken = {}; impResolveToken(codeAddr + 1, &isinstResolvedToken, CORINFO_TOKENKIND_Class); - CORINFO_RESOLVED_TOKEN unboxResolvedToken = {}; - impResolveToken(nextCodeAddr + 1, &unboxResolvedToken, CORINFO_TOKENKIND_Class); - - // See if the resolved tokens describe types that are equal. const TypeCompareState compareBoxIsInst = info.compCompHnd->compareTypesForEquality(isinstResolvedToken.hClass, pResolvedToken->hClass); + CORINFO_RESOLVED_TOKEN unboxResolvedToken = {}; + impResolveToken(nextCodeAddr + 1, &unboxResolvedToken, CORINFO_TOKENKIND_Class); + const TypeCompareState compareBoxUnbox = info.compCompHnd->compareTypesForEquality(unboxResolvedToken.hClass, pResolvedToken->hClass); - // If so, box/unbox.any is a nop. + // If so, box + isinst + unbox.any is a nop. if (compareBoxIsInst == TypeCompareState::Must && compareBoxUnbox == TypeCompareState::Must) { JITDUMP("\n Importing BOX; ISINST, UNBOX.ANY as NOP\n"); From 9b6726f6ff32bd805e83cee2dbfe1621a4546343 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 1 Feb 2020 16:35:59 +0300 Subject: [PATCH 7/9] Address feedback --- src/coreclr/src/jit/importer.cpp | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 9606707cc5b5cb..805815c377f970 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -5953,21 +5953,19 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, const B CORINFO_RESOLVED_TOKEN isinstResolvedToken = {}; impResolveToken(codeAddr + 1, &isinstResolvedToken, CORINFO_TOKENKIND_Class); - const TypeCompareState compareBoxIsInst = - info.compCompHnd->compareTypesForEquality(isinstResolvedToken.hClass, - pResolvedToken->hClass); - CORINFO_RESOLVED_TOKEN unboxResolvedToken = {}; - impResolveToken(nextCodeAddr + 1, &unboxResolvedToken, CORINFO_TOKENKIND_Class); - - const TypeCompareState compareBoxUnbox = - info.compCompHnd->compareTypesForEquality(unboxResolvedToken.hClass, - pResolvedToken->hClass); - - // If so, box + isinst + unbox.any is a nop. - if (compareBoxIsInst == TypeCompareState::Must && compareBoxUnbox == TypeCompareState::Must) + if (info.compCompHnd->compareTypesForEquality(isinstResolvedToken.hClass, + pResolvedToken->hClass) == TypeCompareState::Must) { - JITDUMP("\n Importing BOX; ISINST, UNBOX.ANY as NOP\n"); - return 2 + sizeof(mdToken) * 2; + CORINFO_RESOLVED_TOKEN unboxResolvedToken = {}; + impResolveToken(nextCodeAddr + 1, &unboxResolvedToken, CORINFO_TOKENKIND_Class); + + // If so, box + isinst + unbox.any is a nop. + if (info.compCompHnd->compareTypesForEquality(unboxResolvedToken.hClass, + pResolvedToken->hClass) == TypeCompareState::Must) + { + JITDUMP("\n Importing BOX; ISINST, UNBOX.ANY as NOP\n"); + return 2 + sizeof(mdToken) * 2; + } } } break; From 4e36642201352a18865242ea5be32cf4ee8dca38 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 1 Feb 2020 16:39:28 +0300 Subject: [PATCH 8/9] Remove box_isinst_unbox-noinline.csproj --- .../Boxing/box_isinst_unbox-noinline.csproj | 20 ------------------- .../Conversions/Boxing/box_isinst_unbox.cs | 13 ++++++++++++ 2 files changed, 13 insertions(+), 20 deletions(-) delete mode 100644 src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj diff --git a/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj deleted file mode 100644 index 82b3f06cd0a5d0..00000000000000 --- a/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox-noinline.csproj +++ /dev/null @@ -1,20 +0,0 @@ - - - Exe - None - True - - - - - - - - - diff --git a/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.cs b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.cs index ef56f931667dd0..49bb899f9d4a59 100644 --- a/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.cs +++ b/src/coreclr/tests/src/JIT/Generics/Conversions/Boxing/box_isinst_unbox.cs @@ -10,12 +10,25 @@ public static class Tests { private static int returnCode = 100; + [MethodImpl(MethodImplOptions.NoInlining)] public static int BoxIsInstUnbox1(T t) => t is int n ? n : -1; + + [MethodImpl(MethodImplOptions.NoInlining)] public static int BoxIsInstUnbox2(T t) => t is string n ? n.Length : -1; + + [MethodImpl(MethodImplOptions.NoInlining)] public static int BoxIsInstUnbox3(T t) => t is Struct1 n ? n.a : -1; + + [MethodImpl(MethodImplOptions.NoInlining)] public static int BoxIsInstUnbox4(T t) => t is Struct1 n ? n.GetHashCode() : -1; + + [MethodImpl(MethodImplOptions.NoInlining)] public static int BoxIsInstUnbox5(T t) => t is Class1 n ? n.a : -1; + + [MethodImpl(MethodImplOptions.NoInlining)] public static int BoxIsInstUnbox6(T t) => t is RefBase n ? n.a : -1; + + [MethodImpl(MethodImplOptions.NoInlining)] public static int BoxIsInstUnbox7(T t) => t is object[] n ? n.Length : -1; public static void Expect(this int actual, int expected, [CallerLineNumber] int line = 0) From 8040f7d51a55df1e54862b69d4321553f10d40e7 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 1 Feb 2020 16:58:12 +0300 Subject: [PATCH 9/9] Fix Formatting --- src/coreclr/src/jit/importer.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 805815c377f970..5e475ae2b7d368 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -5954,14 +5954,16 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, const B impResolveToken(codeAddr + 1, &isinstResolvedToken, CORINFO_TOKENKIND_Class); if (info.compCompHnd->compareTypesForEquality(isinstResolvedToken.hClass, - pResolvedToken->hClass) == TypeCompareState::Must) + pResolvedToken->hClass) == + TypeCompareState::Must) { CORINFO_RESOLVED_TOKEN unboxResolvedToken = {}; impResolveToken(nextCodeAddr + 1, &unboxResolvedToken, CORINFO_TOKENKIND_Class); // If so, box + isinst + unbox.any is a nop. if (info.compCompHnd->compareTypesForEquality(unboxResolvedToken.hClass, - pResolvedToken->hClass) == TypeCompareState::Must) + pResolvedToken->hClass) == + TypeCompareState::Must) { JITDUMP("\n Importing BOX; ISINST, UNBOX.ANY as NOP\n"); return 2 + sizeof(mdToken) * 2;