From 250f1617acbb8724c119806a05e1741970c9eb6d Mon Sep 17 00:00:00 2001 From: Annabelle Huo Date: Mon, 23 Oct 2023 17:15:32 -0400 Subject: [PATCH] Fix issue in folding the conditional in the duplicated loop 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 Signed-off-by: Annabelle Huo --- compiler/optimizer/LoopVersioner.cpp | 15 +++++++++-- compiler/optimizer/LoopVersioner.hpp | 37 +++++++++++++++------------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/compiler/optimizer/LoopVersioner.cpp b/compiler/optimizer/LoopVersioner.cpp index 4e472aaeaf..4fd3b8546c 100644 --- a/compiler/optimizer/LoopVersioner.cpp +++ b/compiler/optimizer/LoopVersioner.cpp @@ -4080,7 +4080,14 @@ void TR_LoopVersioner::versionNaturalLoop(TR_RegionStructure *whileLoop, ListgetOption(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)) { @@ -4375,7 +4382,8 @@ void TR_LoopVersioner::versionNaturalLoop(TR_RegionStructure *whileLoop, ListisEmpty() && !disableWrtbarVersion && !shouldOnlySpecializeLoops() - && !refineAliases(); + && !refineAliases() + && !_curLoop->_foldConditionalInDuplicatedLoop; bool alreadyAskedPermission = false; // permission to transform @@ -6403,6 +6411,8 @@ void TR_LoopVersioner::buildConditionalTree( _duplicateConditionalTree, reverseBranch, /* original = */ false)); + + _curLoop->_foldConditionalInDuplicatedLoop = true; } } } @@ -10319,6 +10329,7 @@ TR_LoopVersioner::CurLoop::CurLoop( , _privatizationOK(false) , _hcrGuardVersioningOK(false) , _osrGuardVersioningOK(false) + , _foldConditionalInDuplicatedLoop(false) {} /** diff --git a/compiler/optimizer/LoopVersioner.hpp b/compiler/optimizer/LoopVersioner.hpp index 7b6d413afd..2f135c79c3 100644 --- a/compiler/optimizer/LoopVersioner.hpp +++ b/compiler/optimizer/LoopVersioner.hpp @@ -140,19 +140,19 @@ struct LoopTemps : public TR_Link * 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. * * @@ -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 @@ -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. */