Skip to content

Commit

Permalink
Enable global constant propagation for small types (dotnet#57726)
Browse files Browse the repository at this point in the history
* Centralize the "FitsIn" logic

So that there is one canonical way to check
if a type can represent some value.

* Remove a couple superfluous casts

* Enable VN constant propagation for small types
  • Loading branch information
SingleAccretion authored Sep 10, 2021
1 parent 331823f commit 84bad7e
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 53 deletions.
17 changes: 13 additions & 4 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2647,17 +2647,17 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree)
case TYP_REF:
case TYP_INT:
// Same type no conversion required
conValTree = gtNewIconNode(static_cast<int>(value));
conValTree = gtNewIconNode(value);
break;

case TYP_LONG:
// Implicit assignment conversion to larger integer
conValTree = gtNewLconNode(static_cast<int>(value));
conValTree = gtNewLconNode(value);
break;

case TYP_FLOAT:
// Same sized reinterpretation of bits to float
conValTree = gtNewDconNode(*(reinterpret_cast<float*>(&value)), TYP_FLOAT);
conValTree = gtNewDconNode(*reinterpret_cast<float*>(&value), TYP_FLOAT);
break;

case TYP_DOUBLE:
Expand All @@ -2666,8 +2666,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;
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6035,7 +6035,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<uint8_t>(op2->AsIntCon()->IconValue())))
{
type = TYP_UBYTE;
}
Expand Down Expand Up @@ -6105,8 +6105,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
Expand Down
28 changes: 0 additions & 28 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<UINT8>(value);
case TYP_BYTE:
return FitsIn<INT8>(value);
case TYP_USHORT:
return FitsIn<UINT16>(value);
case TYP_SHORT:
return FitsIn<INT16>(value);
default:
unreached();
}
}

/*****************************************************************************
*
* Return the size in bytes of the given type.
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
26 changes: 9 additions & 17 deletions src/coreclr/jit/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2601,24 +2601,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<uint32_t>(fromValue)) : !FitsIn(toType, fromValue);

case TYP_FLOAT:
case TYP_DOUBLE:
return false;

default:
unreached();
}
Expand All @@ -2628,26 +2625,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<uint64_t>(fromValue)) : !FitsIn(toType, fromValue);

case TYP_FLOAT:
case TYP_DOUBLE:
return false;

default:
unreached();
}
Expand Down
30 changes: 30 additions & 0 deletions src/coreclr/jit/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,36 @@ int64_t GetSigned64Magic(int64_t d, int* shift /*out*/);

double CachedCyclesPerSecond();

template <typename T>
bool FitsIn(var_types type, T value)
{
static_assert_no_msg((std::is_same<T, int32_t>::value || std::is_same<T, int64_t>::value ||
std::is_same<T, uint32_t>::value || std::is_same<T, uint64_t>::value));

switch (type)
{
case TYP_BYTE:
return FitsIn<int8_t>(value);
case TYP_BOOL:
case TYP_UBYTE:
return FitsIn<uint8_t>(value);
case TYP_SHORT:
return FitsIn<int16_t>(value);
case TYP_USHORT:
return FitsIn<uint16_t>(value);
case TYP_INT:
return FitsIn<int32_t>(value);
case TYP_UINT:
return FitsIn<uint32_t>(value);
case TYP_LONG:
return FitsIn<int64_t>(value);
case TYP_ULONG:
return FitsIn<uint64_t>(value);
default:
unreached();
}
}

namespace CheckedOps
{
const bool Unsigned = true;
Expand Down

0 comments on commit 84bad7e

Please sign in to comment.