From 99a982999e806188b71db2f4dff62c4818559e7d Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sat, 17 Apr 2021 16:19:57 +0300 Subject: [PATCH] Implemented folding of checked casts in value numbering --- src/coreclr/jit/assertionprop.cpp | 3 + src/coreclr/jit/valuenum.cpp | 113 ++++++++++++++++++++++-------- src/coreclr/jit/valuenum.h | 10 ++- src/coreclr/jit/valuenumfuncs.h | 2 +- src/coreclr/jit/valuenumtype.h | 13 ++++ 5 files changed, 110 insertions(+), 31 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index c6a71b6ed7cd9..40b3f5cabf8ff 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4992,6 +4992,9 @@ GenTree* Compiler::optExtractSideEffListFromConst(GenTree* tree) bool ignoreRoot = true; gtExtractSideEffList(tree, &sideEffList, GTF_SIDE_EFFECT, ignoreRoot); + + JITDUMP("Extracted side effects from a constant tree [%06u]:\n", tree->gtTreeID); + DISPTREE(sideEffList); } return sideEffList; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 96c16e92f9341..9b9a7558cb855 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -374,6 +374,11 @@ bool ValueNumStore::VNFuncIsOverflowArithmetic(VNFunc vnf) return VNF_ADD_OVF <= vnf && vnf <= VNF_MUL_UN_OVF; } +bool ValueNumStore::VNFuncIsNumericCast(VNFunc vnf) +{ + return (vnf == VNF_Cast) || (vnf == VNF_CastOvf); +} + unsigned ValueNumStore::VNFuncArity(VNFunc vnf) { // Read the bit field out of the table... @@ -1747,7 +1752,8 @@ ValueNum ValueNumStore::VNForByrefCon(target_size_t cnsVal) return VnForConst(cnsVal, GetByrefCnsMap(), TYP_BYREF); } -ValueNum ValueNumStore::VNForCastOper(var_types castToType, bool srcIsUnsigned /*=false*/) +ValueNum ValueNumStore::VNForCastOper(var_types castToType, + bool srcIsUnsigned /*=false*/ DEBUGARG(bool printResult /*=true*/)) { assert(castToType != TYP_STRUCT); INT32 cnsVal = INT32(castToType) << INT32(VCA_BitCount); @@ -1761,7 +1767,7 @@ ValueNum ValueNumStore::VNForCastOper(var_types castToType, bool srcIsUnsigned / ValueNum result = VNForIntCon(cnsVal); #ifdef DEBUG - if (m_pComp->verbose) + if (printResult && m_pComp->verbose) { printf(" VNForCastOper(%s%s) is " FMT_VN "\n", varTypeName(castToType), srcIsUnsigned ? ", unsignedSrc" : "", result); @@ -1771,6 +1777,21 @@ ValueNum ValueNumStore::VNForCastOper(var_types castToType, bool srcIsUnsigned / return result; } +void ValueNumStore::GetCastOperFromVN(ValueNum vn, var_types* pCastToType, bool* pSrcIsUnsigned) +{ + assert(pCastToType != nullptr); + assert(pSrcIsUnsigned != nullptr); + assert(IsVNInt32Constant(vn)); + + int value = GetConstantInt32(vn); + assert(value >= 0); + + *pSrcIsUnsigned = (value & INT32(VCA_UnsignedSrc)) != 0; + *pCastToType = var_types(value >> INT32(VCA_BitCount)); + + assert(VNForCastOper(*pCastToType, *pSrcIsUnsigned DEBUGARG(/*printResult*/ false)) == vn); +} + ValueNum ValueNumStore::VNForHandle(ssize_t cnsVal, GenTreeFlags handleFlags) { assert((handleFlags & ~GTF_ICON_HDL_MASK) == 0); @@ -1976,26 +1997,23 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN, V // Special case for VNF_Cast of constant handles // Don't allow an eval/fold of a GT_CAST(non-I_IMPL, Handle) // - if ((func == VNF_Cast) && (typ != TYP_I_IMPL) && IsVNHandle(arg0VN)) + if (VNFuncIsNumericCast(func) && (typ != TYP_I_IMPL) && IsVNHandle(arg0VN)) { canFold = false; } - // Currently CanEvalForConstantArgs() returns false for VNF_CastOvf - // In the future we could handle this case in folding. - assert(func != VNF_CastOvf); - // It is possible for us to have mismatched types (see Bug 750863) // We don't try to fold a binary operation when one of the constant operands - // is a floating-point constant and the other is not. - // + // is a floating-point constant and the other is not, except for casts. + // For casts, the second operand just carries the information about the source. + var_types arg0VNtyp = TypeOfVN(arg0VN); bool arg0IsFloating = varTypeIsFloating(arg0VNtyp); var_types arg1VNtyp = TypeOfVN(arg1VN); bool arg1IsFloating = varTypeIsFloating(arg1VNtyp); - if (arg0IsFloating != arg1IsFloating) + if (!VNFuncIsNumericCast(func) && (arg0IsFloating != arg1IsFloating)) { canFold = false; } @@ -2634,7 +2652,7 @@ ValueNum ValueNumStore::EvalFuncForConstantArgs(var_types typ, VNFunc func, Valu assert(!VNHasExc(arg0VN) && !VNHasExc(arg1VN)); // Otherwise, would not be constant. // if our func is the VNF_Cast operation we handle it first - if (func == VNF_Cast) + if (VNFuncIsNumericCast(func)) { return EvalCastForConstantArgs(typ, func, arg0VN, arg1VN); } @@ -2845,7 +2863,7 @@ ValueNum ValueNumStore::EvalFuncForConstantFPArgs(var_types typ, VNFunc func, Va // ValueNum ValueNumStore::EvalCastForConstantArgs(var_types typ, VNFunc func, ValueNum arg0VN, ValueNum arg1VN) { - assert(func == VNF_Cast); + assert(VNFuncIsNumericCast(func)); assert(IsVNConstant(arg0VN) && IsVNConstant(arg1VN)); // Stack-normalize the result type. @@ -2855,12 +2873,6 @@ ValueNum ValueNumStore::EvalCastForConstantArgs(var_types typ, VNFunc func, Valu } var_types arg0VNtyp = TypeOfVN(arg0VN); - var_types arg1VNtyp = TypeOfVN(arg1VN); - - // arg1VN is really the gtCastType that we are casting to - assert(arg1VNtyp == TYP_INT); - int arg1Val = ConstantValue(arg1VN); - assert(arg1Val >= 0); if (IsVNHandle(arg0VN)) { @@ -2868,12 +2880,12 @@ ValueNum ValueNumStore::EvalCastForConstantArgs(var_types typ, VNFunc func, Valu assert(typ == TYP_I_IMPL); } - // We previously encoded the castToType operation using vnForCastOper() - // - bool srcIsUnsigned = ((arg1Val & INT32(VCA_UnsignedSrc)) != 0); - var_types castToType = var_types(arg1Val >> INT32(VCA_BitCount)); - + // We previously encoded the castToType operation using VNForCastOper(). + var_types castToType; + bool srcIsUnsigned; + GetCastOperFromVN(arg1VN, &castToType, &srcIsUnsigned); var_types castFromType = arg0VNtyp; + bool checkedCast = func == VNF_CastOvf; switch (castFromType) // GT_CAST source type { @@ -2884,6 +2896,7 @@ ValueNum ValueNumStore::EvalCastForConstantArgs(var_types typ, VNFunc func, Valu case TYP_INT: { int arg0Val = GetConstantInt32(arg0VN); + assert(!checkedCast || !CheckedOps::CastFromIntOverflows(arg0Val, castToType, srcIsUnsigned)); switch (castToType) { @@ -2966,6 +2979,7 @@ ValueNum ValueNumStore::EvalCastForConstantArgs(var_types typ, VNFunc func, Valu #endif case TYP_LONG: INT64 arg0Val = GetConstantInt64(arg0VN); + assert(!checkedCast || !CheckedOps::CastFromLongOverflows(arg0Val, castToType, srcIsUnsigned)); switch (castToType) { @@ -3022,6 +3036,7 @@ ValueNum ValueNumStore::EvalCastForConstantArgs(var_types typ, VNFunc func, Valu case TYP_FLOAT: { float arg0Val = GetConstantSingle(arg0VN); + assert(!checkedCast || !CheckedOps::CastFromFloatOverflows(arg0Val, castToType)); switch (castToType) { @@ -3063,6 +3078,7 @@ ValueNum ValueNumStore::EvalCastForConstantArgs(var_types typ, VNFunc func, Valu case TYP_DOUBLE: { double arg0Val = GetConstantDouble(arg0VN); + assert(!checkedCast || !CheckedOps::CastFromDoubleOverflows(arg0Val, castToType)); switch (castToType) { @@ -3192,6 +3208,7 @@ bool ValueNumStore::CanEvalForConstantArgs(VNFunc vnf) case VNF_MUL_UN_OVF: case VNF_Cast: + case VNF_CastOvf: // We can evaluate these. return true; @@ -3326,6 +3343,29 @@ bool ValueNumStore::VNEvalShouldFold(var_types typ, VNFunc func, ValueNum arg0VN } } + // Is this a checked cast that will always throw an exception? + if (func == VNF_CastOvf) + { + var_types castToType; + bool fromUnsigned; + GetCastOperFromVN(arg1VN, &castToType, &fromUnsigned); + var_types castFromType = TypeOfVN(arg0VN); + + switch (castFromType) + { + case TYP_INT: + return !CheckedOps::CastFromIntOverflows(GetConstantInt32(arg0VN), castToType, fromUnsigned); + case TYP_LONG: + return !CheckedOps::CastFromLongOverflows(GetConstantInt64(arg0VN), castToType, fromUnsigned); + case TYP_FLOAT: + return !CheckedOps::CastFromFloatOverflows(GetConstantSingle(arg0VN), castToType); + case TYP_DOUBLE: + return !CheckedOps::CastFromDoubleOverflows(GetConstantDouble(arg0VN), castToType); + default: + return false; + } + } + return true; } @@ -9206,7 +9246,7 @@ void Compiler::fgValueNumberCastTree(GenTree* tree) bool srcIsUnsigned = ((tree->gtFlags & GTF_UNSIGNED) != 0); bool hasOverflowCheck = tree->gtOverflowEx(); - assert(genActualType(castToType) == genActualType(tree->TypeGet())); // Insure that the resultType is correct + assert(genActualType(castToType) == genActualType(tree->TypeGet())); // Ensure that the resultType is correct tree->gtVNPair = vnStore->VNPairForCast(srcVNPair, castToType, castFromType, srcIsUnsigned, hasOverflowCheck); } @@ -9286,10 +9326,25 @@ ValueNumPair ValueNumStore::VNPairForCast(ValueNumPair srcVNPair, // If we have a check for overflow, add the exception information. if (hasOverflowCheck) { - ValueNumPair ovfChk = VNPairForFunc(TYP_REF, VNF_ConvOverflowExc, castArgVNP, castTypeVNPair); - ValueNumPair excSet = VNPExcSetSingleton(ovfChk); - excSet = VNPExcSetUnion(excSet, castArgxVNP); - resultVNP = VNPWithExc(castNormRes, excSet); + 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; @@ -9869,8 +9924,8 @@ bool Compiler::fgValueNumberHelperCall(GenTreeCall* call) // switch (helpFunc) { + // This helper always throws the VNF_OverflowExc exception. case CORINFO_HELP_OVERFLOW: - // This helper always throws the VNF_OverflowExc exception vnpExc = vnStore->VNPExcSetSingleton( vnStore->VNPairForFunc(TYP_REF, VNF_OverflowExc, vnStore->VNPForVoid())); break; diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 458f1b551348c..3d81e37a3c8d3 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -251,6 +251,9 @@ class ValueNumStore // VNF_ADD_UN_OVF, VNF_SUB_UN_OVF, VNF_MUL_UN_OVF. static bool VNFuncIsOverflowArithmetic(VNFunc vnf); + // Returns "true" iff "vnf" is VNF_Cast or VNF_CastOvf. + static bool VNFuncIsNumericCast(VNFunc vnf); + // Returns the arity of "vnf". static unsigned VNFuncArity(VNFunc vnf); @@ -286,7 +289,12 @@ class ValueNumStore } #endif - ValueNum VNForCastOper(var_types castToType, bool srcIsUnsigned = false); + // 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)); + + // Unpacks the information stored by VNForCastOper in the constant represented by the value number. + void GetCastOperFromVN(ValueNum vn, var_types* pCastToType, bool* pSrcIsUnsigned); // We keep handle values in a separate pool, so we don't confuse a handle with an int constant // that happens to be the same... diff --git a/src/coreclr/jit/valuenumfuncs.h b/src/coreclr/jit/valuenumfuncs.h index c1fa3137fb907..23364910c1e93 100644 --- a/src/coreclr/jit/valuenumfuncs.h +++ b/src/coreclr/jit/valuenumfuncs.h @@ -29,7 +29,7 @@ ValueNumFuncDef(Cast, 2, false, false, false) // VNF_Cast: Cast Operat // Args: 0: Source for the cast operation. // 1: Constant integer representing the operation . // Use VNForCastOper() to construct. -ValueNumFuncDef(CastOvf, 2, false, false, false) // Same as a VNF_Cast but also can throw an overflow exception, currently we don't try to constant fold this +ValueNumFuncDef(CastOvf, 2, false, false, false) // Same as a VNF_Cast but also can throw an overflow exception. ValueNumFuncDef(CastClass, 2, false, false, false) // Args: 0: Handle of class being cast to, 1: object being cast. ValueNumFuncDef(IsInstanceOf, 2, false, false, false) // Args: 0: Handle of class being queried, 1: object being queried. diff --git a/src/coreclr/jit/valuenumtype.h b/src/coreclr/jit/valuenumtype.h index 326f0ef65fda4..1c991e5c05a97 100644 --- a/src/coreclr/jit/valuenumtype.h +++ b/src/coreclr/jit/valuenumtype.h @@ -77,6 +77,19 @@ struct ValueNumPair } } + void Set(ValueNumKind vnk, ValueNum vn) + { + if (vnk == VNK_Liberal) + { + SetLiberal(vn); + } + else + { + assert(vnk == VNK_Conservative); + SetConservative(vn); + } + } + void SetBoth(ValueNum vn) { m_liberal = vn;