From 817bd544ddf684926ddd0df4ff01e4f4b163d142 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 18 Feb 2023 22:01:40 +0100 Subject: [PATCH 1/5] Allow some intrinsics in Tier0 #2 --- src/coreclr/jit/importercalls.cpp | 47 ++++++++++++++++++++++++++++++- src/coreclr/jit/lower.cpp | 16 +++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index b2a2d46892aafa..c57ecc3c71952e 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2597,6 +2597,45 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, break; } + // Allow some lighweight intrinsics in Tier0 which can improve throughput + // we introduced betterToExpand here because we're fine if intrinsic decides to not expand itself + // in this case unlike mustExpand. + bool betterToExpand = false; + + // NOTE: MinOpts() is always true for Tier0 so we have to check explicit flags instead. + // To be fixed in https://github.com/dotnet/runtime/pull/77465 + const bool tier0opts = !opts.compDbgCode && !opts.jitFlags->IsSet(JitFlags::JIT_FLAG_MIN_OPT); + + if (!mustExpand && tier0opts) + { + switch (ni) + { + // This one is just `return true/false` + case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant: + + // We need these to be able to fold "typeof(...) == typeof(...)" + case NI_System_RuntimeTypeHandle_GetValueInternal: + case NI_System_Type_GetTypeFromHandle: + case NI_System_Type_op_Equality: + case NI_System_Type_op_Inequality: + + // Simple cases + case NI_System_String_get_Chars: + case NI_System_String_get_Length: + case NI_System_Span_get_Item: + case NI_System_Span_get_Length: + case NI_System_ReadOnlySpan_get_Item: + case NI_System_ReadOnlySpan_get_Length: + betterToExpand = true; + break; + + default: + // Unsafe.* are all small enough to prefer expansions. + betterToExpand = ni >= NI_SRCS_UNSAFE_START && ni <= NI_SRCS_UNSAFE_END; + break; + } + } + GenTree* retNode = nullptr; // Under debug and minopts, only expand what is required. @@ -2604,7 +2643,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, // If that call is an intrinsic and is expanded, codegen for NextCallReturnAddress will fail. // To avoid that we conservatively expand only required intrinsics in methods that call // the NextCallReturnAddress intrinsic. - if (!mustExpand && (opts.OptimizationDisabled() || info.compHasNextCallRetAddr)) + if (!mustExpand && ((opts.OptimizationDisabled() && !betterToExpand) || info.compHasNextCallRetAddr)) { *pIntrinsicName = NI_Illegal; return retNode; @@ -2715,6 +2754,12 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, JITDUMP("\nExpanding RuntimeHelpers.IsKnownConstant to true early\n"); // We can also consider FTN_ADDR here } + else if (opts.OptimizationDisabled()) + { + // It doesn't make sense to carry it as GT_INTRINSIC till Morph in Tier0 + retNode = gtNewIconNode(0); + JITDUMP("\nExpanding RuntimeHelpers.IsKnownConstant to false early\n"); + } else { // op1 is not a known constant, we'll do the expansion in morph diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index c5a895a4ebccea..baa1df21e9ae65 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3242,6 +3242,22 @@ GenTree* Lowering::LowerCompare(GenTree* cmp) // GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) { + if (jtrue->gtGetOp1()->IsIntegralConst()) + { + // JTRUE(0/1) is possible for Tier0 here where we don't do BB optimizations and if some + // post-importer phase (e.g. Global Morph) manages to fold a comparison we might end up + // with this. Create a fake comparison "cns != 0" and lower it as usual. + GenTree* cns = jtrue->gtGetOp1(); + assert(cns->IsIntegralConst(0) || cns->IsIntegralConst(1)); + assert(comp->opts.OptimizationDisabled()); + + GenTree* zero = comp->gtNewIconNode(0, cns->TypeGet()); + GenTree* cmp = comp->gtNewOperNode(GT_NE, TYP_INT, cns, zero); + BlockRange().InsertBefore(jtrue, zero); + BlockRange().InsertBefore(jtrue, cmp); + jtrue->gtOp1 = cmp; + } + assert(jtrue->gtGetOp1()->OperIsCompare()); assert(jtrue->gtGetOp1()->gtNext == jtrue); GenTreeOp* relop = jtrue->gtGetOp1()->AsOp(); From f171c31e92ffdfe4b7d9602d09ed26a9b98a7a1c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 19 Feb 2023 11:03:28 +0100 Subject: [PATCH 2/5] Address feedback --- src/coreclr/jit/importer.cpp | 3 +++ src/coreclr/jit/lower.cpp | 16 ---------------- src/coreclr/jit/morph.cpp | 10 ++++++---- 3 files changed, 9 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index dc327ff6fd4267..4565f9bc134be8 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -7827,6 +7827,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1->gtFlags |= GTF_RELOP_NAN_UN | GTF_UNSIGNED; } + // See if we can optimize type comparisons + op1 = gtFoldTypeCompare(op1); + // Fold result, if possible. op1 = gtFoldExpr(op1); diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index baa1df21e9ae65..c5a895a4ebccea 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3242,22 +3242,6 @@ GenTree* Lowering::LowerCompare(GenTree* cmp) // GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) { - if (jtrue->gtGetOp1()->IsIntegralConst()) - { - // JTRUE(0/1) is possible for Tier0 here where we don't do BB optimizations and if some - // post-importer phase (e.g. Global Morph) manages to fold a comparison we might end up - // with this. Create a fake comparison "cns != 0" and lower it as usual. - GenTree* cns = jtrue->gtGetOp1(); - assert(cns->IsIntegralConst(0) || cns->IsIntegralConst(1)); - assert(comp->opts.OptimizationDisabled()); - - GenTree* zero = comp->gtNewIconNode(0, cns->TypeGet()); - GenTree* cmp = comp->gtNewOperNode(GT_NE, TYP_INT, cns, zero); - BlockRange().InsertBefore(jtrue, zero); - BlockRange().InsertBefore(jtrue, cmp); - jtrue->gtOp1 = cmp; - } - assert(jtrue->gtGetOp1()->OperIsCompare()); assert(jtrue->gtGetOp1()->gtNext == jtrue); GenTreeOp* relop = jtrue->gtGetOp1()->AsOp(); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index a34552aa1f1972..15521b03761be9 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9014,11 +9014,13 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA case GT_EQ: case GT_NE: { - GenTree* optimizedTree = gtFoldTypeCompare(tree); - - if (optimizedTree != tree) + if (opts.OptimizationEnabled()) { - return fgMorphTree(optimizedTree); + GenTree* optimizedTree = gtFoldTypeCompare(tree); + if (optimizedTree != tree) + { + return fgMorphTree(optimizedTree); + } } // Pattern-matching optimization: From 8e59f1289bbd2b126bc7466c0bb464abba11927b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 19 Feb 2023 17:52:10 +0100 Subject: [PATCH 3/5] fix tier1 tp regressions --- src/coreclr/jit/importer.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 4565f9bc134be8..08c8f2a31f7d69 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -7827,8 +7827,11 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1->gtFlags |= GTF_RELOP_NAN_UN | GTF_UNSIGNED; } - // See if we can optimize type comparisons - op1 = gtFoldTypeCompare(op1); + // See if we can optimize type comparisons, IsCall() checks for better TP + if (op1->gtGetOp1()->IsCall() && op1->gtGetOp2()->IsCall()) + { + op1 = gtFoldTypeCompare(op1); + } // Fold result, if possible. op1 = gtFoldExpr(op1); From 24e24c9c926bfa5d6ce5183c612f869c6ae37321 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sun, 19 Feb 2023 20:54:07 +0100 Subject: [PATCH 4/5] Update importer.cpp --- src/coreclr/jit/importer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 08c8f2a31f7d69..274ec59767ab8a 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -7827,8 +7827,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1->gtFlags |= GTF_RELOP_NAN_UN | GTF_UNSIGNED; } - // See if we can optimize type comparisons, IsCall() checks for better TP - if (op1->gtGetOp1()->IsCall() && op1->gtGetOp2()->IsCall()) + // See if we can optimize type comparisons, gtIsTypeof() checks for better TP + if (gtIsTypeof(op1->gtGetOp1()) && gtIsTypeof(op1->gtGetOp2())) { op1 = gtFoldTypeCompare(op1); } From 0f5ff19351d56f7046f39ea41d767dad7d5396b7 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sun, 19 Feb 2023 23:22:03 +0100 Subject: [PATCH 5/5] Update importer.cpp --- src/coreclr/jit/importer.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 274ec59767ab8a..dc327ff6fd4267 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -7827,12 +7827,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1->gtFlags |= GTF_RELOP_NAN_UN | GTF_UNSIGNED; } - // See if we can optimize type comparisons, gtIsTypeof() checks for better TP - if (gtIsTypeof(op1->gtGetOp1()) && gtIsTypeof(op1->gtGetOp2())) - { - op1 = gtFoldTypeCompare(op1); - } - // Fold result, if possible. op1 = gtFoldExpr(op1);