Skip to content

Make JitOptRepeat available in Release builds #100494

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

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Make JitOptRepeat available in Release builds #100494

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
22 changes: 9 additions & 13 deletions src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2902,10 +2902,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
opts.disAlignment = false;
opts.disCodeBytes = false;

#ifdef OPT_CONFIG
opts.optRepeat = false;
#endif // OPT_CONFIG

opts.optRepeat = false;
opts.optRepeatIteration = 0;
opts.optRepeatCount = 1;
opts.optRepeatActive = false;
Expand Down Expand Up @@ -3084,6 +3081,13 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
{
opts.disAsm = true;
}

if ((JitConfig.JitEnableOptRepeat() != 0) &&
(JitConfig.JitOptRepeat().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args)))
{
opts.optRepeat = true;
opts.optRepeatCount = JitConfig.JitOptRepeatCount();
}
#endif // !DEBUG

#ifndef DEBUG
Expand All @@ -3109,8 +3113,6 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
}
}

#ifdef OPT_CONFIG

if (opts.optRepeat)
{
// Defer printing this until now, after the "START" line printed above.
Expand All @@ -3132,7 +3134,6 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
JITDUMP("\n*************** JitOptRepeat enabled by JitOptRepeatRange; repetition count: %d\n\n",
opts.optRepeatCount);
}
#endif // DEBUG

if (!opts.optRepeat && compStressCompile(STRESS_OPT_REPEAT, 10))
{
Expand All @@ -3146,10 +3147,9 @@ void Compiler::compInitOptions(JitFlags* jitFlags)

JITDUMP("\n*************** JitOptRepeat for stress; repetition count: %d\n\n", opts.optRepeatCount);
}
#endif // DEBUG
}

#endif // OPT_CONFIG

#ifdef DEBUG
assert(!codeGen->isGCTypeFixed());
opts.compGcChecks = (JitConfig.JitGCChecks() != 0) || compStressCompile(STRESS_GENERIC_VARN, 5);
Expand Down Expand Up @@ -5012,12 +5012,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
doVNBasedDeadStoreRemoval = doValueNum && (JitConfig.JitDoVNBasedDeadStoreRemoval() != 0);
#endif // defined(OPT_CONFIG)

#ifdef DEBUG
if (opts.optRepeat)
{
opts.optRepeatActive = true;
}
#endif // DEBUG

while (++opts.optRepeatIteration <= opts.optRepeatCount)
{
Expand Down Expand Up @@ -5166,12 +5164,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
#endif // DEBUG
}

#ifdef DEBUG
if (opts.optRepeat)
{
opts.optRepeatActive = false;
}
#endif // DEBUG
}

optLoopsCanonical = false;
Expand Down
15 changes: 6 additions & 9 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9874,15 +9874,12 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
bool genFPopt; // Can we do frame-pointer-omission optimization?
bool altJit; // True if we are an altjit and are compiling this method

#ifdef OPT_CONFIG
bool optRepeat; // Repeat optimizer phases k times
#endif

int optRepeatIteration; // The current optRepeat iteration: from 0 to optRepeatCount. optRepeatCount can be
// zero, in which case no optimizations in the set of repeated optimizations are
// performed. optRepeatIteration will only be zero if optRepeatCount is zero.
int optRepeatCount; // How many times to repeat. By default, comes from JitConfig.JitOptRepeatCount().
bool optRepeatActive; // `true` if we are in the range of phases being repeated.
bool optRepeat; // Repeat optimizer phases k times
int optRepeatIteration; // The current optRepeat iteration: from 0 to optRepeatCount. optRepeatCount can be
// zero, in which case no optimizations in the set of repeated optimizations are
// performed. optRepeatIteration will only be zero if optRepeatCount is zero.
int optRepeatCount; // How many times to repeat. By default, comes from JitConfig.JitOptRepeatCount().
bool optRepeatActive; // `true` if we are in the range of phases being repeated.

bool disAsm; // Display native code as it is generated
bool disTesting; // Display BEGIN METHOD/END METHOD anchors for disasm testing
Expand Down
9 changes: 5 additions & 4 deletions src/coreclr/jit/jitconfigvalues.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,15 +521,16 @@ CONFIG_STRING(JitEnableInductionVariableOptsRange, W("JitEnableInductionVariable
CONFIG_INTEGER(JitDoSsa, W("JitDoSsa"), 1) // Perform Static Single Assignment (SSA) numbering on the variables
CONFIG_INTEGER(JitDoValueNumber, W("JitDoValueNumber"), 1) // Perform value numbering on method expressions

CONFIG_INTEGER(JitEnableOptRepeat, W("JitEnableOptRepeat"), 1) // If zero, do not allow JitOptRepeat
CONFIG_METHODSET(JitOptRepeat, W("JitOptRepeat")) // Runs optimizer multiple times on specified methods
CONFIG_INTEGER(JitOptRepeatCount, W("JitOptRepeatCount"), 2) // Number of times to repeat opts when repeating
CONFIG_STRING(JitOptRepeatRange, W("JitOptRepeatRange")) // Enable JitOptRepeat based on method hash range
CONFIG_STRING(JitOptRepeatRange, W("JitOptRepeatRange")) // Enable JitOptRepeat based on method hash range

CONFIG_INTEGER(JitDoIfConversion, W("JitDoIfConversion"), 1) // Perform If conversion

#endif // defined(OPT_CONFIG)

CONFIG_INTEGER(JitEnableOptRepeat, W("JitEnableOptRepeat"), 1) // If zero, do not allow JitOptRepeat
CONFIG_METHODSET(JitOptRepeat, W("JitOptRepeat")) // Runs optimizer multiple times on specified methods
Copy link
Member

Choose a reason for hiding this comment

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

I wonder - maybe it's worth making its default value * so it will be enough to only define DOTNET_JitEnableOptRepeat ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

METHODSET doesn't have a default value.

I thought about getting rid of DOTNET_JitEnableOptRepeat but after #94250 is merged, which enables OptRepeat in stress, I want a way to disable OptRepeat in stress.

Maybe we could instead have (after #94250 is merged):

  1. Default: OptRepeat enabled in stress.
  2. DOTNET_JitOptRepeat=method set: enable OptRepeat for exactly the specified methods
  3. DOTNET_JitEnableOptRepeat=0: disable OptRepeat in stress. Or maybe name it DOTNET_JitEnableOptRepeatStress to be clear that's all it affects?
    • But, perhaps weirdly, if you set DOTNET_JitOptRepeat=* and DOTNET_JitEnableOptRepeat=0, we still enable OptRepeat? (Maybe leaning towards renaming to DOTNET_JitEnableOptRepeatStress?)

CONFIG_INTEGER(JitOptRepeatCount, W("JitOptRepeatCount"), 2) // Number of times to repeat opts when repeating

// Max # of MapSelect's considered for a particular top-level invocation.
CONFIG_INTEGER(JitVNMapSelBudget, W("JitVNMapSelBudget"), DEFAULT_MAP_SELECT_BUDGET)

Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/jit/optimizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,14 +503,12 @@ bool Compiler::optExtractInitTestIncr(
if (initStmt->GetRootNode()->OperIs(GT_JTRUE))
{
bool doGetPrev = true;
#ifdef OPT_CONFIG
if (opts.optRepeat)
{
// Previous optimization passes may have inserted compiler-generated
// statements other than duplicated loop conditions.
doGetPrev = (initStmt->GetPrevStmt() != nullptr);
}
#endif // OPT_CONFIG
if (doGetPrev)
{
initStmt = initStmt->GetPrevStmt();
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/scripts/superpmi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2345,8 +2345,9 @@ def create_exception():
diff_perfscore = diff_metrics["Overall"]["Diffed PerfScore"]
logging.info("Total PerfScore of base: {}".format(base_perfscore))
logging.info("Total PerfScore of diff: {}".format(diff_perfscore))
delta_perfscore = diff_perfscore - base_perfscore
logging.info("Total PerfScore of delta: {} ({:.2%} of base)".format(delta_perfscore, delta_perfscore / base_perfscore))
if base_perfscore != 0:
delta_perfscore = diff_perfscore - base_perfscore
logging.info("Total PerfScore of delta: {} ({:.2%} of base)".format(delta_perfscore, delta_perfscore / base_perfscore))
logging.info("")

relative_perfscore_geomean = diff_metrics["Overall"]["Relative PerfScore Geomean"]
Expand Down