From 7c4ef9f87765d5e921b2bbb5002132ed228e7e4e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 3 Sep 2020 15:45:32 +0300 Subject: [PATCH] Optimize 'x == ""', 'x is ""', 'string.Equals(x, "")' patterns --- src/coreclr/src/jit/gentree.h | 5 +- src/coreclr/src/jit/importer.cpp | 58 +++++++++++++++++++ src/coreclr/src/jit/morph.cpp | 2 + src/coreclr/src/jit/namedintrinsiclist.h | 1 + .../src/System/String.Comparison.cs | 1 + 5 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h index 5c3a94395db6fc..1978f8724ae7e1 100644 --- a/src/coreclr/src/jit/gentree.h +++ b/src/coreclr/src/jit/gentree.h @@ -3577,6 +3577,9 @@ struct GenTreeArgList : public GenTreeOp // TODO-Cleanup: If we could get these accessors used everywhere, then we could switch them. struct GenTreeColon : public GenTreeOp { + unsigned __int64 bbThenNodeFlags; + unsigned __int64 bbElseNodeFlags; + GenTree*& ThenNode() { return gtOp2; @@ -3592,7 +3595,7 @@ struct GenTreeColon : public GenTreeOp } #endif - GenTreeColon(var_types typ, GenTree* thenNode, GenTree* elseNode) : GenTreeOp(GT_COLON, typ, elseNode, thenNode) + GenTreeColon(var_types typ, GenTree* thenNode, GenTree* elseNode) : GenTreeOp(GT_COLON, typ, elseNode, thenNode), bbThenNodeFlags(0), bbElseNodeFlags(0) { } }; diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index f03f639431208e..f6c0a8ea38c6b6 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -4268,6 +4268,57 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, break; } + case NI_System_String_Equals: + { + GenTree* arg0 = impStackTop(1).val; + GenTree* arg1 = impStackTop(0).val; + + GenTreeStrCon* arg0cns = nullptr; + GenTreeStrCon* arg1cns = nullptr; + + if (arg0->OperIs(GT_CNS_STR)) + { + arg0cns = arg0->AsStrCon(); + } + + if (arg1->OperIs(GT_CNS_STR)) + { + arg1cns = arg1->AsStrCon(); + } + + // Convert 'x == ""' to 'x != null && x.Length == 0' + if ((arg0cns != nullptr) ^ (arg1cns != nullptr)) + { + GenTreeStrCon* cnsArg = arg0cns != nullptr ? arg0cns : arg1cns; + GenTree* varArg = arg0cns == nullptr ? arg0 : arg1; + + int length = -1; + info.compCompHnd->getStringLiteral(cnsArg->gtScpHnd, cnsArg->gtSconCPX, &length); + + if (length == 0) + { + GenTree* varArgIsNull = gtNewOperNode(GT_NE, TYP_INT, varArg, gtNewIconNode(0, TYP_REF)); + GenTree* lengthIsZero = gtNewOperNode(GT_EQ, TYP_INT, gtNewArrLen(TYP_INT, gtClone(varArg), + OFFSETOF__CORINFO_String__stringLen, nullptr), gtNewIconNode(0)); + + GenTreeColon* colon = new (this, GT_COLON) GenTreeColon(TYP_INT, lengthIsZero, gtNewIconNode(0)); + + // since colon will be expanded into new basic-blocks we need to keep + // BBF_HAS_IDX_LEN flag in the one ElseNode will be added to. + colon->bbThenNodeFlags |= BBF_HAS_IDX_LEN; + + unsigned tmp = lvaGrabTemp(true DEBUGARG("spilling string.Equals(x, \"\") tree ")); + impAssignTempGen(tmp, gtNewQmarkNode(TYP_INT, varArgIsNull, colon), (unsigned)CHECK_SPILL_NONE); + retNode = gtNewLclvNode(tmp, TYP_INT); + + impPopStack(); + impPopStack(); + break; + } + } + break; + } + case NI_System_Buffers_Binary_BinaryPrimitives_ReverseEndianness: { assert(sig->numArgs == 1); @@ -4623,6 +4674,13 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) result = NI_System_GC_KeepAlive; } } + else if (strcmp(className, "String") == 0) + { + if (strcmp(methodName, "Equals") == 0) + { + result = NI_System_String_Equals; + } + } else if (strcmp(className, "Type") == 0) { if (strcmp(methodName, "get_IsValueType") == 0) diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 5430a2b02a96f4..bba3a4a56b241b 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -17367,6 +17367,7 @@ void Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt) } Statement* trueStmt = fgNewStmtFromTree(trueExpr, stmt->GetILOffsetX()); fgInsertStmtAtEnd(thenBlock, trueStmt); + thenBlock->bbFlags |= qmark->gtGetOp2()->AsColon()->bbThenNodeFlags; } // Assign the falseExpr into the dst or tmp, insert in elseBlock @@ -17378,6 +17379,7 @@ void Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt) } Statement* falseStmt = fgNewStmtFromTree(falseExpr, stmt->GetILOffsetX()); fgInsertStmtAtEnd(elseBlock, falseStmt); + elseBlock->bbFlags |= qmark->gtGetOp2()->AsColon()->bbElseNodeFlags; } #ifdef DEBUG diff --git a/src/coreclr/src/jit/namedintrinsiclist.h b/src/coreclr/src/jit/namedintrinsiclist.h index 8ab264e36061c4..d3e3250ebf64df 100644 --- a/src/coreclr/src/jit/namedintrinsiclist.h +++ b/src/coreclr/src/jit/namedintrinsiclist.h @@ -37,6 +37,7 @@ enum NamedIntrinsic : unsigned short NI_System_Collections_Generic_EqualityComparer_get_Default, NI_System_Buffers_Binary_BinaryPrimitives_ReverseEndianness, NI_System_GC_KeepAlive, + NI_System_String_Equals, NI_System_Type_get_IsValueType, NI_System_Type_IsAssignableFrom, NI_System_Type_IsAssignableTo, diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs b/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs index c616ae9e28a8ed..7e03b18d457d2a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs @@ -676,6 +676,7 @@ public bool Equals(string? value, StringComparison comparisonType) } // Determines whether two Strings match. + [Intrinsic] public static bool Equals(string? a, string? b) { if (object.ReferenceEquals(a, b))