Skip to content

Commit a950953

Browse files
EgorBoEgorBo
authored
Remove bounds checks for Log2 function in FormattingHelpers.CountDigits (#113790)
1 parent 9c7434b commit a950953

File tree

4 files changed

+140
-7
lines changed

4 files changed

+140
-7
lines changed

‎src/coreclr/jit/rangecheck.cpp

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,7 +1048,20 @@ void RangeCheck::MergeAssertion(BasicBlock* block, GenTree* op, Range* pRange DE
10481048
// Compute the range for a binary operation.
10491049
Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool monIncreasing DEBUGARG(int indent))
10501050
{
1051-
assert(binop->OperIs(GT_ADD, GT_AND, GT_RSH, GT_RSZ, GT_LSH, GT_UMOD, GT_MUL));
1051+
assert(binop->OperIs(GT_ADD, GT_XOR, GT_AND, GT_RSH, GT_RSZ, GT_LSH, GT_UMOD, GT_MUL));
1052+
1053+
// For XOR we only care about Log2 pattern for now
1054+
if (binop->OperIs(GT_XOR))
1055+
{
1056+
int upperBound;
1057+
if (m_pCompiler->vnStore->IsVNLog2(m_pCompiler->vnStore->VNConservativeNormalValue(binop->gtVNPair),
1058+
&upperBound))
1059+
{
1060+
assert(upperBound > 0);
1061+
return Range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, upperBound));
1062+
}
1063+
return Range(Limit(Limit::keUnknown));
1064+
}
10521065

10531066
GenTree* op1 = binop->gtGetOp1();
10541067
GenTree* op2 = binop->gtGetOp2();
@@ -1472,6 +1485,8 @@ bool RangeCheck::DoesOverflow(BasicBlock* block, GenTree* expr, const Range& ran
14721485

14731486
bool RangeCheck::ComputeDoesOverflow(BasicBlock* block, GenTree* expr, const Range& range)
14741487
{
1488+
ValueNumStore* vnStore = m_pCompiler->vnStore;
1489+
14751490
JITDUMP("Does overflow [%06d]?\n", Compiler::dspTreeID(expr));
14761491
GetSearchPath()->Set(expr, block, SearchPath::Overwrite);
14771492

@@ -1482,7 +1497,7 @@ bool RangeCheck::ComputeDoesOverflow(BasicBlock* block, GenTree* expr, const Ran
14821497
overflows = true;
14831498
}
14841499
// If the definition chain resolves to a constant, it doesn't overflow.
1485-
else if (m_pCompiler->vnStore->IsVNConstant(expr->gtVNPair.GetConservative()))
1500+
else if (vnStore->IsVNConstant(expr->gtVNPair.GetConservative()))
14861501
{
14871502
overflows = false;
14881503
}
@@ -1510,6 +1525,11 @@ bool RangeCheck::ComputeDoesOverflow(BasicBlock* block, GenTree* expr, const Ran
15101525
{
15111526
overflows = false;
15121527
}
1528+
else if (expr->OperIs(GT_XOR) && vnStore->IsVNLog2(m_pCompiler->vnStore->VNConservativeNormalValue(expr->gtVNPair)))
1529+
{
1530+
// For XOR we only care about Log2 pattern for now, which never overflows.
1531+
overflows = false;
1532+
}
15131533
// Walk through phi arguments to check if phi arguments involve arithmetic that overflows.
15141534
else if (expr->OperIs(GT_PHI))
15151535
{
@@ -1597,7 +1617,7 @@ Range RangeCheck::ComputeRange(BasicBlock* block, GenTree* expr, bool monIncreas
15971617
MergeAssertion(block, expr, &range DEBUGARG(indent + 1));
15981618
}
15991619
// compute the range for binary operation
1600-
else if (expr->OperIs(GT_ADD, GT_AND, GT_RSH, GT_RSZ, GT_LSH, GT_UMOD, GT_MUL))
1620+
else if (expr->OperIs(GT_XOR, GT_ADD, GT_AND, GT_RSH, GT_RSZ, GT_LSH, GT_UMOD, GT_MUL))
16011621
{
16021622
range = ComputeRangeForBinOp(block, expr->AsOp(), monIncreasing DEBUGARG(indent + 1));
16031623
}

‎src/coreclr/jit/valuenum.cpp

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6601,6 +6601,48 @@ bool ValueNumStore::IsVNInt32Constant(ValueNum vn)
66016601
return TypeOfVN(vn) == TYP_INT;
66026602
}
66036603

6604+
//------------------------------------------------------------------------
6605+
// IsVNLog2: Determine if the value number is a log2 pattern, which is
6606+
// "XOR(LZCNT32(OR(X, 1), 31)" or "XOR(LZCNT64(OR(X, 1), 63))".
6607+
//
6608+
// Arguments:
6609+
// vn - the value number to analyze
6610+
// upperBound - if not null, will be set to the upper bound of the log2 pattern (31 or 63)
6611+
//
6612+
// Return value:
6613+
// true if the value number is a log2 pattern, false otherwise.
6614+
//
6615+
bool ValueNumStore::IsVNLog2(ValueNum vn, int* upperBound)
6616+
{
6617+
#if defined(FEATURE_HW_INTRINSICS) && (defined(TARGET_XARCH) || defined(TARGET_ARM64))
6618+
int xorBy;
6619+
ValueNum op;
6620+
// First, see if it's "X ^ 31" or "X ^ 63".
6621+
if (IsVNBinFuncWithConst(vn, VNF_XOR, &op, &xorBy) && ((xorBy == 31) || (xorBy == 63)))
6622+
{
6623+
// Drop any integer cast if any, we're dealing with [0..63] range, any integer cast is redundant.
6624+
IsVNBinFunc(op, VNF_Cast, &op);
6625+
6626+
#ifdef TARGET_XARCH
6627+
VNFunc lzcntFunc = (xorBy == 31) ? VNF_HWI_LZCNT_LeadingZeroCount : VNF_HWI_LZCNT_X64_LeadingZeroCount;
6628+
#else
6629+
VNFunc lzcntFunc = (xorBy == 31) ? VNF_HWI_ArmBase_LeadingZeroCount : VNF_HWI_ArmBase_Arm64_LeadingZeroCount;
6630+
#endif
6631+
// Next, see if it's "LZCNT32(X | 1)" or "LZCNT64(X | 1)".
6632+
int orBy;
6633+
if (IsVNBinFunc(op, lzcntFunc, &op) && IsVNBinFuncWithConst(op, VNF_OR, &op, &orBy) && (orBy == 1))
6634+
{
6635+
if (upperBound != nullptr)
6636+
{
6637+
*upperBound = xorBy;
6638+
}
6639+
return true;
6640+
}
6641+
}
6642+
#endif
6643+
return false;
6644+
}
6645+
66046646
//------------------------------------------------------------------------
66056647
// IsVNNeverNegative: Determines if the given value number can never take on a negative value
66066648
// in a signed context (i.e. when the most-significant bit represents signedness).
@@ -6676,6 +6718,13 @@ bool ValueNumStore::IsVNNeverNegative(ValueNum vn)
66766718
case VNF_HWI_ArmBase_Arm64_LeadingSignCount:
66776719
return VNVisit::Continue;
66786720
#endif
6721+
case VNF_XOR:
6722+
if (IsVNLog2(vn))
6723+
{
6724+
return VNVisit::Continue;
6725+
}
6726+
break;
6727+
66796728
#endif // FEATURE_HW_INTRINSICS
66806729

66816730
default:
@@ -9978,6 +10027,37 @@ bool ValueNumStore::GetVNFunc(ValueNum vn, VNFuncApp* funcApp)
997810027
return false;
997910028
}
998010029

10030+
//----------------------------------------------------------------------------------
10031+
// IsVNBinFunc: A specialized version of GetVNFunc that checks if the given ValueNum
10032+
// is the given VNFunc with arity 2. If so, it returns the two operands.
10033+
//
10034+
// Arguments:
10035+
// vn - The ValueNum to check.
10036+
// func - The VNFunc to check for.
10037+
// op1 - The first operand (if not null).
10038+
// op2 - The second operand (if not null).
10039+
//
10040+
// Return Value:
10041+
// true if the given vn is the given VNFunc with two operands.
10042+
//
10043+
bool ValueNumStore::IsVNBinFunc(ValueNum vn, VNFunc func, ValueNum* op1, ValueNum* op2)
10044+
{
10045+
VNFuncApp funcApp;
10046+
if (GetVNFunc(vn, &funcApp) && (funcApp.m_func == func) && (funcApp.m_arity == 2))
10047+
{
10048+
if (op1 != nullptr)
10049+
{
10050+
*op1 = funcApp.m_args[0];
10051+
}
10052+
if (op2 != nullptr)
10053+
{
10054+
*op2 = funcApp.m_args[1];
10055+
}
10056+
return true;
10057+
}
10058+
return false;
10059+
}
10060+
998110061
bool ValueNumStore::VNIsValid(ValueNum vn)
998210062
{
998310063
ChunkNum cn = GetChunkNum(vn);

‎src/coreclr/jit/valuenum.h

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,9 @@ class ValueNumStore
10561056
// Returns true if the VN represents a node that is never negative.
10571057
bool IsVNNeverNegative(ValueNum vn);
10581058

1059+
// Returns true if the VN represents BitOperations.Log2 pattern
1060+
bool IsVNLog2(ValueNum vn, int* upperBound = nullptr);
1061+
10591062
typedef SmallHashTable<ValueNum, bool, 8U> CheckedBoundVNSet;
10601063

10611064
// Returns true if the VN is known or likely to appear as the conservative value number
@@ -1409,6 +1412,38 @@ class ValueNumStore
14091412
// the function application it represents; otherwise, return "false."
14101413
bool GetVNFunc(ValueNum vn, VNFuncApp* funcApp);
14111414

1415+
// Returns "true" iff "vn" is a function application of the form "func(op1, op2)".
1416+
bool IsVNBinFunc(ValueNum vn, VNFunc func, ValueNum* op1 = nullptr, ValueNum* op2 = nullptr);
1417+
1418+
// Returns "true" iff "vn" is a function application of the form "func(op, cns)"
1419+
// the cns can be on the left side if the function is commutative.
1420+
template <typename T>
1421+
bool IsVNBinFuncWithConst(ValueNum vn, VNFunc func, ValueNum* op, T* cns)
1422+
{
1423+
T opCns;
1424+
ValueNum op1, op2;
1425+
if (IsVNBinFunc(vn, func, &op1, &op2))
1426+
{
1427+
if (IsVNIntegralConstant(op2, &opCns))
1428+
{
1429+
if (op != nullptr)
1430+
*op = op1;
1431+
if (cns != nullptr)
1432+
*cns = opCns;
1433+
return true;
1434+
}
1435+
else if (VNFuncIsCommutative(func) && IsVNIntegralConstant(op1, &opCns))
1436+
{
1437+
if (op != nullptr)
1438+
*op = op2;
1439+
if (cns != nullptr)
1440+
*cns = opCns;
1441+
return true;
1442+
}
1443+
}
1444+
return false;
1445+
}
1446+
14121447
// Returns "true" iff "vn" is a valid value number -- one that has been previously returned.
14131448
bool VNIsValid(ValueNum vn);
14141449

‎src/libraries/System.Private.CoreLib/src/System/Buffers/Text/FormattingHelpers.CountDigits.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ public static int CountDigits(ulong value)
2424
];
2525
Debug.Assert(log2ToPow10.Length == 64);
2626

27-
// TODO: Replace with log2ToPow10[BitOperations.Log2(value)] once https://github.com/dotnet/runtime/issues/79257 is fixed
28-
nint elementOffset = Unsafe.Add(ref MemoryMarshal.GetReference(log2ToPow10), BitOperations.Log2(value));
27+
nint elementOffset = log2ToPow10[(int)ulong.Log2(value)];
2928

3029
// Read the associated power of 10.
3130
ReadOnlySpan<ulong> powersOf10 =
@@ -102,8 +101,7 @@ public static int CountDigits(uint value)
102101
];
103102
Debug.Assert(table.Length == 32, "Every result of uint.Log2(value) needs a long entry in the table.");
104103

105-
// TODO: Replace with table[uint.Log2(value)] once https://github.com/dotnet/runtime/issues/79257 is fixed
106-
long tableValue = Unsafe.Add(ref MemoryMarshal.GetReference(table), uint.Log2(value));
104+
long tableValue = table[(int)uint.Log2(value)];
107105
return (int)((value + tableValue) >> 32);
108106
}
109107

0 commit comments

Comments
 (0)