Skip to content

Commit 86cb7b9

Browse files
authored
JIT: use VNVisitReachingVNs in IsVNNeverNegative (#105197)
1 parent 5377b58 commit 86cb7b9

File tree

2 files changed

+225
-104
lines changed

2 files changed

+225
-104
lines changed

src/coreclr/jit/valuenum.cpp

Lines changed: 124 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1635,7 +1635,20 @@ bool ValueNumStore::IsKnownNonNull(ValueNum vn)
16351635
}
16361636

16371637
VNFuncApp funcAttr;
1638-
return GetVNFunc(vn, &funcAttr) && (s_vnfOpAttribs[funcAttr.m_func] & VNFOA_KnownNonNull) != 0;
1638+
if (!GetVNFunc(vn, &funcAttr))
1639+
{
1640+
return false;
1641+
}
1642+
1643+
if ((s_vnfOpAttribs[funcAttr.m_func] & VNFOA_KnownNonNull) != 0)
1644+
{
1645+
return true;
1646+
}
1647+
1648+
// TODO: we can recognize more non-null idioms here, e.g.
1649+
// ADD(IsKnownNonNull(op1), smallCns), etc.
1650+
1651+
return false;
16391652
}
16401653

16411654
bool ValueNumStore::IsSharedStatic(ValueNum vn)
@@ -3191,81 +3204,81 @@ ValueNum ValueNumStore::VNForMapPhysicalSelect(
31913204
return result;
31923205
}
31933206

3194-
typedef JitHashTable<ValueNum, JitSmallPrimitiveKeyFuncs<ValueNum>, bool> ValueNumSet;
3195-
3196-
class SmallValueNumSet
3207+
bool ValueNumStore::SmallValueNumSet::Lookup(ValueNum vn)
31973208
{
3198-
union
3209+
// O(N) lookup for inline elements
3210+
if (m_numElements <= ArrLen(m_inlineElements))
31993211
{
3200-
ValueNum m_inlineElements[4];
3201-
ValueNumSet* m_set;
3202-
};
3203-
unsigned m_numElements = 0;
3204-
3205-
public:
3206-
unsigned Count()
3207-
{
3208-
return m_numElements;
3209-
}
3210-
3211-
template <typename Func>
3212-
void ForEach(Func func)
3213-
{
3214-
if (m_numElements <= ArrLen(m_inlineElements))
3212+
for (unsigned i = 0; i < m_numElements; i++)
32153213
{
3216-
for (unsigned i = 0; i < m_numElements; i++)
3214+
if (m_inlineElements[i] == vn)
32173215
{
3218-
func(m_inlineElements[i]);
3219-
}
3220-
}
3221-
else
3222-
{
3223-
for (ValueNum vn : ValueNumSet::KeyIteration(m_set))
3224-
{
3225-
func(vn);
3216+
return true;
32263217
}
32273218
}
3219+
3220+
// Not found
3221+
return false;
32283222
}
32293223

3230-
void Add(Compiler* comp, ValueNum vn)
3224+
return m_set->Lookup(vn);
3225+
}
3226+
3227+
// Returns false if the value already exists
3228+
bool ValueNumStore::SmallValueNumSet::Add(Compiler* comp, ValueNum vn)
3229+
{
3230+
if (m_numElements <= ArrLen(m_inlineElements))
32313231
{
3232-
if (m_numElements <= ArrLen(m_inlineElements))
3232+
for (unsigned i = 0; i < m_numElements; i++)
32333233
{
3234-
for (unsigned i = 0; i < m_numElements; i++)
3234+
if (m_inlineElements[i] == vn)
32353235
{
3236-
if (m_inlineElements[i] == vn)
3237-
{
3238-
return;
3239-
}
3236+
// Already exists
3237+
return false;
32403238
}
3239+
}
32413240

3242-
if (m_numElements < ArrLen(m_inlineElements))
3241+
if (m_numElements < ArrLen(m_inlineElements))
3242+
{
3243+
m_inlineElements[m_numElements] = vn;
3244+
m_numElements++;
3245+
}
3246+
else
3247+
{
3248+
ValueNumSet* set = new (comp, CMK_ValueNumber) ValueNumSet(comp->getAllocator(CMK_ValueNumber));
3249+
for (ValueNum oldVn : m_inlineElements)
32433250
{
3244-
m_inlineElements[m_numElements] = vn;
3245-
m_numElements++;
3251+
set->Set(oldVn, true);
32463252
}
3247-
else
3248-
{
3249-
ValueNumSet* set = new (comp, CMK_ValueNumber) ValueNumSet(comp->getAllocator(CMK_ValueNumber));
3250-
for (ValueNum oldVn : m_inlineElements)
3251-
{
3252-
set->Set(oldVn, true);
3253-
}
32543253

3255-
set->Set(vn, true);
3254+
set->Set(vn, true);
32563255

3257-
m_set = set;
3258-
m_numElements++;
3259-
assert(m_numElements == set->GetCount());
3260-
}
3261-
}
3262-
else
3263-
{
3264-
m_set->Set(vn, true, ValueNumSet::SetKind::Overwrite);
3265-
m_numElements = m_set->GetCount();
3256+
m_set = set;
3257+
m_numElements++;
3258+
assert(m_numElements == set->GetCount());
32663259
}
3260+
return true;
32673261
}
3268-
};
3262+
3263+
bool exists = m_set->Set(vn, true, ValueNumSet::SetKind::Overwrite);
3264+
m_numElements = m_set->GetCount();
3265+
return !exists;
3266+
}
3267+
3268+
//------------------------------------------------------------------------------
3269+
// VNPhiDefToVN: Extracts the VN for a specific argument of a phi definition.
3270+
//
3271+
// Arguments:
3272+
// phiDef - The phi definition
3273+
// ssaArgNum - The argument number to extract
3274+
//
3275+
// Return Value:
3276+
// The VN for the specified argument of the phi definition.
3277+
//
3278+
ValueNum ValueNumStore::VNPhiDefToVN(const VNPhiDef& phiDef, unsigned ssaArgNum)
3279+
{
3280+
return m_pComp->lvaGetDesc(phiDef.LclNum)->GetPerSsaData(phiDef.SsaArgs[ssaArgNum])->m_vnPair.Get(VNK_Conservative);
3281+
}
32693282

32703283
//------------------------------------------------------------------------------
32713284
// VNForMapSelectInner: Select value from a map and record loop memory dependencies.
@@ -6513,68 +6526,75 @@ bool ValueNumStore::IsVNInt32Constant(ValueNum vn)
65136526

65146527
bool ValueNumStore::IsVNNeverNegative(ValueNum vn)
65156528
{
6516-
assert(varTypeIsIntegral(TypeOfVN(vn)));
6517-
6518-
if (IsVNConstant(vn))
6519-
{
6520-
var_types vnTy = TypeOfVN(vn);
6521-
if (vnTy == TYP_INT)
6529+
auto vnVisitor = [this](ValueNum vn) -> VNVisit {
6530+
if ((vn == NoVN) || !varTypeIsIntegral(TypeOfVN(vn)))
65226531
{
6523-
return GetConstantInt32(vn) >= 0;
6532+
return VNVisit::Abort;
65246533
}
6525-
else if (vnTy == TYP_LONG)
6534+
6535+
if (IsVNConstant(vn))
65266536
{
6527-
return GetConstantInt64(vn) >= 0;
6537+
var_types vnTy = TypeOfVN(vn);
6538+
if (vnTy == TYP_INT)
6539+
{
6540+
return GetConstantInt32(vn) >= 0 ? VNVisit::Continue : VNVisit::Abort;
6541+
}
6542+
if (vnTy == TYP_LONG)
6543+
{
6544+
return GetConstantInt64(vn) >= 0 ? VNVisit::Continue : VNVisit::Abort;
6545+
}
6546+
return VNVisit::Abort;
65286547
}
65296548

6530-
return false;
6531-
}
6549+
// Array length can never be negative.
6550+
if (IsVNArrLen(vn))
6551+
{
6552+
return VNVisit::Continue;
6553+
}
65326554

6533-
// Array length can never be negative.
6534-
if (IsVNArrLen(vn))
6535-
{
6536-
return true;
6537-
}
6555+
// TODO-VN: Recognize Span.Length
6556+
// Handle more intrinsics such as Math.Max(neverNegative1, neverNegative2)
65386557

6539-
VNFuncApp funcApp;
6540-
if (GetVNFunc(vn, &funcApp))
6541-
{
6542-
switch (funcApp.m_func)
6558+
VNFuncApp funcApp;
6559+
if (GetVNFunc(vn, &funcApp))
65436560
{
6544-
case VNF_GE_UN:
6545-
case VNF_GT_UN:
6546-
case VNF_LE_UN:
6547-
case VNF_LT_UN:
6548-
case VNF_COUNT:
6549-
case VNF_ADD_UN_OVF:
6550-
case VNF_SUB_UN_OVF:
6551-
case VNF_MUL_UN_OVF:
6561+
switch (funcApp.m_func)
6562+
{
6563+
case VNF_GE_UN:
6564+
case VNF_GT_UN:
6565+
case VNF_LE_UN:
6566+
case VNF_LT_UN:
6567+
case VNF_COUNT:
6568+
case VNF_ADD_UN_OVF:
6569+
case VNF_SUB_UN_OVF:
6570+
case VNF_MUL_UN_OVF:
65526571
#ifdef FEATURE_HW_INTRINSICS
65536572
#ifdef TARGET_XARCH
6554-
case VNF_HWI_POPCNT_PopCount:
6555-
case VNF_HWI_POPCNT_X64_PopCount:
6556-
case VNF_HWI_LZCNT_LeadingZeroCount:
6557-
case VNF_HWI_LZCNT_X64_LeadingZeroCount:
6558-
case VNF_HWI_BMI1_TrailingZeroCount:
6559-
case VNF_HWI_BMI1_X64_TrailingZeroCount:
6560-
return true;
6573+
case VNF_HWI_POPCNT_PopCount:
6574+
case VNF_HWI_POPCNT_X64_PopCount:
6575+
case VNF_HWI_LZCNT_LeadingZeroCount:
6576+
case VNF_HWI_LZCNT_X64_LeadingZeroCount:
6577+
case VNF_HWI_BMI1_TrailingZeroCount:
6578+
case VNF_HWI_BMI1_X64_TrailingZeroCount:
6579+
return VNVisit::Continue;
65616580
#elif defined(TARGET_ARM64)
6562-
case VNF_HWI_AdvSimd_PopCount:
6563-
case VNF_HWI_AdvSimd_LeadingZeroCount:
6564-
case VNF_HWI_AdvSimd_LeadingSignCount:
6565-
case VNF_HWI_ArmBase_LeadingZeroCount:
6566-
case VNF_HWI_ArmBase_Arm64_LeadingZeroCount:
6567-
case VNF_HWI_ArmBase_Arm64_LeadingSignCount:
6568-
return true;
6581+
case VNF_HWI_AdvSimd_PopCount:
6582+
case VNF_HWI_AdvSimd_LeadingZeroCount:
6583+
case VNF_HWI_AdvSimd_LeadingSignCount:
6584+
case VNF_HWI_ArmBase_LeadingZeroCount:
6585+
case VNF_HWI_ArmBase_Arm64_LeadingZeroCount:
6586+
case VNF_HWI_ArmBase_Arm64_LeadingSignCount:
6587+
return VNVisit::Continue;
65696588
#endif
65706589
#endif // FEATURE_HW_INTRINSICS
65716590

6572-
default:
6573-
break;
6591+
default:
6592+
break;
6593+
}
65746594
}
6575-
}
6576-
6577-
return false;
6595+
return VNVisit::Abort;
6596+
};
6597+
return VNVisitReachingVNs(vn, vnVisitor) == VNVisit::Continue;
65786598
}
65796599

65806600
GenTreeFlags ValueNumStore::GetHandleFlags(ValueNum vn)

src/coreclr/jit/valuenum.h

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,107 @@ class ValueNumStore
529529

530530
void PeelOffsets(ValueNum* vn, target_ssize_t* offset);
531531

532+
typedef JitHashTable<ValueNum, JitSmallPrimitiveKeyFuncs<ValueNum>, bool> ValueNumSet;
533+
534+
class SmallValueNumSet
535+
{
536+
union
537+
{
538+
ValueNum m_inlineElements[4];
539+
ValueNumSet* m_set;
540+
};
541+
unsigned m_numElements = 0;
542+
543+
public:
544+
unsigned Count()
545+
{
546+
return m_numElements;
547+
}
548+
549+
template <typename Func>
550+
void ForEach(Func func)
551+
{
552+
if (m_numElements <= ArrLen(m_inlineElements))
553+
{
554+
for (unsigned i = 0; i < m_numElements; i++)
555+
{
556+
func(m_inlineElements[i]);
557+
}
558+
}
559+
else
560+
{
561+
for (ValueNum vn : ValueNumSet::KeyIteration(m_set))
562+
{
563+
func(vn);
564+
}
565+
}
566+
}
567+
568+
// Returns false if the value wasn't found
569+
bool Lookup(ValueNum vn);
570+
571+
// Returns false if the value already exists
572+
bool Add(Compiler* comp, ValueNum vn);
573+
};
574+
575+
enum class VNVisit
576+
{
577+
Continue,
578+
Abort,
579+
};
580+
581+
ValueNum VNPhiDefToVN(const VNPhiDef& phiDef, unsigned ssaArgNum);
582+
583+
//--------------------------------------------------------------------------------
584+
// VNVisitReachingVNs: given a VN, call the specified callback function on it and all the VNs that reach it
585+
// via PHI definitions if any.
586+
//
587+
// Arguments:
588+
// vn - The VN to visit all the reaching VNs for
589+
// argVisitor - The callback function to call on the vn and its PHI arguments if any
590+
//
591+
// Return Value:
592+
// VNVisit::Aborted - an argVisitor returned VNVisit::Abort, we stop the walk and return
593+
// VNVisit::Continue - all argVisitor returned VNVisit::Continue
594+
//
595+
template <typename TArgVisitor>
596+
VNVisit VNVisitReachingVNs(ValueNum vn, TArgVisitor argVisitor)
597+
{
598+
ArrayStack<ValueNum> toVisit(m_alloc);
599+
toVisit.Push(vn);
600+
601+
SmallValueNumSet visited;
602+
visited.Add(m_pComp, vn);
603+
while (toVisit.Height() > 0)
604+
{
605+
ValueNum vnToVisit = toVisit.Pop();
606+
607+
// We need to handle nested (and, potentially, recursive) phi definitions.
608+
// For now, we ignore memory phi definitions.
609+
VNPhiDef phiDef;
610+
if (GetPhiDef(vnToVisit, &phiDef))
611+
{
612+
for (unsigned ssaArgNum = 0; ssaArgNum < phiDef.NumArgs; ssaArgNum++)
613+
{
614+
ValueNum childVN = VNPhiDefToVN(phiDef, ssaArgNum);
615+
if (visited.Add(m_pComp, childVN))
616+
{
617+
toVisit.Push(childVN);
618+
}
619+
}
620+
}
621+
else
622+
{
623+
if (argVisitor(vnToVisit) == VNVisit::Abort)
624+
{
625+
// The visitor wants to abort the walk.
626+
return VNVisit::Abort;
627+
}
628+
}
629+
}
630+
return VNVisit::Continue;
631+
}
632+
532633
// And the single constant for an object reference type.
533634
static ValueNum VNForNull()
534635
{

0 commit comments

Comments
 (0)