Skip to content
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.

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

Save 260k in InitValueNumStoreStatics #85945

Merged
merged 25 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ae2ecf0
Save 240k in InitValueNumStoreStatics
markples May 8, 2023
cc2a644
constexpr in gentree
markples May 8, 2023
4109e02
Make valuenum s_vnfOpAttribs a const table
markples May 9, 2023
3754327
no std
markples May 9, 2023
d11e5db
experiment
markples May 10, 2023
34d1e09
Expand tables, draft of vn table, restore old code for assertions
markples May 11, 2023
3e9ad0c
fix assertion to not incremental from the new data
markples May 11, 2023
8fb5a28
fix tables
markples May 11, 2023
c8b8b0c
assertions for tables + fixes
markples May 11, 2023
ce579fe
cleanup, mainly undoing unnecessary changes from main
markples May 11, 2023
5cf6b8b
more cleanup
markples May 11, 2023
fa9ca35
Merge remote-tracking branch 'dotnet/main' into jitsize
markples May 12, 2023
42e59df
Update header comments
markples May 12, 2023
0180232
Merge remote-tracking branch 'dotnet/main' into jitsize
markples May 22, 2023
e7c0585
Merge remote-tracking branch 'dotnet/main' into jitsize
markples May 22, 2023
69cec94
finish merge
markples May 22, 2023
0965c20
fix
markples May 23, 2023
38c5e2d
undo whitespace change
markples May 23, 2023
c5cceba
jitformat
markples May 23, 2023
0db814c
attempt to fix constexpr errors from linux build
markples May 23, 2023
baec94c
jitformat
markples May 23, 2023
80130a0
add helper, jitformat
markples May 23, 2023
09b3066
Merge remote-tracking branch 'dotnet/main' into jitsize
markples May 31, 2023
3311223
oops
markples May 31, 2023
8c4179e
Merge remote-tracking branch 'dotnet/main' into jitsize
markples Jun 2, 2023
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
3 changes: 0 additions & 3 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1329,9 +1329,6 @@ void Compiler::compStartup()

emitter::emitInit();

// Static vars of ValueNumStore
ValueNumStore::InitValueNumStoreStatics();

compDisplayStaticSizes(jitstdout);
}

Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,7 @@ struct GenTree

static const unsigned char gtOperKindTable[];

static unsigned OperKind(unsigned gtOper)
static constexpr unsigned OperKind(unsigned gtOper)
{
assert(gtOper < GT_COUNT);

Expand Down Expand Up @@ -1446,7 +1446,7 @@ struct GenTree
}
#endif // TARGET_XARCH

static bool OperIsUnary(genTreeOps gtOper)
static constexpr bool OperIsUnary(genTreeOps gtOper)
{
return (OperKind(gtOper) & GTK_UNOP) != 0;
}
Expand Down Expand Up @@ -1508,7 +1508,7 @@ struct GenTree
}
#endif // FEATURE_HW_INTRINSICS

static bool OperIsCommutative(genTreeOps gtOper)
static constexpr bool OperIsCommutative(genTreeOps gtOper)
{
return (OperKind(gtOper) & GTK_COMMUTE) != 0;
}
Expand Down
128 changes: 74 additions & 54 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8816,70 +8816,90 @@ void ValueNumStore::vnDumpZeroObj(Compiler* comp, VNFuncApp* zeroObj)
#endif // DEBUG

// Static fields, methods.
static UINT8 vnfOpAttribs[VNF_COUNT];
static genTreeOps genTreeOpsIllegalAsVNFunc[] = {GT_IND, // When we do heap memory.
GT_NULLCHECK, GT_QMARK, GT_COLON, GT_LOCKADD, GT_XADD, GT_XCHG,
GT_CMPXCHG, GT_LCLHEAP, GT_BOX, GT_XORR, GT_XAND, GT_STORE_DYN_BLK,
GT_STORE_LCL_VAR, GT_STORE_LCL_FLD, GT_STOREIND, GT_STORE_BLK,
// These need special semantics:
GT_COMMA, // == second argument (but with exception(s) from first).
GT_ARR_ADDR, GT_BOUNDS_CHECK,
GT_BLK, // May reference heap memory.
GT_INIT_VAL, // Not strictly a pass-through.
GT_MDARR_LENGTH,
GT_MDARR_LOWER_BOUND, // 'dim' value must be considered
GT_BITCAST, // Needs to encode the target type.

// These control-flow operations need no values.
GT_JTRUE, GT_RETURN, GT_SWITCH, GT_RETFILT, GT_CKFINITE};

UINT8* ValueNumStore::s_vnfOpAttribs = nullptr;

void ValueNumStore::InitValueNumStoreStatics()
static const genTreeOps genTreeOpsIllegalAsVNFunc[] = {GT_IND, // When we do heap memory.
GT_NULLCHECK, GT_QMARK, GT_COLON, GT_LOCKADD, GT_XADD, GT_XCHG,
GT_CMPXCHG, GT_LCLHEAP, GT_BOX, GT_XORR, GT_XAND, GT_STORE_DYN_BLK,
GT_STORE_LCL_VAR, GT_STORE_LCL_FLD, GT_STOREIND, GT_STORE_BLK,
// These need special semantics:
GT_COMMA, // == second argument (but with exception(s) from first).
GT_ARR_ADDR, GT_BOUNDS_CHECK,
GT_BLK, // May reference heap memory.
GT_INIT_VAL, // Not strictly a pass-through.
GT_MDARR_LENGTH,
GT_MDARR_LOWER_BOUND, // 'dim' value must be considered
GT_BITCAST, // Needs to encode the target type.

// These control-flow operations need no values.
GT_JTRUE, GT_RETURN, GT_SWITCH, GT_RETFILT, GT_CKFINITE};

/* static */ const ValueNumStore::VnfOpAttribsType ValueNumStore::s_vnfOpAttribs = ValueNumStore::VnfOpAttribsType();

/* static */ constexpr unsigned ValueNumStore::VnfOpAttribsType::GetArity(unsigned oper)
{
genTreeOps gtOper = static_cast<genTreeOps>(oper);
unsigned arity = 0;
if (GenTree::OperIsUnary(gtOper))
{
arity = 1;
}
else if (GenTree::OperIsBinary(gtOper))
{
arity = 2;
}
else if (GenTree::StaticOperIs(gtOper, GT_SELECT))
{
arity = 3;
}

return (arity << VNFOA_ArityShift) & VNFOA_ArityMask;
}

/* static */ constexpr unsigned ValueNumStore::VnfOpAttribsType::GetCommutative(unsigned oper)
{
genTreeOps gtOper = static_cast<genTreeOps>(oper);
return GenTree::OperIsCommutative(gtOper) ? VNFOA_Commutative : 0;
}

/* static */ constexpr unsigned ValueNumStore::VnfOpAttribsType::GetFunc(int arity, bool commute, bool knownNonNull, bool sharedStatic)
{
unsigned value = 0;
if (commute)
{
value |= VNFOA_Commutative;
}
if (knownNonNull)
{
value |= VNFOA_KnownNonNull;
}
if (sharedStatic)
{
value |= VNFOA_SharedStatic;
}
if (arity > 0)
{
value |= ((arity << VNFOA_ArityShift) & VNFOA_ArityMask);
}
return value;
}

constexpr ValueNumStore::VnfOpAttribsType::VnfOpAttribsType() : m_arr()
{
// Make sure we have the constants right...
assert(unsigned(VNFOA_Arity1) == (1 << VNFOA_ArityShift));
assert(VNFOA_ArityMask == (VNFOA_MaxArity << VNFOA_ArityShift));

s_vnfOpAttribs = &vnfOpAttribs[0];
for (unsigned i = 0; i < GT_COUNT; i++)
{
genTreeOps gtOper = static_cast<genTreeOps>(i);
unsigned arity = 0;
if (GenTree::OperIsUnary(gtOper))
{
arity = 1;
}
else if (GenTree::OperIsBinary(gtOper))
{
arity = 2;
}
else if (GenTree::StaticOperIs(gtOper, GT_SELECT))
{
arity = 3;
}

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

if (GenTree::OperIsCommutative(gtOper))
{
vnfOpAttribs[i] |= VNFOA_Commutative;
}
m_arr[i] |= GetArity(i);
m_arr[i] |= GetCommutative(i);
}

// 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, sharedStatic) \
if (commute) \
vnfOpAttribs[vnfNum] |= VNFOA_Commutative; \
if (knownNonNull) \
vnfOpAttribs[vnfNum] |= VNFOA_KnownNonNull; \
if (sharedStatic) \
vnfOpAttribs[vnfNum] |= VNFOA_SharedStatic; \
if (arity > 0) \
vnfOpAttribs[vnfNum] |= ((arity << VNFOA_ArityShift) & VNFOA_ArityMask); \
m_arr[vnfNum] |= GetFunc(arity, commute, knownNonNull, sharedStatic); \
vnfNum++;

#include "valuenumfuncs.h"
Expand All @@ -8888,8 +8908,8 @@ void ValueNumStore::InitValueNumStoreStatics()
assert(vnfNum == VNF_COUNT);

#define ValueNumFuncSetArity(vnfNum, arity) \
vnfOpAttribs[vnfNum] &= ~VNFOA_ArityMask; /* clear old arity value */ \
vnfOpAttribs[vnfNum] |= ((arity << VNFOA_ArityShift) & VNFOA_ArityMask) /* set the new arity value */
m_arr[vnfNum] &= ~VNFOA_ArityMask; /* clear old arity value */ \
m_arr[vnfNum] |= ((arity << VNFOA_ArityShift) & VNFOA_ArityMask) /* set the new arity value */

#ifdef FEATURE_HW_INTRINSICS

Expand All @@ -8912,7 +8932,7 @@ void ValueNumStore::InitValueNumStoreStatics()
if (HWIntrinsicInfo::IsCommutative(id))
{
VNFunc func = VNFunc(VNF_HWI_FIRST + (id - NI_HW_INTRINSIC_START - 1));
vnfOpAttribs[func] |= VNFOA_Commutative;
m_arr[func] |= VNFOA_Commutative;
}
}

Expand All @@ -8922,7 +8942,7 @@ void ValueNumStore::InitValueNumStoreStatics()

for (unsigned i = 0; i < ArrLen(genTreeOpsIllegalAsVNFunc); i++)
{
vnfOpAttribs[genTreeOpsIllegalAsVNFunc[i]] |= VNFOA_IllegalGenTreeOp;
m_arr[genTreeOpsIllegalAsVNFunc[i]] |= VNFOA_IllegalGenTreeOp;
}
}

Expand Down
21 changes: 17 additions & 4 deletions src/coreclr/jit/valuenum.h
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ class ValueNumStore
};

// An array of length GT_COUNT, mapping genTreeOp values to their VNFOpAttrib.
static UINT8* s_vnfOpAttribs;
//static const UINT8* s_vnfOpAttribs;

// Returns "true" iff gtOper is a legal value number function.
// (Requires InitValueNumStoreStatics to have been run.)
Expand Down Expand Up @@ -359,6 +359,22 @@ class ValueNumStore
#endif // FEATURE_SIMD

private:
// This struct mainly exists to give a place to put the constexpr code for creating
// the OpAttribs table. It also gives a scope for the helpers functions used during
// initialization.
struct VnfOpAttribsType
{
public:
constexpr VnfOpAttribsType();
const UINT8& operator[](size_t idx) const { return m_arr[idx]; }
private:
static constexpr unsigned GetArity(unsigned oper);
static constexpr unsigned GetCommutative(unsigned oper);
static constexpr unsigned GetFunc(int arity, bool commute, bool knownNonNull, bool sharedStatic);

UINT8 m_arr[VNF_COUNT];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const UINT8& operator[](size_t idx) const { return m_arr[idx]; }
private:
static constexpr unsigned GetArity(unsigned oper);
static constexpr unsigned GetCommutative(unsigned oper);
static constexpr unsigned GetFunc(int arity, bool commute, bool knownNonNull, bool sharedStatic);
UINT8 m_arr[VNF_COUNT];
const uint8_t& operator[](size_t idx) const { return m_arr[idx]; }
private:
static constexpr unsigned GetArity(unsigned oper);
static constexpr unsigned GetCommutative(unsigned oper);
static constexpr unsigned GetFunc(int arity, bool commute, bool knownNonNull, bool sharedStatic);
uint8_t m_arr[VNF_COUNT];

Nit: We want to stick to std C++ type names where possible.

} static const s_vnfOpAttribs;

// Assumes that all the ValueNum arguments of each of these functions have been shown to represent constants.
// Assumes that "vnf" is a operator of the appropriate arity (unary for the first, binary for the second).
// Assume that "CanEvalForConstantArgs(vnf)" is true.
Expand Down Expand Up @@ -387,9 +403,6 @@ class ValueNumStore
GenTreeFlags GetFoldedArithOpResultHandleFlags(ValueNum vn);

public:
// Initializes any static variables of ValueNumStore.
static void InitValueNumStoreStatics();

// Initialize an empty ValueNumStore.
ValueNumStore(Compiler* comp, CompAllocator allocator);

Expand Down