diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp index f0b042b16ca91..eba1849389f7e 100644 --- a/src/coreclr/jit/helperexpansion.cpp +++ b/src/coreclr/jit/helperexpansion.cpp @@ -1805,6 +1805,7 @@ PhaseStatus Compiler::fgLateCastExpansion() enum class TypeCheckFailedAction { + Unknown, ReturnNull, CallHelper, CallHelper_Specialized, @@ -1813,8 +1814,10 @@ enum class TypeCheckFailedAction enum class TypeCheckPassedAction { + Unknown, ReturnObj, ReturnNull, + CallHelper_AlwaysThrows, }; // Some arbitrary limit on the number of guesses we can make @@ -1846,6 +1849,9 @@ static int PickCandidatesForTypeCheck(Compiler* comp, TypeCheckFailedAction* typeCheckFailed, TypeCheckPassedAction* typeCheckPassed) { + *typeCheckFailed = TypeCheckFailedAction::Unknown; + *typeCheckPassed = TypeCheckPassedAction::Unknown; + if (!castHelper->IsHelperCall() || ((castHelper->gtCallMoreFlags & GTF_CALL_M_CAST_CAN_BE_EXPANDED) == 0)) { // It's not eligible for expansion (already expanded in importer) @@ -1891,16 +1897,26 @@ static int PickCandidatesForTypeCheck(Compiler* comp, // First, let's grab the expected class we're casting to/checking instance of: // E.g. "call CORINFO_HELP_ISINSTANCEOFCLASS(castToCls, obj)" GenTree* clsArg = castHelper->gtArgs.GetUserArgByIndex(0)->GetNode(); + GenTree* objArg = castHelper->gtArgs.GetUserArgByIndex(1)->GetNode(); CORINFO_CLASS_HANDLE castToCls = comp->gtGetHelperArgClassHandle(clsArg); if (castToCls == NO_CLASS_HANDLE) { - // clsArg doesn't represent a class handle - bail out - // TODO-InlineCast: if CSE becomes a problem - move the whole phase after assertion prop, - // so we can still rely on VN to get the class handle. + // We don't expect the constant handle to be CSE'd here because importer + // sets GTF_DONT_CSE for it. The only case when this arg is not a constant is RUNTIMELOOKUP + // TODO-InlineCast: we should be able to handle RUNTIMELOOKUP as well. JITDUMP("clsArg is not a constant handle - bail out.\n"); return 0; } + if ((objArg->gtFlags & GTF_ALL_EFFECT) != 0 && comp->lvaHaveManyLocals()) + { + // TODO: Revise this: + // * Some casts are profitable even when ran out of tracked locals + // * We might want to use a shared local in all casts (similar to what we do boxing) + JITDUMP("lvaHaveManyLocals() is true and objArg has side effects - bail out.") + return 0; + } + // Assume that in the slow path (fallback) we'll always invoke the helper. // In some cases we can optimize this further e.g. either mark it additionally // as no-return (BBJ_THROW) or simply return null. @@ -1917,6 +1933,31 @@ static int PickCandidatesForTypeCheck(Compiler* comp, const unsigned isAbstractFlags = CORINFO_FLG_INTERFACE | CORINFO_FLG_ABSTRACT; + // See what we already know about the type of the object being cast. + bool fromClassIsExact = false; + bool fromClassIsNonNull = false; + CORINFO_CLASS_HANDLE fromClass = comp->gtGetClassHandle(objArg, &fromClassIsExact, &fromClassIsNonNull); + if ((fromClass != NO_CLASS_HANDLE) && fromClassIsExact) + { + if (fromClassIsNonNull) + { + // An additional hint for the expansion that the object is not null + castHelper->gtCallMoreFlags |= GTF_CALL_M_CAST_OBJ_NONNULL; + } + + const TypeCompareState castResult = comp->info.compCompHnd->compareTypesForCast(fromClass, castToCls); + if (isCastClass && (castResult == TypeCompareState::MustNot)) + { + // The cast is guaranteed to fail, the expansion logic can skip the type check entirely + *typeCheckPassed = TypeCheckPassedAction::CallHelper_AlwaysThrows; + return 0; + } + + // TODO-InlineCast: + // isinst and MustNot -> just return null + // isinst/castclass and Must -> just return obj + } + // // Now we need to figure out what classes to use for the fast path, we have 4 options: // 1) If "cast to" class is already exact we can go ahead and make some decisions @@ -2119,36 +2160,29 @@ static int PickCandidatesForTypeCheck(Compiler* comp, return 0; case CORINFO_HELP_CHKCASTARRAY: - // CHKCASTARRAY against exact classes is already handled above, so it's not exact here. - // - // (int[])obj - can we use int[] as a guess? No! It's an overhead if obj is uint[] - // or any int-backed enum - return 0; - case CORINFO_HELP_CHKCASTCLASS: - // CHKCASTCLASS against exact classes is already handled above, so it's not exact here. + case CORINFO_HELP_CHKCASTANY: + // These casts against exact classes are already handled above, so it's not exact here. // // let's use castToCls as a guess, we might regress some cases, but at least we know that unrelated // types are going to throw InvalidCastException, so we can assume the overhead happens rarely. - candidates[0] = castToCls; - // 50% chance of successful type check (speculative guess) - likelihoods[0] = 50; // - // A small optimization - use a slightly faster fallback which assumes that we've already checked - // for null and for castToCls itself, so it won't do it again. - *typeCheckFailed = TypeCheckFailedAction::CallHelper_Specialized; - return 1; - - case CORINFO_HELP_CHKCASTANY: - // Same as CORINFO_HELP_CHKCASTCLASS above, the only difference - let's check castToCls for - // being non-abstract and non-interface first as it makes no sense to speculate on those. if ((comp->info.compCompHnd->getClassAttribs(castToCls) & isAbstractFlags) != 0) { + // The guess is abstract - it will never pass the type check return 0; } candidates[0] = castToCls; // 50% chance of successful type check (speculative guess) likelihoods[0] = 50; + // + // A small optimization - use a slightly faster fallback which assumes that we've already checked + // for null and for castToCls itself, so it won't do it again. + // + if (helper == CORINFO_HELP_CHKCASTCLASS) + { + *typeCheckFailed = TypeCheckFailedAction::CallHelper_Specialized; + } return 1; case CORINFO_HELP_ISINSTANCEOFINTERFACE: @@ -2201,7 +2235,7 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, const int numOfCandidates = PickCandidatesForTypeCheck(this, call, expectedExactClasses, &commonCls, likelihoods, &typeCheckFailedAction, &typeCheckPassedAction); - if (numOfCandidates == 0) + if ((numOfCandidates == 0) && (typeCheckPassedAction != TypeCheckPassedAction::CallHelper_AlwaysThrows)) { return false; } @@ -2256,6 +2290,14 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, // use(tmp); // + // NOTE: if the cast is known to always fail (TypeCheckPassedAction::CallHelper_AlwaysThrows) + // we can omit the typeCheckBb and typeCheckSucceedBb and only have: + // + // if (obj == null) goto lastBb; + // throw InvalidCastException; + // + // if obj is known to be non-null, then it will be just the throw block. + // Block 1: nullcheckBb // TODO-InlineCast: assertionprop should leave us a mark that objArg is never null, so we can omit this check // it's too late to rely on upstream phases to do this for us (unless we do optRepeat). @@ -2278,17 +2320,45 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, BasicBlock* lastTypeCheckBb = nullcheckBb; for (int candidateId = 0; candidateId < numOfCandidates; candidateId++) { - GenTree* likelyClsNode = gtNewIconEmbClsHndNode(expectedExactClasses[candidateId]); - GenTree* mtCheck = gtNewOperNode(GT_EQ, TYP_INT, gtNewMethodTableLookup(gtCloneExpr(tmpNode)), likelyClsNode); + GenTree* expectedClsNode = gtNewIconEmbClsHndNode(expectedExactClasses[candidateId]); + GenTree* storeCseVal = nullptr; + + // Manually CSE the expectedClsNode for first type check if it's the same as the original clsArg + // TODO-InlineCast: consider not doing this if the helper call is cold + if (candidateId == 0) + { + GenTree*& castArg = call->gtArgs.GetUserArgByIndex(0)->LateNodeRef(); + if (GenTree::Compare(castArg, expectedClsNode)) + { + const unsigned clsTmp = lvaGrabTemp(true DEBUGARG("CSE for expectedClsNode")); + storeCseVal = gtNewTempStore(clsTmp, expectedClsNode); + expectedClsNode = gtNewLclvNode(clsTmp, TYP_I_IMPL); + castArg = gtNewLclvNode(clsTmp, TYP_I_IMPL); + } + } + + GenTree* mtCheck = gtNewOperNode(GT_EQ, TYP_INT, gtNewMethodTableLookup(gtCloneExpr(tmpNode)), expectedClsNode); mtCheck->gtFlags |= GTF_RELOP_JMP_USED; GenTree* jtrue = gtNewOperNode(GT_JTRUE, TYP_VOID, mtCheck); typeChecksBbs[candidateId] = fgNewBBFromTreeAfter(BBJ_COND, lastTypeCheckBb, jtrue, debugInfo, lastBb, true); lastTypeCheckBb = typeChecksBbs[candidateId]; + + // Insert the CSE node as the first statement in the block + if (storeCseVal != nullptr) + { + Statement* clsStmt = fgNewStmtAtBeg(typeChecksBbs[0], storeCseVal, debugInfo); + gtSetStmtInfo(clsStmt); + fgSetStmtSeq(clsStmt); + } } + // numOfCandidates being 0 means that we don't need any type checks + // as we already know that the cast is going to fail. + const bool typeCheckNotNeeded = numOfCandidates == 0; + // Block 3: fallbackBb BasicBlock* fallbackBb; - if (typeCheckFailedAction == TypeCheckFailedAction::CallHelper_AlwaysThrows) + if (typeCheckNotNeeded || (typeCheckFailedAction == TypeCheckFailedAction::CallHelper_AlwaysThrows)) { // fallback call is used only to throw InvalidCastException call->gtCallMoreFlags |= GTF_CALL_M_DOES_NOT_RETURN; @@ -2320,19 +2390,19 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, } else { - assert(typeCheckPassedAction == TypeCheckPassedAction::ReturnObj); // No-op because tmp was already assigned to obj typeCheckSucceedTree = gtNewNothingNode(); } BasicBlock* typeCheckSucceedBb = - fgNewBBFromTreeAfter(BBJ_ALWAYS, fallbackBb, typeCheckSucceedTree, debugInfo, lastBb); + typeCheckNotNeeded ? nullptr + : fgNewBBFromTreeAfter(BBJ_ALWAYS, fallbackBb, typeCheckSucceedTree, debugInfo, lastBb); // // Wire up the blocks // firstBb->SetTarget(nullcheckBb); nullcheckBb->SetTrueTarget(lastBb); - nullcheckBb->SetFalseTarget(typeChecksBbs[0]); + nullcheckBb->SetFalseTarget(typeCheckNotNeeded ? fallbackBb : typeChecksBbs[0]); // Tricky case - wire up multiple type check blocks (in most cases there is only one) for (int candidateId = 0; candidateId < numOfCandidates; candidateId++) @@ -2360,10 +2430,18 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, fgRemoveRefPred(lastBb, firstBb); fgAddRefPred(nullcheckBb, firstBb); - fgAddRefPred(typeChecksBbs[0], nullcheckBb); fgAddRefPred(lastBb, nullcheckBb); - fgAddRefPred(lastBb, typeCheckSucceedBb); - if (typeCheckFailedAction != TypeCheckFailedAction::CallHelper_AlwaysThrows) + if (typeCheckNotNeeded) + { + fgAddRefPred(fallbackBb, nullcheckBb); + } + else + { + fgAddRefPred(typeChecksBbs[0], nullcheckBb); + fgAddRefPred(lastBb, typeCheckSucceedBb); + } + + if (!fallbackBb->KindIs(BBJ_THROW)) { // if fallbackBb is BBJ_THROW then it has no successors fgAddRefPred(lastBb, fallbackBb); @@ -2391,8 +2469,19 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, } totalLikelihood += likelihood; } - fallbackBb->inheritWeightPercentage(lastTypeCheckBb, fallbackBb->KindIs(BBJ_THROW) ? 0 : 100 - totalLikelihood); - typeCheckSucceedBb->inheritWeightPercentage(typeChecksBbs[0], totalLikelihood); + + if (fallbackBb->KindIs(BBJ_THROW)) + { + fallbackBb->bbSetRunRarely(); + } + else + { + fallbackBb->inheritWeightPercentage(lastTypeCheckBb, 100 - totalLikelihood); + } + if (!typeCheckNotNeeded) + { + typeCheckSucceedBb->inheritWeightPercentage(typeChecksBbs[0], totalLikelihood); + } lastBb->inheritWeight(firstBb); // @@ -2401,14 +2490,13 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, assert(BasicBlock::sameEHRegion(firstBb, lastBb)); assert(BasicBlock::sameEHRegion(firstBb, nullcheckBb)); assert(BasicBlock::sameEHRegion(firstBb, fallbackBb)); - assert(BasicBlock::sameEHRegion(firstBb, lastTypeCheckBb)); // call guarantees that obj is never null, we can drop the nullcheck // by converting it to a BBJ_ALWAYS to typeCheckBb. if ((call->gtCallMoreFlags & GTF_CALL_M_CAST_OBJ_NONNULL) != 0) { fgRemoveStmt(nullcheckBb, nullcheckBb->lastStmt()); - nullcheckBb->SetKindAndTarget(BBJ_ALWAYS, typeChecksBbs[0]); + nullcheckBb->SetKindAndTarget(BBJ_ALWAYS, typeCheckNotNeeded ? fallbackBb : typeChecksBbs[0]); fgRemoveRefPred(lastBb, nullcheckBb); } diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 86d04c064334b..2e79a60439020 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -5514,7 +5514,15 @@ GenTree* Compiler::impCastClassOrIsInstToTree( } } - const bool expandInline = canExpandInline && shouldExpandInline; + bool expandInline = canExpandInline && shouldExpandInline; + + if (op2->IsIconHandle(GTF_ICON_CLASS_HDL) && (helper != CORINFO_HELP_ISINSTANCEOFCLASS || !isClassExact)) + { + // TODO-InlineCast: move these to the late cast expansion phase as well: + // 1) isinst + // 2) op2 being GT_RUNTIMELOOKUP + expandInline = false; + } if (!expandInline) { @@ -5529,7 +5537,8 @@ GenTree* Compiler::impCastClassOrIsInstToTree( GenTreeCall* call = gtNewHelperCallNode(helper, TYP_REF, op2, op1); // Instrument this castclass/isinst - if ((JitConfig.JitClassProfiling() > 0) && impIsCastHelperEligibleForClassProbe(call) && !isClassExact) + if ((JitConfig.JitClassProfiling() > 0) && impIsCastHelperEligibleForClassProbe(call) && !isClassExact && + !compCurBB->isRunRarely()) { // It doesn't make sense to instrument "x is T" or "(T)x" for shared T if ((info.compCompHnd->getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) == 0) diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 544bf5d41b8e6..554c0ce6dc9a3 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -1102,7 +1102,8 @@ class ValueNumStore #ifdef _MSC_VER - assert(&typeid(T) == &typeid(size_t)); // We represent ref/byref constants as size_t's. + assert((&typeid(T) == &typeid(size_t)) || + (&typeid(T) == &typeid(ssize_t))); // We represent ref/byref constants as size_t/ssize_t #endif // _MSC_VER