diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index e6334311c6aab..6f35ec48038dd 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -5425,7 +5425,14 @@ GenTree* Compiler::optExtractSideEffListFromConst(GenTree* tree) { // Do a sanity check to ensure persistent side effects aren't discarded and // tell gtExtractSideEffList to ignore the root of the tree. - assert(!gtNodeHasSideEffects(tree, GTF_PERSISTENT_SIDE_EFFECTS)); + // We are relying here on an invariant that VN will only fold non-throwing expressions. + const bool ignoreExceptions = true; + const bool ignoreCctors = false; + // We have to check "AsCall()->HasSideEffects()" here separately because "gtNodeHasSideEffects" + // also checks for side effects that arguments introduce (incosistently so, it otherwise only + // checks for the side effects the node itself has). TODO-Cleanup: change it to not do that? + assert(!gtNodeHasSideEffects(tree, GTF_PERSISTENT_SIDE_EFFECTS) || + (tree->IsCall() && !tree->AsCall()->HasSideEffects(this, ignoreExceptions, ignoreCctors))); // Exception side effects may be ignored because the root is known to be a constant // (e.g. VN may evaluate a DIV/MOD node to a constant and the node may still @@ -5625,6 +5632,13 @@ Compiler::fgWalkResult Compiler::optVNConstantPropCurStmt(BasicBlock* block, Sta } break; + case GT_CALL: + if (!tree->AsCall()->IsPure(this)) + { + return WALK_CONTINUE; + } + break; + default: // Unknown node, continue to walk. return WALK_CONTINUE; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 3758c56da45f5..9fe811025a1db 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5494,6 +5494,9 @@ class Compiler // Does value-numbering for a call. We interpret some helper calls. void fgValueNumberCall(GenTreeCall* call); + // Does value-numbering for a helper representing a cast operation. + void fgValueNumberCastHelper(GenTreeCall* call); + // Does value-numbering for a helper "call" that has a VN function symbol "vnf". void fgValueNumberHelperCallFunc(GenTreeCall* call, VNFunc vnf, ValueNumPair vnpExc); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a9f5402b50852..09fc05d141fdb 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -15924,7 +15924,7 @@ void Compiler::gtExtractSideEffList(GenTree* expr, { if (m_compiler->gtNodeHasSideEffects(node, m_flags)) { - m_sideEffects.Push(node); + PushSideEffects(node); if (node->OperIsBlk() && !node->OperIsStoreBlk()) { JITDUMP("Replace an unused OBJ/BLK node [%06d] with a NULLCHECK\n", dspTreeID(node)); @@ -15941,7 +15941,7 @@ void Compiler::gtExtractSideEffList(GenTree* expr, // gtNodeHasSideEffects and make this check unconditionally. if (node->OperIsAtomicOp()) { - m_sideEffects.Push(node); + PushSideEffects(node); return Compiler::WALK_SKIP_SUBTREES; } @@ -15952,7 +15952,7 @@ void Compiler::gtExtractSideEffList(GenTree* expr, (node->gtGetOp1()->TypeGet() == TYP_STRUCT)) { JITDUMP("Keep the GT_ADDR and GT_IND together:\n"); - m_sideEffects.Push(node); + PushSideEffects(node); return Compiler::WALK_SKIP_SUBTREES; } } @@ -15969,7 +15969,7 @@ void Compiler::gtExtractSideEffList(GenTree* expr, // those need to be extracted as if they're side effects. if (!UnmarkCSE(node)) { - m_sideEffects.Push(node); + PushSideEffects(node); return Compiler::WALK_SKIP_SUBTREES; } @@ -16005,6 +16005,16 @@ void Compiler::gtExtractSideEffList(GenTree* expr, return false; } } + + void PushSideEffects(GenTree* node) + { + // The extracted side effect will no longer be an argument, so unmark it. + // This is safe to do because the side effects will be visited in pre-order, + // aborting as soon as any tree is extracted. Thus if an argument for a call + // is being extracted, it is guaranteed that the call itself will not be. + node->gtFlags &= ~GTF_LATE_ARG; + m_sideEffects.Push(node); + } }; SideEffectExtractor extractor(this, flags); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 71c68cd7d78e7..8143582285386 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -1731,8 +1731,7 @@ ValueNum ValueNumStore::VNForByrefCon(target_size_t cnsVal) return VnForConst(cnsVal, GetByrefCnsMap(), TYP_BYREF); } -ValueNum ValueNumStore::VNForCastOper(var_types castToType, - bool srcIsUnsigned /*=false*/ DEBUGARG(bool printResult /*=true*/)) +ValueNum ValueNumStore::VNForCastOper(var_types castToType, bool srcIsUnsigned) { assert(castToType != TYP_STRUCT); INT32 cnsVal = INT32(castToType) << INT32(VCA_BitCount); @@ -1745,14 +1744,6 @@ ValueNum ValueNumStore::VNForCastOper(var_types castToType, } ValueNum result = VNForIntCon(cnsVal); -#ifdef DEBUG - if (printResult && m_pComp->verbose) - { - printf(" VNForCastOper(%s%s) is " FMT_VN "\n", varTypeName(castToType), srcIsUnsigned ? ", unsignedSrc" : "", - result); - } -#endif - return result; } @@ -1768,7 +1759,7 @@ void ValueNumStore::GetCastOperFromVN(ValueNum vn, var_types* pCastToType, bool* *pSrcIsUnsigned = (value & INT32(VCA_UnsignedSrc)) != 0; *pCastToType = var_types(value >> INT32(VCA_BitCount)); - assert(VNForCastOper(*pCastToType, *pSrcIsUnsigned DEBUGARG(/*printResult*/ false)) == vn); + assert(VNForCastOper(*pCastToType, *pSrcIsUnsigned) == vn); } ValueNum ValueNumStore::VNForHandle(ssize_t cnsVal, GenTreeFlags handleFlags) @@ -5608,6 +5599,10 @@ void ValueNumStore::vnDump(Compiler* comp, ValueNum vn, bool isPtr) vnDumpSimdType(comp, &funcApp); break; #endif // FEATURE_SIMD + case VNF_Cast: + case VNF_CastOvf: + vnDumpCast(comp, vn); + break; default: printf("%s(", VNFuncName(funcApp.m_func)); @@ -5799,6 +5794,36 @@ void ValueNumStore::vnDumpSimdType(Compiler* comp, VNFuncApp* simdType) } #endif // FEATURE_SIMD +void ValueNumStore::vnDumpCast(Compiler* comp, ValueNum castVN) +{ + VNFuncApp cast; + bool castFound = GetVNFunc(castVN, &cast); + assert(castFound && ((cast.m_func == VNF_Cast) || (cast.m_func == VNF_CastOvf))); + + var_types castToType; + bool srcIsUnsigned; + GetCastOperFromVN(cast.m_args[1], &castToType, &srcIsUnsigned); + + var_types castFromType = TypeOfVN(cast.m_args[0]); + var_types resultType = TypeOfVN(castVN); + if (srcIsUnsigned) + { + castFromType = varTypeToUnsigned(castFromType); + } + + comp->vnPrint(cast.m_args[0], 0); + + printf(", "); + if ((resultType != castToType) && (castToType != castFromType)) + { + printf("%s <- %s <- %s", varTypeName(resultType), varTypeName(castToType), varTypeName(castFromType)); + } + else + { + printf("%s <- %s", varTypeName(resultType), varTypeName(castFromType)); + } +} + #endif // DEBUG // Static fields, methods. @@ -9418,38 +9443,41 @@ void Compiler::fgValueNumberCastTree(GenTree* tree) tree->gtVNPair = vnStore->VNPairForCast(srcVNPair, castToType, castFromType, srcIsUnsigned, hasOverflowCheck); } -// Compute the normal ValueNumber for a cast operation with no exceptions +// Compute the ValueNumber for a cast operation ValueNum ValueNumStore::VNForCast(ValueNum srcVN, var_types castToType, var_types castFromType, - bool srcIsUnsigned /* = false */) + bool srcIsUnsigned, /* = false */ + bool hasOverflowCheck) /* = false */ { - // The resulting type after performingthe cast is always widened to a supported IL stack size + // The resulting type after performing the cast is always widened to a supported IL stack size var_types resultType = genActualType(castToType); - // When we're considering actual value returned by a non-checking cast whether or not the source is - // unsigned does *not* matter for non-widening casts. That is, if we cast an int or a uint to short, - // we just extract the first two bytes from the source bit pattern, not worrying about the interpretation. - // The same is true in casting between signed/unsigned types of the same width. Only when we're doing - // a widening cast do we care about whether the source was unsigned,so we know whether to sign or zero extend it. - // - bool srcIsUnsignedNorm = srcIsUnsigned; - if (genTypeSize(castToType) <= genTypeSize(castFromType)) + // For integral unchecked casts, only the "int -> long" upcasts use + // "srcIsUnsigned", to decide whether to use sign or zero extension. + if (!hasOverflowCheck && !varTypeIsFloating(castToType) && (genTypeSize(castToType) <= genTypeSize(castFromType))) { - srcIsUnsignedNorm = false; + srcIsUnsigned = false; } - ValueNum castTypeVN = VNForCastOper(castToType, srcIsUnsigned); - ValueNum resultVN = VNForFunc(resultType, VNF_Cast, srcVN, castTypeVN); + ValueNum srcExcVN; + ValueNum srcNormVN; + VNUnpackExc(srcVN, &srcNormVN, &srcExcVN); -#ifdef DEBUG - if (m_pComp->verbose) + VNFunc castFunc = hasOverflowCheck ? VNF_CastOvf : VNF_Cast; + ValueNum castTypeVN = VNForCastOper(castToType, srcIsUnsigned); + ValueNum resultNormVN = VNForFunc(resultType, castFunc, srcNormVN, castTypeVN); + ValueNum resultExcVN = srcExcVN; + + // Add an exception, except if folding took place. + // We only fold checked casts that do not overflow. + if (hasOverflowCheck && !IsVNConstant(resultNormVN)) { - printf(" VNForCast(" FMT_VN ", " FMT_VN ") returns ", srcVN, castTypeVN); - m_pComp->vnPrint(resultVN, 1); - printf("\n"); + ValueNum ovfChk = VNForFunc(TYP_REF, VNF_ConvOverflowExc, srcNormVN, castTypeVN); + resultExcVN = VNExcSetUnion(VNExcSetSingleton(ovfChk), srcExcVN); } -#endif + + ValueNum resultVN = VNWithExc(resultNormVN, resultExcVN); return resultVN; } @@ -9461,60 +9489,12 @@ ValueNumPair ValueNumStore::VNPairForCast(ValueNumPair srcVNPair, bool srcIsUnsigned, /* = false */ bool hasOverflowCheck) /* = false */ { - // The resulting type after performingthe cast is always widened to a supported IL stack size - var_types resultType = genActualType(castToType); + ValueNum srcLibVN = srcVNPair.GetLiberal(); + ValueNum srcConVN = srcVNPair.GetConservative(); + ValueNum castLibVN = VNForCast(srcLibVN, castToType, castFromType, srcIsUnsigned, hasOverflowCheck); + ValueNum castConVN = VNForCast(srcConVN, castToType, castFromType, srcIsUnsigned, hasOverflowCheck); - ValueNumPair castArgVNP; - ValueNumPair castArgxVNP; - VNPUnpackExc(srcVNPair, &castArgVNP, &castArgxVNP); - - // When we're considering actual value returned by a non-checking cast, (hasOverflowCheck is false) - // whether or not the source is unsigned does *not* matter for non-widening casts. - // That is, if we cast an int or a uint to short, we just extract the first two bytes from the source - // bit pattern, not worrying about the interpretation. The same is true in casting between signed/unsigned - // types of the same width. Only when we're doing a widening cast do we care about whether the source - // was unsigned, so we know whether to sign or zero extend it. - // - // Important: Casts to floating point cannot be optimized in this fashion. (bug 946768) - // - bool srcIsUnsignedNorm = srcIsUnsigned; - if (!hasOverflowCheck && !varTypeIsFloating(castToType) && (genTypeSize(castToType) <= genTypeSize(castFromType))) - { - srcIsUnsignedNorm = false; - } - - VNFunc vnFunc = hasOverflowCheck ? VNF_CastOvf : VNF_Cast; - ValueNum castTypeVN = VNForCastOper(castToType, srcIsUnsignedNorm); - ValueNumPair castTypeVNPair(castTypeVN, castTypeVN); - ValueNumPair castNormRes = VNPairForFunc(resultType, vnFunc, castArgVNP, castTypeVNPair); - - ValueNumPair resultVNP = VNPWithExc(castNormRes, castArgxVNP); - - // If we have a check for overflow, add the exception information. - if (hasOverflowCheck) - { - ValueNumPair excSet = ValueNumStore::VNPForEmptyExcSet(); - - ValueNumKind vnKinds[2] = {VNK_Liberal, VNK_Conservative}; - for (ValueNumKind vnKind : vnKinds) - { - // Do not add exceptions for folded casts. - // We only fold checked casts that do not overflow. - if (IsVNConstant(castNormRes.Get(vnKind))) - { - continue; - } - - ValueNum ovfChk = - VNForFunc(TYP_REF, VNF_ConvOverflowExc, castArgVNP.Get(vnKind), castTypeVNPair.Get(vnKind)); - excSet.Set(vnKind, VNExcSetSingleton(ovfChk)); - } - - excSet = VNPExcSetUnion(excSet, castArgxVNP); - resultVNP = VNPWithExc(castNormRes, excSet); - } - - return resultVNP; + return {castLibVN, castConVN}; } void Compiler::fgValueNumberHelperCallFunc(GenTreeCall* call, VNFunc vnf, ValueNumPair vnpExc) @@ -9789,6 +9769,81 @@ void Compiler::fgValueNumberCall(GenTreeCall* call) } } +void Compiler::fgValueNumberCastHelper(GenTreeCall* call) +{ + CorInfoHelpFunc helpFunc = eeGetHelperNum(call->gtCallMethHnd); + var_types castToType = TYP_UNDEF; + var_types castFromType = TYP_UNDEF; + bool srcIsUnsigned = false; + bool hasOverflowCheck = false; + + switch (helpFunc) + { + case CORINFO_HELP_LNG2DBL: + castToType = TYP_DOUBLE; + castFromType = TYP_LONG; + break; + + case CORINFO_HELP_ULNG2DBL: + castToType = TYP_DOUBLE; + castFromType = TYP_LONG; + srcIsUnsigned = true; + break; + + case CORINFO_HELP_DBL2INT: + castToType = TYP_INT; + castFromType = TYP_DOUBLE; + break; + + case CORINFO_HELP_DBL2INT_OVF: + castToType = TYP_INT; + castFromType = TYP_DOUBLE; + hasOverflowCheck = true; + break; + + case CORINFO_HELP_DBL2LNG: + castToType = TYP_LONG; + castFromType = TYP_DOUBLE; + break; + + case CORINFO_HELP_DBL2LNG_OVF: + castToType = TYP_LONG; + castFromType = TYP_DOUBLE; + hasOverflowCheck = true; + break; + + case CORINFO_HELP_DBL2UINT: + castToType = TYP_UINT; + castFromType = TYP_DOUBLE; + break; + + case CORINFO_HELP_DBL2UINT_OVF: + castToType = TYP_UINT; + castFromType = TYP_DOUBLE; + hasOverflowCheck = true; + break; + + case CORINFO_HELP_DBL2ULNG: + castToType = TYP_ULONG; + castFromType = TYP_DOUBLE; + break; + + case CORINFO_HELP_DBL2ULNG_OVF: + castToType = TYP_ULONG; + castFromType = TYP_DOUBLE; + hasOverflowCheck = true; + break; + + default: + unreached(); + } + + ValueNumPair argVNP = call->gtCallArgs->GetNode()->gtVNPair; + ValueNumPair castVNP = vnStore->VNPairForCast(argVNP, castToType, castFromType, srcIsUnsigned, hasOverflowCheck); + + call->SetVNs(castVNP); +} + VNFunc Compiler::fgValueNumberJitHelperMethodVNFunc(CorInfoHelpFunc helpFunc) { assert(s_helperCallProperties.IsPure(helpFunc) || s_helperCallProperties.IsAllocator(helpFunc)); @@ -9819,12 +9874,14 @@ VNFunc Compiler::fgValueNumberJitHelperMethodVNFunc(CorInfoHelpFunc helpFunc) vnf = VNFunc(GT_RSZ); break; case CORINFO_HELP_LMUL: - case CORINFO_HELP_LMUL_OVF: vnf = VNFunc(GT_MUL); break; + case CORINFO_HELP_LMUL_OVF: + vnf = VNF_MUL_OVF; + break; case CORINFO_HELP_ULMUL_OVF: - vnf = VNFunc(GT_MUL); - break; // Is this the right thing? + vnf = VNF_MUL_UN_OVF; + break; case CORINFO_HELP_LDIV: vnf = VNFunc(GT_DIV); break; @@ -9837,37 +9894,6 @@ VNFunc Compiler::fgValueNumberJitHelperMethodVNFunc(CorInfoHelpFunc helpFunc) case CORINFO_HELP_ULMOD: vnf = VNFunc(GT_UMOD); break; - - case CORINFO_HELP_LNG2DBL: - vnf = VNF_Lng2Dbl; - break; - case CORINFO_HELP_ULNG2DBL: - vnf = VNF_ULng2Dbl; - break; - case CORINFO_HELP_DBL2INT: - vnf = VNF_Dbl2Int; - break; - case CORINFO_HELP_DBL2INT_OVF: - vnf = VNF_Dbl2IntOvf; - break; - case CORINFO_HELP_DBL2LNG: - vnf = VNF_Dbl2Lng; - break; - case CORINFO_HELP_DBL2LNG_OVF: - vnf = VNF_Dbl2LngOvf; - break; - case CORINFO_HELP_DBL2UINT: - vnf = VNF_Dbl2UInt; - break; - case CORINFO_HELP_DBL2UINT_OVF: - vnf = VNF_Dbl2UIntOvf; - break; - case CORINFO_HELP_DBL2ULNG: - vnf = VNF_Dbl2ULng; - break; - case CORINFO_HELP_DBL2ULNG_OVF: - vnf = VNF_Dbl2ULngOvf; - break; case CORINFO_HELP_FLTREM: vnf = VNFunc(GT_MOD); break; @@ -10072,12 +10098,32 @@ VNFunc Compiler::fgValueNumberJitHelperMethodVNFunc(CorInfoHelpFunc helpFunc) bool Compiler::fgValueNumberHelperCall(GenTreeCall* call) { - CorInfoHelpFunc helpFunc = eeGetHelperNum(call->gtCallMethHnd); - bool pure = s_helperCallProperties.IsPure(helpFunc); - bool isAlloc = s_helperCallProperties.IsAllocator(helpFunc); - bool modHeap = s_helperCallProperties.MutatesHeap(helpFunc); - bool mayRunCctor = s_helperCallProperties.MayRunCctor(helpFunc); - bool noThrow = s_helperCallProperties.NoThrow(helpFunc); + CorInfoHelpFunc helpFunc = eeGetHelperNum(call->gtCallMethHnd); + + switch (helpFunc) + { + case CORINFO_HELP_LNG2DBL: + case CORINFO_HELP_ULNG2DBL: + case CORINFO_HELP_DBL2INT: + case CORINFO_HELP_DBL2INT_OVF: + case CORINFO_HELP_DBL2LNG: + case CORINFO_HELP_DBL2LNG_OVF: + case CORINFO_HELP_DBL2UINT: + case CORINFO_HELP_DBL2UINT_OVF: + case CORINFO_HELP_DBL2ULNG: + case CORINFO_HELP_DBL2ULNG_OVF: + fgValueNumberCastHelper(call); + return false; + + default: + break; + } + + bool pure = s_helperCallProperties.IsPure(helpFunc); + bool isAlloc = s_helperCallProperties.IsAllocator(helpFunc); + bool modHeap = s_helperCallProperties.MutatesHeap(helpFunc); + bool mayRunCctor = s_helperCallProperties.MayRunCctor(helpFunc); + bool noThrow = s_helperCallProperties.NoThrow(helpFunc); ValueNumPair vnpExc = ValueNumStore::VNPForEmptyExcSet(); @@ -10843,7 +10889,6 @@ void Compiler::vnpPrint(ValueNumPair vnp, unsigned level) void Compiler::vnPrint(ValueNum vn, unsigned level) { - if (ValueNumStore::isReservedVN(vn)) { printf(ValueNumStore::reservedName(vn)); diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index b64d19993fa72..d09bd3197e91c 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -291,7 +291,7 @@ class ValueNumStore // Packs information about the cast into an integer constant represented by the returned value number, // to be used as the second operand of VNF_Cast & VNF_CastOvf. - ValueNum VNForCastOper(var_types castToType, bool srcIsUnsigned = false DEBUGARG(bool printResult = true)); + ValueNum VNForCastOper(var_types castToType, bool srcIsUnsigned); // Unpacks the information stored by VNForCastOper in the constant represented by the value number. void GetCastOperFromVN(ValueNum vn, var_types* pCastToType, bool* pSrcIsUnsigned); @@ -556,8 +556,12 @@ class ValueNumStore rhs.GetConservative(), indType, block)); } - // Compute the normal ValueNumber for a cast with no exceptions - ValueNum VNForCast(ValueNum srcVN, var_types castToType, var_types castFromType, bool srcIsUnsigned = false); + // Compute the ValueNumber for a cast + ValueNum VNForCast(ValueNum srcVN, + var_types castToType, + var_types castFromType, + bool srcIsUnsigned = false, + bool hasOverflowCheck = false); // Compute the ValueNumberPair for a cast ValueNumPair VNPairForCast(ValueNumPair srcVNPair, @@ -898,6 +902,10 @@ class ValueNumStore void vnDumpSimdType(Compiler* comp, VNFuncApp* simdType); #endif // FEATURE_SIMD + // Requires "castVN" to represent VNF_Cast or VNF_CastOvf. + // Prints the cast's representation mirroring GT_CAST's dump format. + void vnDumpCast(Compiler* comp, ValueNum castVN); + // Returns the string name of "vnf". static const char* VNFuncName(VNFunc vnf); // Used in the implementation of the above. diff --git a/src/coreclr/jit/valuenumfuncs.h b/src/coreclr/jit/valuenumfuncs.h index 448f40e382146..7665770881bcd 100644 --- a/src/coreclr/jit/valuenumfuncs.h +++ b/src/coreclr/jit/valuenumfuncs.h @@ -69,16 +69,6 @@ ValueNumFuncDef(InvalidCastExc, 2, false, false, false) // CastClass check, ValueNumFuncDef(NewArrOverflowExc, 1, false, false, false) // Raises Integer overflow when Arg 0 is negative ValueNumFuncDef(HelperMultipleExc, 0, false, false, false) // Represents one or more different exceptions that could be thrown by a Jit Helper method -ValueNumFuncDef(Lng2Dbl, 1, false, false, false) -ValueNumFuncDef(ULng2Dbl, 1, false, false, false) -ValueNumFuncDef(Dbl2Int, 1, false, false, false) -ValueNumFuncDef(Dbl2UInt, 1, false, false, false) -ValueNumFuncDef(Dbl2Lng, 1, false, false, false) -ValueNumFuncDef(Dbl2ULng, 1, false, false, false) -ValueNumFuncDef(Dbl2IntOvf, 1, false, false, false) -ValueNumFuncDef(Dbl2UIntOvf, 1, false, false, false) -ValueNumFuncDef(Dbl2LngOvf, 1, false, false, false) -ValueNumFuncDef(Dbl2ULngOvf, 1, false, false, false) ValueNumFuncDef(FltRound, 1, false, false, false) ValueNumFuncDef(DblRound, 1, false, false, false)