Optimize 'x == ""', 'x is ""', 'string.Equals(x, "")' patterns

EgorBo · Sep 3, 2020 · 7c4ef9f · 7c4ef9f
1 parent 1789775
commit 7c4ef9f
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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))