Skip to content

Commit

Permalink
Use VNVisitReachingVNs in more places (#108420)
Browse files Browse the repository at this point in the history
  • Loading branch information
@EgorBo
EgorBo authored Oct 3, 2024
1 parent b3e8c3c commit b9ffdb0
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 133 deletions.
121 changes: 14 additions & 107 deletions src/coreclr/jit/assertionprop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2322,38 +2322,6 @@ AssertionInfo Compiler::optAssertionGenJtrue(GenTree* tree)
return NO_ASSERTION_INDEX;
}

/*****************************************************************************
*
* Create an assertion on the phi node if some information can be gleaned
* from all of the constituent phi operands.
*
*/
AssertionIndex Compiler::optAssertionGenPhiDefn(GenTree* tree)
{
if (!tree->IsPhiDefn())
{
return NO_ASSERTION_INDEX;
}

// Try to find if all phi arguments are known to be non-null.
bool isNonNull = true;
for (GenTreePhi::Use& use : tree->AsLclVar()->Data()->AsPhi()->Uses())
{
if (!vnStore->IsKnownNonNull(use.GetNode()->gtVNPair.GetConservative()))
{
isNonNull = false;
break;
}
}

// All phi arguments are non-null implies phi rhs is non-null.
if (isNonNull)
{
return optCreateAssertion(tree->AsOp()->gtOp1, nullptr, OAK_NOT_EQUAL);
}
return NO_ASSERTION_INDEX;
}

/*****************************************************************************
*
* If this node creates an assertion then assign an index to the assertion
Expand Down Expand Up @@ -2387,20 +2355,9 @@ void Compiler::optAssertionGen(GenTree* tree)
{
assertionInfo = optCreateAssertion(tree, tree->AsLclVar()->Data(), OAK_EQUAL);
}
else
{
assertionInfo = optAssertionGenPhiDefn(tree);
}
break;

case GT_IND:
// Dynamic block copy sources could be zero-sized and so should not generate assertions.
if (tree->TypeIs(TYP_STRUCT))
{
break;
}
FALLTHROUGH;

case GT_XAND:
case GT_XORR:
case GT_XADD:
Expand Down Expand Up @@ -5097,20 +5054,22 @@ bool Compiler::optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions
GCInfo::WriteBarrierForm barrierType = GCInfo::WriteBarrierForm::WBF_BarrierUnknown;

// First, analyze the value being stored
if (value->IsIntegralConst(0) || (value->gtVNPair.GetConservative() == ValueNumStore::VNForNull()))
{
// The value being stored is null
barrierType = GCInfo::WriteBarrierForm::WBF_NoBarrier;
}
else if (value->IsIconHandle(GTF_ICON_OBJ_HDL) || vnStore->IsVNObjHandle(value->gtVNPair.GetConservative()))
auto vnVisitor = [this](ValueNum vn) -> ValueNumStore::VNVisit {
if ((vn == ValueNumStore::VNForNull()) || vnStore->IsVNObjHandle(vn))
{
// No write barrier is required for null or nongc object handles as values
return ValueNumStore::VNVisit::Continue;
}
return ValueNumStore::VNVisit::Abort;
};

if (vnStore->VNVisitReachingVNs(value->gtVNPair.GetConservative(), vnVisitor) == ValueNumStore::VNVisit::Continue)
{
// The value being stored is a handle
barrierType = GCInfo::WriteBarrierForm::WBF_NoBarrier;
}
// Next, analyze the address if we haven't already determined the barrier type from the value
else if ((indir->gtFlags & GTF_IND_TGT_HEAP) == 0)
{
// Next, analyze the address if we haven't already determined the barrier type from the value
//
// NOTE: we might want to inspect indirs with GTF_IND_TGT_HEAP flag as well - what if we can prove
// that they actually need no barrier? But that comes with a TP regression.
barrierType = GetWriteBarrierForm(vnStore, addr->gtVNPair.GetConservative());
Expand Down Expand Up @@ -5512,63 +5471,11 @@ void Compiler::optImpliedAssertions(AssertionIndex assertionIndex, ASSERT_TP& ac
noway_assert(assertionIndex != 0);
noway_assert(assertionIndex <= optAssertionCount);

AssertionDsc* curAssertion = optGetAssertion(assertionIndex);
if (!BitVecOps::IsEmpty(apTraits, activeAssertions))
{
const ASSERT_TP mappedAssertions = optGetVnMappedAssertions(curAssertion->op1.vn);
if (mappedAssertions == nullptr)
{
return;
}

ASSERT_TP chkAssertions = BitVecOps::MakeCopy(apTraits, mappedAssertions);

if (curAssertion->op2.kind == O2K_LCLVAR_COPY)
{
const ASSERT_TP op2Assertions = optGetVnMappedAssertions(curAssertion->op2.vn);
if (op2Assertions != nullptr)
{
BitVecOps::UnionD(apTraits, chkAssertions, op2Assertions);
}
}
BitVecOps::IntersectionD(apTraits, chkAssertions, activeAssertions);

if (BitVecOps::IsEmpty(apTraits, chkAssertions))
{
return;
}

// Check each assertion in chkAssertions to see if it can be applied to curAssertion
BitVecOps::Iter chkIter(apTraits, chkAssertions);
unsigned chkIndex = 0;
while (chkIter.NextElem(&chkIndex))
{
AssertionIndex chkAssertionIndex = GetAssertionIndex(chkIndex);
if (chkAssertionIndex > optAssertionCount)
{
break;
}
if (chkAssertionIndex == assertionIndex)
{
continue;
}

// Determine which one is a copy assertion and use the other to check for implied assertions.
AssertionDsc* iterAssertion = optGetAssertion(chkAssertionIndex);
if (curAssertion->IsCopyAssertion())
{
optImpliedByCopyAssertion(curAssertion, iterAssertion, activeAssertions);
}
else if (iterAssertion->IsCopyAssertion())
{
optImpliedByCopyAssertion(iterAssertion, curAssertion, activeAssertions);
}
}
}
// Is curAssertion a constant store of a 32-bit integer?
// (i.e GT_LVL_VAR X == GT_CNS_INT)
else if ((curAssertion->assertionKind == OAK_EQUAL) && (curAssertion->op1.kind == O1K_LCLVAR) &&
(curAssertion->op2.kind == O2K_CONST_INT))
AssertionDsc* curAssertion = optGetAssertion(assertionIndex);
if ((curAssertion->assertionKind == OAK_EQUAL) && (curAssertion->op1.kind == O1K_LCLVAR) &&
(curAssertion->op2.kind == O2K_CONST_INT))
{
optImpliedByConstAssertion(curAssertion, activeAssertions);
}
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8078,7 +8078,6 @@ class Compiler
// Assertion Gen functions.
void optAssertionGen(GenTree* tree);
AssertionIndex optAssertionGenCast(GenTreeCast* cast);
AssertionIndex optAssertionGenPhiDefn(GenTree* tree);
AssertionInfo optCreateJTrueBoundsAssertion(GenTree* tree);
AssertionInfo optAssertionGenJtrue(GenTree* tree);
AssertionIndex optCreateJtrueAssertions(GenTree* op1,
Expand Down
62 changes: 37 additions & 25 deletions src/coreclr/jit/valuenum.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1623,32 +1623,24 @@ ValueNumPair ValueNumStore::VNPWithExc(ValueNumPair vnp, ValueNumPair excSetVNP)

bool ValueNumStore::IsKnownNonNull(ValueNum vn)
{
if (vn == NoVN)
{
return false;
}

if (IsVNHandle(vn))
{
assert(CoercedConstantValue<size_t>(vn) != 0);
return true;
}

VNFuncApp funcAttr;
if (!GetVNFunc(vn, &funcAttr))
{
return false;
}

if ((s_vnfOpAttribs[funcAttr.m_func] & VNFOA_KnownNonNull) != 0)
{
return true;
}

// TODO: we can recognize more non-null idioms here, e.g.
// ADD(IsKnownNonNull(op1), smallCns), etc.
auto vnVisitor = [this](ValueNum vn) -> VNVisit {
if (vn != NoVN)
{
if (IsVNHandle(vn))
{
assert(CoercedConstantValue<size_t>(vn) != 0);
return VNVisit::Continue;
}

return false;
VNFuncApp funcAttr;
if (GetVNFunc(vn, &funcAttr) && ((s_vnfOpAttribs[funcAttr.m_func] & VNFOA_KnownNonNull) != 0))
{
return VNVisit::Continue;
}
}
return VNVisit::Abort;
};
return VNVisitReachingVNs(vn, vnVisitor) == VNVisit::Continue;
}

bool ValueNumStore::IsSharedStatic(ValueNum vn)
Expand Down Expand Up @@ -3032,6 +3024,26 @@ bool ValueNumStore::GetPhiDef(ValueNum vn, VNPhiDef* phiDef)
return false;
}

// ----------------------------------------------------------------------------------------
// IsPhiDef - Check if a VN represents a phi definition.
//
// Arguments:
// vn - Value number to check
//
// Return Value:
// True if the VN is a phi def.
//
bool ValueNumStore::IsPhiDef(ValueNum vn) const
{
if (vn == NoVN)
{
return false;
}
Chunk* c = m_chunks.GetNoExpand(GetChunkNum(vn));
assert(ChunkOffset(vn) < c->m_numUsed);
return c->m_attribs == CEA_PhiDef;
}

// ----------------------------------------------------------------------------------------
// VNForMemoryPhiDef - Return a new VN number for a memory phi definition.
//
Expand Down
18 changes: 18 additions & 0 deletions src/coreclr/jit/valuenum.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@
// Defines the type ValueNum.
#include "valuenumtype.h"
// Defines the type SmallHashTable.
#include "compiler.h"
#include "smallhash.h"

// A "ValueNumStore" represents the "universe" of value numbers used in a single
Expand Down Expand Up @@ -594,6 +595,20 @@ class ValueNumStore
//
template <typename TArgVisitor>
VNVisit VNVisitReachingVNs(ValueNum vn, TArgVisitor argVisitor)
{
// Fast-path: in most cases vn is not a phi definition
if (!IsPhiDef(vn))
{
return argVisitor(vn);
}
return VNVisitReachingVNsWorker(vn, argVisitor);
}

private:

// Helper function for VNVisitReachingVNs
template <typename TArgVisitor>
VNVisit VNVisitReachingVNsWorker(ValueNum vn, TArgVisitor argVisitor)
{
ArrayStack<ValueNum> toVisit(m_alloc);
toVisit.Push(vn);
Expand Down Expand Up @@ -630,6 +645,8 @@ class ValueNumStore
return VNVisit::Continue;
}

public:

// And the single constant for an object reference type.
static ValueNum VNForNull()
{
Expand Down Expand Up @@ -818,6 +835,7 @@ class ValueNumStore

ValueNum VNForPhiDef(var_types type, unsigned lclNum, unsigned ssaDef, ArrayStack<unsigned>& ssaArgs);
bool GetPhiDef(ValueNum vn, VNPhiDef* phiDef);
bool IsPhiDef(ValueNum vn) const;
ValueNum VNForMemoryPhiDef(BasicBlock* block, ArrayStack<unsigned>& vns);
bool GetMemoryPhiDef(ValueNum vn, VNMemoryPhiDef* memoryPhiDef);

Expand Down

0 comments on commit b9ffdb0

Please sign in to comment.