From ff3d99b9438bde50dc975920764dd83ccff41848 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Thu, 19 Aug 2021 14:56:17 +0300 Subject: [PATCH 1/3] Centralize the "FitsIn" logic So that there is one canonical way to check if a type can represent some value. --- src/coreclr/jit/codegenxarch.cpp | 5 ++--- src/coreclr/jit/compiler.hpp | 28 ---------------------------- src/coreclr/jit/lower.cpp | 2 +- src/coreclr/jit/utils.cpp | 26 +++++++++----------------- src/coreclr/jit/utils.h | 30 ++++++++++++++++++++++++++++++ 5 files changed, 42 insertions(+), 49 deletions(-) diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index f268c6a066a86a..fc053a4a9039e7 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -6034,7 +6034,7 @@ void CodeGen::genCompareInt(GenTree* treeNode) #ifdef TARGET_X86 (!op1->isUsedFromReg() || isByteReg(op1->GetRegNum())) && #endif - (op2->IsCnsIntOrI() && genSmallTypeCanRepresentValue(TYP_UBYTE, op2->AsIntCon()->IconValue()))) + (op2->IsCnsIntOrI() && FitsIn(op2->AsIntCon()->IconValue()))) { type = TYP_UBYTE; } @@ -6104,8 +6104,7 @@ void CodeGen::genCompareInt(GenTree* treeNode) // If op2 is smaller then it cannot be in memory, we're probably missing a cast assert((genTypeSize(op2Type) >= genTypeSize(type)) || !op2->isUsedFromMemory()); // If we ended up with a small type and op2 is a constant then make sure we don't lose constant bits - assert(!op2->IsCnsIntOrI() || !varTypeIsSmall(type) || - genSmallTypeCanRepresentValue(type, op2->AsIntCon()->IconValue())); + assert(!op2->IsCnsIntOrI() || !varTypeIsSmall(type) || FitsIn(type, op2->AsIntCon()->IconValue())); } // The type cannot be larger than the machine word size diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index ed0b0940fee66b..6612c4812663e7 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -533,34 +533,6 @@ inline regNumber genRegNumFromMask(regMaskTP mask) return regNum; } -//------------------------------------------------------------------------------ -// genSmallTypeCanRepresentValue: Checks if a value can be represented by a given small type. -// -// Arguments: -// value - the value to check -// type - the type -// -// Return Value: -// True if the value is representable, false otherwise. - -inline bool genSmallTypeCanRepresentValue(var_types type, ssize_t value) -{ - switch (type) - { - case TYP_UBYTE: - case TYP_BOOL: - return FitsIn(value); - case TYP_BYTE: - return FitsIn(value); - case TYP_USHORT: - return FitsIn(value); - case TYP_SHORT: - return FitsIn(value); - default: - unreached(); - } -} - /***************************************************************************** * * Return the size in bytes of the given type. diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 08cbc8b0ad0d37..c0e4510ccf7340 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2481,7 +2481,7 @@ GenTree* Lowering::OptimizeConstCompare(GenTree* cmp) #ifdef TARGET_XARCH var_types op1Type = op1->TypeGet(); - if (IsContainableMemoryOp(op1) && varTypeIsSmall(op1Type) && genSmallTypeCanRepresentValue(op1Type, op2Value)) + if (IsContainableMemoryOp(op1) && varTypeIsSmall(op1Type) && FitsIn(op1Type, op2Value)) { // // If op1's type is small then try to narrow op2 so it has the same type as op1. diff --git a/src/coreclr/jit/utils.cpp b/src/coreclr/jit/utils.cpp index f18c1a6a6709cb..eca5bc1723675d 100644 --- a/src/coreclr/jit/utils.cpp +++ b/src/coreclr/jit/utils.cpp @@ -2592,24 +2592,21 @@ bool CastFromIntOverflows(int32_t fromValue, var_types toType, bool fromUnsigned { switch (toType) { - case TYP_BYTE: - return ((int8_t)fromValue != fromValue) || (fromUnsigned && fromValue < 0); case TYP_BOOL: + case TYP_BYTE: case TYP_UBYTE: - return (uint8_t)fromValue != fromValue; case TYP_SHORT: - return ((int16_t)fromValue != fromValue) || (fromUnsigned && fromValue < 0); case TYP_USHORT: - return (uint16_t)fromValue != fromValue; case TYP_INT: - return fromUnsigned && (fromValue < 0); case TYP_UINT: - case TYP_ULONG: - return !fromUnsigned && (fromValue < 0); case TYP_LONG: + case TYP_ULONG: + return fromUnsigned ? !FitsIn(toType, static_cast(fromValue)) : !FitsIn(toType, fromValue); + case TYP_FLOAT: case TYP_DOUBLE: return false; + default: unreached(); } @@ -2619,26 +2616,21 @@ bool CastFromLongOverflows(int64_t fromValue, var_types toType, bool fromUnsigne { switch (toType) { - case TYP_BYTE: - return ((int8_t)fromValue != fromValue) || (fromUnsigned && fromValue < 0); case TYP_BOOL: + case TYP_BYTE: case TYP_UBYTE: - return (uint8_t)fromValue != fromValue; case TYP_SHORT: - return ((int16_t)fromValue != fromValue) || (fromUnsigned && fromValue < 0); case TYP_USHORT: - return (uint16_t)fromValue != fromValue; case TYP_INT: - return ((int32_t)fromValue != fromValue) || (fromUnsigned && fromValue < 0); case TYP_UINT: - return (uint32_t)fromValue != fromValue; case TYP_LONG: - return fromUnsigned && (fromValue < 0); case TYP_ULONG: - return !fromUnsigned && (fromValue < 0); + return fromUnsigned ? !FitsIn(toType, static_cast(fromValue)) : !FitsIn(toType, fromValue); + case TYP_FLOAT: case TYP_DOUBLE: return false; + default: unreached(); } diff --git a/src/coreclr/jit/utils.h b/src/coreclr/jit/utils.h index 8599c6dbc810c6..a774967001a49f 100644 --- a/src/coreclr/jit/utils.h +++ b/src/coreclr/jit/utils.h @@ -776,6 +776,36 @@ int64_t GetSigned64Magic(int64_t d, int* shift /*out*/); double CachedCyclesPerSecond(); +template +bool FitsIn(var_types type, T value) +{ + static_assert_no_msg((std::is_same::value || std::is_same::value || + std::is_same::value || std::is_same::value)); + + switch (type) + { + case TYP_BYTE: + return FitsIn(value); + case TYP_BOOL: + case TYP_UBYTE: + return FitsIn(value); + case TYP_SHORT: + return FitsIn(value); + case TYP_USHORT: + return FitsIn(value); + case TYP_INT: + return FitsIn(value); + case TYP_UINT: + return FitsIn(value); + case TYP_LONG: + return FitsIn(value); + case TYP_ULONG: + return FitsIn(value); + default: + unreached(); + } +} + namespace CheckedOps { const bool Unsigned = true; From d35b1329f58200013923adc8a243490e551e1670 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Thu, 19 Aug 2021 14:41:23 +0300 Subject: [PATCH 2/3] Remove a couple superfluous casts --- src/coreclr/jit/assertionprop.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 52246f8fdc76db..3f091edc0f1a0a 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2648,17 +2648,17 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) case TYP_REF: case TYP_INT: // Same type no conversion required - conValTree = gtNewIconNode(static_cast(value)); + conValTree = gtNewIconNode(value); break; case TYP_LONG: // Implicit assignment conversion to larger integer - conValTree = gtNewLconNode(static_cast(value)); + conValTree = gtNewLconNode(value); break; case TYP_FLOAT: // Same sized reinterpretation of bits to float - conValTree = gtNewDconNode(*(reinterpret_cast(&value)), TYP_FLOAT); + conValTree = gtNewDconNode(*reinterpret_cast(&value), TYP_FLOAT); break; case TYP_DOUBLE: From 97f96ddb83004f6a458a90a4a34716604b54fef1 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 16 Aug 2021 20:41:25 +0300 Subject: [PATCH 3/3] Enable VN constant propagation for small types --- src/coreclr/jit/assertionprop.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 3f091edc0f1a0a..852d14ccc3f1b8 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2667,8 +2667,17 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) unreached(); break; + case TYP_BOOL: + case TYP_BYTE: + case TYP_UBYTE: + case TYP_SHORT: + case TYP_USHORT: + assert(FitsIn(tree->TypeGet(), value)); + conValTree = gtNewIconNode(value); + break; + default: - // Do not support (e.g. bool(const int)). + // Do not support (e.g. byref(const int)). break; } }