Skip to content

Commit

Permalink
Optimize string.Equals(str, "True", StringComparison.Ordinal)
Browse files Browse the repository at this point in the history
  • Loading branch information
EgorBo committed Dec 5, 2020
1 parent dc0e11d commit 5883362
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 37 deletions.
4 changes: 3 additions & 1 deletion src/coreclr/src/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -3582,6 +3582,8 @@ struct GenTreeArgList : public GenTreeOp
// TODO-Cleanup: If we could get these accessors used everywhere, then we could switch them.
struct GenTreeColon : public GenTreeOp
{
unsigned bbTrueFlags;

GenTree*& ThenNode()
{
return gtOp2;
Expand All @@ -3597,7 +3599,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), bbTrueFlags(0)
{
}
};
Expand Down
81 changes: 81 additions & 0 deletions src/coreclr/src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4291,6 +4291,80 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
break;
}

case NI_System_String_Equals:
{
if (!strcmp(info.compMethodName, "Test") && sig->numArgs == 3)
{
GenTree* str = impStackTop(2).val;
GenTree* cnsStr = impStackTop(1).val;
GenTree* mode = impStackTop(0).val;

if (str->OperIs(GT_CNS_STR) || !cnsStr->OperIs(GT_CNS_STR) || !mode->IsIntegralConst(4))
{
break;
}
// TODO: handle case when strCon is on the left
// TODO: OrdinalIgnoreCase support

int cnsLength = 0;
LPCWSTR strContent = info.compCompHnd->getStringLiteral(cnsStr->AsStrCon()->gtScpHnd,
cnsStr->AsStrCon()->gtSconCPX, &cnsLength);

if ((strContent == nullptr) || (cnsLength < 0) || (cnsLength > 4))
{
break;
}

ULONG strAsUlong = 0x4242;
for (int i = 0; i < cnsLength; i++)
{
// TODO: check is ASCII
//strAsUlong |= strContent[i] << xxx
}

// qmark1
// / \
// str!=null colon1
// / \
// 0 qmark2
// / \
// str.Length==cnsLength colone2
// / \
// 0 *(&str+12)==strAsUlong


// first, let's spawn two temp variables for `str` local
GenTree* strClone;
GenTree* strClone2;
str = impCloneExpr(str, &strClone, NO_CLASS_HANDLE, (unsigned)CHECK_SPILL_ALL, nullptr DEBUGARG("STREQ tmp1"));
strClone = impCloneExpr(strClone, &strClone2, NO_CLASS_HANDLE, (unsigned)CHECK_SPILL_ALL, nullptr DEBUGARG("STREQ tmp2"));

// Unsafe.ReadUnaligned<ulong>(ref Unsafe.As<char, byte>(ref str._firstChar)) == 0x.....
GenTree* indirCmp = gtNewOperNode(GT_EQ, TYP_INT,
gtNewIndir(TYP_LONG,
gtNewOperNode(GT_ADD, TYP_BYREF, strClone2,
gtNewIconNode(12, TYP_LONG))), // 12 is an offset of `_firstChar` field
gtNewIconNode(strAsUlong, TYP_LONG));

GenTreeArrLen* strLen = gtNewArrLen(TYP_INT, str, OFFSETOF__CORINFO_String__stringLen, compCurBB);
GenTreeColon* colon2 = new(this, GT_COLON) GenTreeColon(TYP_INT, indirCmp, gtNewIconNode(0));
GenTreeQmark* qmark2 = gtNewQmarkNode(TYP_INT, gtNewOperNode(GT_EQ, TYP_INT, strLen, gtNewIconNode(cnsLength)), colon2);
GenTreeColon* colon1 = new (this, GT_COLON) GenTreeColon(TYP_INT, qmark2, gtNewIconNode(0));
// QMark expander must set this flag to a block it will create:
colon1->bbTrueFlags = BBF_HAS_IDX_LEN;
GenTreeQmark* qmark1 = gtNewQmarkNode(TYP_INT, gtNewOperNode(GT_NE, TYP_INT, strClone, gtNewIconNode(0, TYP_REF)), colon1);

// One more temp, qmark can't be a root
unsigned tmp = lvaGrabTemp(true DEBUGARG("spilling STREQ root qmark"));
impAssignTempGen(tmp, qmark1, (unsigned)CHECK_SPILL_NONE);
retNode = gtNewLclvNode(tmp, TYP_INT);
impPopStack();
impPopStack();
impPopStack();
}
break;
}

case NI_System_Type_get_IsValueType:
{
// Optimize
Expand Down Expand Up @@ -4827,6 +4901,13 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
result = NI_System_Type_IsAssignableTo;
}
}
else if (strcmp(className, "String") == 0)
{
if (strcmp(methodName, "Equals") == 0)
{
result = NI_System_String_Equals;
}
}
}
else if (strcmp(namespaceName, "System.Threading") == 0)
{
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17508,6 +17508,7 @@ void Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
GenTree* condExpr = qmark->gtGetOp1();
GenTree* trueExpr = qmark->gtGetOp2()->AsColon()->ThenNode();
GenTree* falseExpr = qmark->gtGetOp2()->AsColon()->ElseNode();
unsigned trueBbFlags = qmark->gtGetOp2()->AsColon()->bbTrueFlags;

assert(condExpr->gtFlags & GTF_RELOP_QMARK);
condExpr->gtFlags &= ~GTF_RELOP_QMARK;
Expand Down Expand Up @@ -17639,6 +17640,7 @@ void Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
trueExpr = gtNewTempAssign(lclNum, trueExpr);
}
Statement* trueStmt = fgNewStmtFromTree(trueExpr, stmt->GetILOffsetX());
thenBlock->bbFlags |= trueBbFlags;
fgInsertStmtAtEnd(thenBlock, trueStmt);
}

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/jit/namedintrinsiclist.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ enum NamedIntrinsic : unsigned short
NI_System_Type_get_IsValueType,
NI_System_Type_IsAssignableFrom,
NI_System_Type_IsAssignableTo,
NI_System_String_Equals,

// These are used by HWIntrinsics but are defined more generally
// to allow dead code optimization and handle the recursion case
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -639,42 +639,7 @@ public bool Equals(string? value)

public bool Equals(string? value, StringComparison comparisonType)
{
if (object.ReferenceEquals(this, value))
{
CheckStringComparison(comparisonType);
return true;
}

if (value is null)
{
CheckStringComparison(comparisonType);
return false;
}

switch (comparisonType)
{
case StringComparison.CurrentCulture:
case StringComparison.CurrentCultureIgnoreCase:
return CultureInfo.CurrentCulture.CompareInfo.Compare(this, value, GetCaseCompareOfComparisonCulture(comparisonType)) == 0;

case StringComparison.InvariantCulture:
case StringComparison.InvariantCultureIgnoreCase:
return CompareInfo.Invariant.Compare(this, value, GetCaseCompareOfComparisonCulture(comparisonType)) == 0;

case StringComparison.Ordinal:
if (this.Length != value.Length)
return false;
return EqualsHelper(this, value);

case StringComparison.OrdinalIgnoreCase:
if (this.Length != value.Length)
return false;

return EqualsOrdinalIgnoreCaseNoLengthCheck(this, value);

default:
throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType));
}
return Equals(this, value, comparisonType);
}

// Determines whether two Strings match.
Expand All @@ -693,6 +658,7 @@ public static bool Equals(string? a, string? b)
return EqualsHelper(a, b);
}

[Intrinsic]
public static bool Equals(string? a, string? b, StringComparison comparisonType)
{
if (object.ReferenceEquals(a, b))
Expand Down

0 comments on commit 5883362

Please sign in to comment.