Skip to content

Commit

Permalink
Fix issue in folding the conditional in the duplicated loop
Browse files Browse the repository at this point in the history
It is incorrect to both version asynch check and fold
the conditional in the duplicated loop, or version write
barrier and fold the conditional in the duplicated loop.

For asynch check, when the loops are unbiased, do not
version asynch check so that the conditional in both
loops can be folded away.

For write barriers, the determination of whether or not to
version write barriers happens after the duplicated loop has
already been added to `FoldConditional`. If the conditional
in the duplicated loop will be folded, do not version write
barrier.

Fixes eclipse-openj9/openj9#17249

Co-Authored-By: Devin Papineau <devin@ajdmp.ca>

Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
  • Loading branch information
a7ehuo committed Oct 25, 2023
1 parent 9bec530 commit 36ca08b
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 19 deletions.
15 changes: 13 additions & 2 deletions compiler/optimizer/LoopVersioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4080,7 +4080,14 @@ void TR_LoopVersioner::versionNaturalLoop(TR_RegionStructure *whileLoop, List<TR
_canPredictIters = false;
}

if (!comp()->getOption(TR_DisableAsyncCheckVersioning) &&
// `_duplicateConditionalTree` is set only when `_conditionalTree` is set. `_conditionalTree`
// is set only when`_neitherLoopCold` is true. Therefore, if `_duplicateConditionalTree` exists,
// the loops are unbiased. When the loops are unbiased, do not version asynch check
// so that the conditional in both loops can be folded away.
bool asyncCheckVersioningOK = _duplicateConditionalTree ? false : true;

if (asyncCheckVersioningOK &&
!comp()->getOption(TR_DisableAsyncCheckVersioning) &&
!refineAliases() && _canPredictIters && comp()->getProfilingMode() != JitProfiling &&
performTransformation(comp(), "%s Creating test outside loop for deciding if async check is required\n", OPT_DETAILS_LOOP_VERSIONER))
{
Expand Down Expand Up @@ -4375,7 +4382,8 @@ void TR_LoopVersioner::versionNaturalLoop(TR_RegionStructure *whileLoop, List<TR
!awrtbariTrees->isEmpty()
&& !disableWrtbarVersion
&& !shouldOnlySpecializeLoops()
&& !refineAliases();
&& !refineAliases()
&& !_curLoop->_foldConditionalInDuplicatedLoop;

bool alreadyAskedPermission = false; // permission to transform

Expand Down Expand Up @@ -6403,6 +6411,8 @@ void TR_LoopVersioner::buildConditionalTree(
_duplicateConditionalTree,
reverseBranch,
/* original = */ false));

_curLoop->_foldConditionalInDuplicatedLoop = true;
}
}
}
Expand Down Expand Up @@ -10319,6 +10329,7 @@ TR_LoopVersioner::CurLoop::CurLoop(
, _privatizationOK(false)
, _hcrGuardVersioningOK(false)
, _osrGuardVersioningOK(false)
, _foldConditionalInDuplicatedLoop(false)
{}

/**
Expand Down
37 changes: 20 additions & 17 deletions compiler/optimizer/LoopVersioner.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,19 +140,19 @@ struct LoopTemps : public TR_Link<LoopTemps>
* Class TR_LoopVersioner
* ======================
*
* The loop versioner optimization eliminates loop invariant null checks,
* bound checks, div checks, checkcasts and inline guards (where
* the "this" pointer does not change in the loop) from loops by
* placing equivalent tests outside the loop. Also eliminates bound
* checks where the range of values of the index inside the loop is
* discovered (must be compile-time constant) and checks are inserted
* outside the loop that ensure that no bound checks fail inside the loop.
* This analysis uses induction variable information found by value
* propagation.
*
* Another versioning test that is emitted ensures the loop will not
* run for a very long period of time (in which case the async check
* inside the loop can be removed). This special versioning test depends
* The loop versioner optimization eliminates loop invariant null checks,
* bound checks, div checks, checkcasts and inline guards (where
* the "this" pointer does not change in the loop) from loops by
* placing equivalent tests outside the loop. Also eliminates bound
* checks where the range of values of the index inside the loop is
* discovered (must be compile-time constant) and checks are inserted
* outside the loop that ensure that no bound checks fail inside the loop.
* This analysis uses induction variable information found by value
* propagation.
*
* Another versioning test that is emitted ensures the loop will not
* run for a very long period of time (in which case the async check
* inside the loop can be removed). This special versioning test depends
* on the number of trees (code) inside the loop.
*
*
Expand Down Expand Up @@ -744,6 +744,9 @@ class TR_LoopVersioner : public TR_LoopTransformer

/// The result of the analysis to say whether OSR guards can be versioned.
bool _osrGuardVersioningOK;

// Whether or not conditional is folded in the duplicated loop.
bool _foldConditionalInDuplicatedLoop;
};

class Hoist : public LoopImprovement
Expand Down Expand Up @@ -1144,10 +1147,10 @@ class TR_LoopVersioner : public TR_LoopTransformer
* Class TR_LoopSpecializer
* ========================
*
* The loop specializer optimization replaces loop-invariant expressions
* that are profiled and found to be constants, by the constant value
* after inserting a test outside the loop that compares the value to
* the constant. Note that this cannot be done in the the absence of
* The loop specializer optimization replaces loop-invariant expressions
* that are profiled and found to be constants, by the constant value
* after inserting a test outside the loop that compares the value to
* the constant. Note that this cannot be done in the the absence of
* value profiling infrastructure.
*/

Expand Down

0 comments on commit 36ca08b

Please sign in to comment.