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

Do not hoist constants if they will not be CSEed #64039

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -7395,6 +7395,8 @@ class Compiler

bool optIsCSEcandidate(GenTree* tree);

bool optIsIntegralConstCSEcandidate(GenTreeIntConCommon* tree);

// lclNumIsTrueCSE returns true if the LclVar was introduced by the CSE phase of the compiler
//
bool lclNumIsTrueCSE(unsigned lclNum) const
Expand Down
95 changes: 49 additions & 46 deletions src/coreclr/jit/optcse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -734,29 +734,6 @@ unsigned Compiler::optValnumCSE_Index(GenTree* tree, Statement* stmt)
//
bool Compiler::optValnumCSE_Locate()
{
bool enableConstCSE = true;

int configValue = JitConfig.JitConstCSE();

// all platforms - disable CSE of constant values when config is 1
if (configValue == CONST_CSE_DISABLE_ALL)
{
enableConstCSE = false;
}

#if !defined(TARGET_ARM64)
// non-ARM64 platforms - disable by default
//
enableConstCSE = false;

// Check for the two enable cases for all platforms
//
if ((configValue == CONST_CSE_ENABLE_ALL) || (configValue == CONST_CSE_ENABLE_ALL_NO_SHARING))
{
enableConstCSE = true;
}
#endif

for (BasicBlock* const block : Blocks())
{
/* Make the block publicly available */
Expand Down Expand Up @@ -784,16 +761,6 @@ bool Compiler::optValnumCSE_Locate()
optCseUpdateCheckedBoundMap(tree);
}

// Don't allow CSE of constants if it is disabled
//
if (tree->IsIntegralConst())
{
if (!enableConstCSE)
{
continue;
}
}

// Don't allow non-SIMD struct CSEs under a return; we don't fully
// re-morph these if we introduce a CSE assignment, and so may create
// IR that lower is not yet prepared to handle.
Expand Down Expand Up @@ -3500,16 +3467,7 @@ bool Compiler::optIsCSEcandidate(GenTree* tree)
return false;
}

#if !CSE_CONSTS
/* Don't bother with constants */
if (tree->OperIsConst())
{
return false;
}
#endif

/* Check for some special cases */

switch (oper)
{
case GT_CALL:
Expand Down Expand Up @@ -3559,13 +3517,12 @@ bool Compiler::optIsCSEcandidate(GenTree* tree)
return (tree->AsOp()->gtOp1->gtOper != GT_ARR_ELEM);

case GT_CNS_LNG:
#ifndef TARGET_64BIT
return false; // Don't CSE 64-bit constants on 32-bit platforms
#endif
case GT_CNS_INT:
return optIsIntegralConstCSEcandidate(tree->AsIntConCommon());

case GT_CNS_DBL:
case GT_CNS_STR:
return true; // We reach here only when CSE_CONSTS is enabled
return true;

case GT_ARR_ELEM:
case GT_ARR_LENGTH:
Expand Down Expand Up @@ -3692,6 +3649,52 @@ bool Compiler::optIsCSEcandidate(GenTree* tree)
return false;
}

//------------------------------------------------------------------------
// optIsIntegralConstCSEcandidate: Is the constant "tree" a CSE candidate?
//
// Encapsulates the logic related to configuration switches affecting
// CSE of integral constants.
//
// Arguments:
// tree - The integral constant to check
//
// Return Value:
// Whether "tree" should be considered for CSE.
//
bool Compiler::optIsIntegralConstCSEcandidate(GenTreeIntConCommon* tree)
{
#ifndef TARGET_64BIT
// We won't CSE LONG constants on 32 bit platforms.
if (tree->OperIs(GT_CNS_LNG))
{
return false;
}
#endif // TARGET_64BIT

// Force disable/enable if we've been requested to.
switch (JitConfig.JitConstCSE())
{
case CONST_CSE_DISABLE_ALL:
return false;

case CONST_CSE_ENABLE_ALL:
case CONST_CSE_ENABLE_ALL_NO_SHARING:
return true;

default:
break;
}

// Otherwise, follow the defaults. Currently, these are to only CSE on ARM64.
CLANG_FORMAT_COMMENT_ANCHOR;

#if CSE_INTEGRAL_CONSTS
return true;
#else
return false;
#endif
}

#ifdef DEBUG
//
// A Debug only method that allows you to control whether the CSE logic is enabled for this method.
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/targetamd64.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
#else // !UNIX_AMD64_ABI
#define ETW_EBP_FRAMED 0 // if 1 we cannot use EBP as a scratch register and must create EBP based frames for most methods
#endif // !UNIX_AMD64_ABI
#define CSE_CONSTS 1 // Enable if we want to CSE constants
#define CSE_INTEGRAL_CONSTS 0 // Do we *want* to CSE integral constants?

#define RBM_ALLFLOAT (RBM_XMM0 | RBM_XMM1 | RBM_XMM2 | RBM_XMM3 | RBM_XMM4 | RBM_XMM5 | RBM_XMM6 | RBM_XMM7 | RBM_XMM8 | RBM_XMM9 | RBM_XMM10 | RBM_XMM11 | RBM_XMM12 | RBM_XMM13 | RBM_XMM14 | RBM_XMM15)
#define RBM_ALLDOUBLE RBM_ALLFLOAT
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/targetarm.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
#define FEATURE_EH 1 // To aid platform bring-up, eliminate exceptional EH clauses (catch, filter, filter-handler, fault) and directly execute 'finally' clauses.
#define FEATURE_EH_CALLFINALLY_THUNKS 0 // Generate call-to-finally code in "thunks" in the enclosing EH region, protected by "cloned finally" clauses.
#define ETW_EBP_FRAMED 1 // if 1 we cannot use REG_FP as a scratch register and must setup the frame pointer for most methods
#define CSE_CONSTS 1 // Enable if we want to CSE constants
#define CSE_INTEGRAL_CONSTS 0 // Do we *want* to CSE integral constants?

#define REG_FP_FIRST REG_F0
#define REG_FP_LAST REG_F31
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/targetarm64.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
#define FEATURE_EH 1 // To aid platform bring-up, eliminate exceptional EH clauses (catch, filter, filter-handler, fault) and directly execute 'finally' clauses.
#define FEATURE_EH_CALLFINALLY_THUNKS 1 // Generate call-to-finally code in "thunks" in the enclosing EH region, protected by "cloned finally" clauses.
#define ETW_EBP_FRAMED 1 // if 1 we cannot use REG_FP as a scratch register and must setup the frame pointer for most methods
#define CSE_CONSTS 1 // Enable if we want to CSE constants
#define CSE_INTEGRAL_CONSTS 1 // Do we *want* to CSE integral constants?

#define REG_FP_FIRST REG_V0
#define REG_FP_LAST REG_V31
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/targetx86.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
// protected by "cloned finally" clauses.
#define ETW_EBP_FRAMED 1 // if 1 we cannot use EBP as a scratch register and must create EBP based
// frames for most methods
#define CSE_CONSTS 1 // Enable if we want to CSE constants
#define CSE_INTEGRAL_CONSTS 0 // Do we *want* to CSE integral constants?

// The following defines are useful for iterating a regNumber
#define REG_FIRST REG_EAX
Expand Down