Skip to content

JIT: Remove VNFOA_Commutative from value numbering attribute table #126895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 1 commit into from
Closed

JIT: Remove VNFOA_Commutative from value numbering attribute table #126895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 43 additions & 30 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,42 @@ unsigned ValueNumStore::VNFuncArity(VNFunc vnf)
return (s_vnfOpAttribs[vnf] & VNFOA_ArityMask) >> VNFOA_ArityShift;
}

// static
bool ValueNumStore::VNFuncIsCommutative(VNFunc vnf)
{
if (vnf < VNF_Boundary)
{
return GenTree::OperIsCommutative(static_cast<genTreeOps>(vnf));
}

switch (vnf)
{
case VNF_ADD_OVF:
case VNF_MUL_OVF:
case VNF_ADD_UN_OVF:
case VNF_MUL_UN_OVF:
#if defined(TARGET_RISCV64)
case VNF_MinInt:
case VNF_MaxInt:
case VNF_MinInt_UN:
case VNF_MaxInt_UN:
#endif
return true;
default:
break;
}

#ifdef FEATURE_HW_INTRINSICS
if ((vnf >= VNF_HWI_FIRST) && (vnf <= VNF_HWI_LAST))
{
NamedIntrinsic id = NamedIntrinsic((vnf - VNF_HWI_FIRST) + (NI_HW_INTRINSIC_START + 1));
return HWIntrinsicInfo::IsCommutative(id);
}
#endif

return false;
}

template <>
bool ValueNumStore::IsOverflowIntDiv(int v0, int v1)
{
Expand Down Expand Up @@ -10588,29 +10624,27 @@ void ValueNumStore::vnDumpZeroObj(Compiler* comp, VNFuncApp* zeroObj)
}

/* static */ constexpr uint8_t ValueNumStore::GetOpAttribsForGenTree(genTreeOps oper,
bool commute,
bool illegalAsVNFunc,
GenTreeOperKind kind)
{
return GetOpAttribsForArity(oper, kind) | (static_cast<uint8_t>(commute) << VNFOA_CommutativeShift) |
return GetOpAttribsForArity(oper, kind) |
(static_cast<uint8_t>(illegalAsVNFunc) << VNFOA_IllegalGenTreeOpShift);
}

/* static */ constexpr uint8_t ValueNumStore::GetOpAttribsForFunc(int arity, bool commute, bool knownNonNull)
/* static */ constexpr uint8_t ValueNumStore::GetOpAttribsForFunc(int arity, bool knownNonNull)
{
return (static_cast<uint8_t>(commute) << VNFOA_CommutativeShift) |
(static_cast<uint8_t>(knownNonNull) << VNFOA_KnownNonNullShift) |
return (static_cast<uint8_t>(knownNonNull) << VNFOA_KnownNonNullShift) |
((static_cast<uint8_t>(arity & ~(arity >> 31)) << VNFOA_ArityShift) & VNFOA_ArityMask);
}

const uint8_t ValueNumStore::s_vnfOpAttribs[VNF_COUNT] = {
#define GTNODE(en, st, cm, ivn, ok) \
GetOpAttribsForGenTree(static_cast<genTreeOps>(GT_##en), cm, ivn, static_cast<GenTreeOperKind>(ok)),
GetOpAttribsForGenTree(static_cast<genTreeOps>(GT_##en), ivn, static_cast<GenTreeOperKind>(ok)),
#include "gtlist.h"

0, // VNF_Boundary

#define ValueNumFuncDef(vnf, arity, commute, knownNonNull) GetOpAttribsForFunc(arity, commute, knownNonNull),
#define ValueNumFuncDef(vnf, arity, knownNonNull) GetOpAttribsForFunc(arity, knownNonNull),
#include "valuenumfuncs.h"
};

Expand Down Expand Up @@ -10654,20 +10688,13 @@ void ValueNumStore::ValidateValueNumStoreStatics()
}

arr[i] |= ((arity << VNFOA_ArityShift) & VNFOA_ArityMask);

if (GenTree::OperIsCommutative(gtOper))
{
arr[i] |= VNFOA_Commutative;
}
}

// I so wish this wasn't the best way to do this...

int vnfNum = VNF_Boundary + 1; // The macro definition below will update this after using it.

#define ValueNumFuncDef(vnf, arity, commute, knownNonNull) \
if (commute) \
arr[vnfNum] |= VNFOA_Commutative; \
#define ValueNumFuncDef(vnf, arity, knownNonNull) \
if (knownNonNull) \
arr[vnfNum] |= VNFOA_KnownNonNull; \
if (arity > 0) \
Expand All @@ -10682,20 +10709,6 @@ void ValueNumStore::ValidateValueNumStoreStatics()
arr[vnfNum] &= ~VNFOA_ArityMask; /* clear old arity value */ \
arr[vnfNum] |= ((arity << VNFOA_ArityShift) & VNFOA_ArityMask) /* set the new arity value */

#ifdef FEATURE_HW_INTRINSICS

for (NamedIntrinsic id = (NamedIntrinsic)(NI_HW_INTRINSIC_START + 1); (id < NI_HW_INTRINSIC_END);
id = (NamedIntrinsic)(id + 1))
{
if (HWIntrinsicInfo::IsCommutative(id))
{
VNFunc func = VNFunc(VNF_HWI_FIRST + (id - NI_HW_INTRINSIC_START - 1));
arr[func] |= VNFOA_Commutative;
}
}

#endif // FEATURE_HW_INTRINSICS

#undef ValueNumFuncSetArity

for (unsigned i = 0; i < ArrLen(genTreeOpsIllegalAsVNFunc); i++)
Expand All @@ -10713,7 +10726,7 @@ void ValueNumStore::ValidateValueNumStoreStatics()

#ifdef DEBUG
// Define the name array.
#define ValueNumFuncDef(vnf, arity, commute, knownNonNull) #vnf,
#define ValueNumFuncDef(vnf, arity, knownNonNull) #vnf,

const char* ValueNumStore::VNFuncNameArr[] = {
#include "valuenumfuncs.h"
Expand Down
26 changes: 8 additions & 18 deletions src/coreclr/jit/valuenum.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ enum VNFunc
#define GTNODE(en, st, cm, ivn, ok) VNF_##en,
#include "gtlist.h"
VNF_Boundary = GT_COUNT,
#define ValueNumFuncDef(nm, arity, commute, knownNonNull) VNF_##nm,
#define ValueNumFuncDef(nm, arity, knownNonNull) VNF_##nm,
#include "valuenumfuncs.h"
VNF_COUNT
};
Expand Down Expand Up @@ -295,23 +295,20 @@ class ValueNumStore
enum VNFOpAttrib
{
VNFOA_IllegalGenTreeOp = 0x1, // corresponds to a genTreeOps value that is not a legal VN func.
VNFOA_Commutative = 0x2, // 1 iff the function is commutative.
VNFOA_Arity1 = 0x4, // Bits 2,3,4 encode the arity.
VNFOA_Arity2 = 0x8, // Bits 2,3,4 encode the arity.
VNFOA_Arity4 = 0x10, // Bits 2,3,4 encode the arity.
VNFOA_KnownNonNull = 0x20, // 1 iff the result is known to be non-null.
VNFOA_Arity1 = 0x2, // Bits 1,2,3 encode the arity.
VNFOA_Arity2 = 0x4, // Bits 1,2,3 encode the arity.
VNFOA_Arity4 = 0x8, // Bits 1,2,3 encode the arity.
VNFOA_KnownNonNull = 0x10, // 1 iff the result is known to be non-null.
};

static const unsigned VNFOA_IllegalGenTreeOpShift = 0;
static const unsigned VNFOA_CommutativeShift = 1;
static const unsigned VNFOA_ArityShift = 2;
static const unsigned VNFOA_ArityShift = 1;
static const unsigned VNFOA_ArityBits = 3;
static const unsigned VNFOA_MaxArity = (1 << VNFOA_ArityBits) - 1; // Max arity we can represent.
static const unsigned VNFOA_ArityMask = (VNFOA_Arity4 | VNFOA_Arity2 | VNFOA_Arity1);
static const unsigned VNFOA_KnownNonNullShift = 5;
static const unsigned VNFOA_KnownNonNullShift = 4;

static_assert(unsigned(VNFOA_IllegalGenTreeOp) == (1 << VNFOA_IllegalGenTreeOpShift));
static_assert(unsigned(VNFOA_Commutative) == (1 << VNFOA_CommutativeShift));
static_assert(unsigned(VNFOA_Arity1) == (1 << VNFOA_ArityShift));
static_assert(VNFOA_ArityMask == (VNFOA_MaxArity << VNFOA_ArityShift));
static_assert(unsigned(VNFOA_KnownNonNull) == (1 << VNFOA_KnownNonNullShift));
Expand All @@ -328,10 +325,9 @@ class ValueNumStore
// Helpers and an array of length GT_COUNT, mapping genTreeOp values to their VNFOpAttrib.
static constexpr uint8_t GetOpAttribsForArity(genTreeOps oper, GenTreeOperKind kind);
static constexpr uint8_t GetOpAttribsForGenTree(genTreeOps oper,
bool commute,
bool illegalAsVNFunc,
GenTreeOperKind kind);
static constexpr uint8_t GetOpAttribsForFunc(int arity, bool commute, bool knownNonNull);
static constexpr uint8_t GetOpAttribsForFunc(int arity, bool knownNonNull);
static const uint8_t s_vnfOpAttribs[];

// Returns "true" iff gtOper is a legal value number function.
Expand Down Expand Up @@ -2314,12 +2310,6 @@ inline bool ValueNumStore::GenTreeOpIsLegalVNFunc(genTreeOps gtOper)
return (s_vnfOpAttribs[gtOper] & VNFOA_IllegalGenTreeOp) == 0;
}

// static
inline bool ValueNumStore::VNFuncIsCommutative(VNFunc vnf)
{
return (s_vnfOpAttribs[vnf] & VNFOA_Commutative) != 0;
}

inline bool ValueNumStore::VNFuncIsComparison(VNFunc vnf)
{
if (vnf >= VNF_Boundary)
Expand Down
Loading
Loading