From 775d7cf5233519ae3d1009b9a27a732596cfd1d0 Mon Sep 17 00:00:00 2001 From: Dibyendu Majumdar Date: Sat, 18 May 2019 14:51:04 +0100 Subject: [PATCH] Refactor the optimizer to allocate UseDefInfo through a central location This change implements the recommended refactoring suggested by @andrewcraik in the review comments for https://github.com/eclipse/omr/pull/2823. The background is that we need to allow setting of the _hasCallAsUses flag depending on the language front-end. However this is currently hard coded to false. Following is a copy of Andrew's recommendation. Setting _hasCallAsUses should be handled as part of the constructor if it needs to be configured by different languages. This is the first time we are going to have a language specific override of a flag like this for UseDefInfo. Looking at how it is currently used instances of TR_UseDef_info are allocated in the OMROptimizer.cpp, ValueNumberInfo.cpp and LoadExtensions.cpp. There is some variation between them in how they configure UseDefInfo for globals. I think the best way to handle this is to create a new API on the Optimizer object (eg OMROptimizer.hpp and OMROptimizer.cpp) which all current allocation sites switch to calling - the method will accept as parameters the bools which vary between the different request sites within the compiler. The value for _hasCallAsUses could be either stored as a flag in the optimizer or by overriding the build method to change how UseDefInfo is built. I would prefer the later since it means languages have a common place to configure UseDefInfo according to their needs. This small refactor would ideally be done as a separate commit and pull request ahead of changes introducing the new flag to the constructor. I hope that makes sense - if it doesn't do keep asking questions and I'll try to clarify more on how this would work. An added bonus of this is that we could improve the UseDefInfo memory management in a follow-on item which would be a another good reason to Refactor the optimizer to allocate UseDefInfo through a central location. End quote. Note that this change does not address the construction of TR_OSRDefInfo, which extends the TR_UseDefInfo class. To allow language front-ends to control this flag when use defs are analysed we need a callsAsUses flag to be held by the Optimizer and we need methods to get and set the flag. This flag will be used when constructing TR_UseDefInfo objects. The callsAsUses parameter is controlled by a virtual method in the Optimizer that front-end language implementations may override to change the default behaviour. Also the TR_UseDefInfo constructor has been made protected to avoid users making a mistake trying to construct it directly. Signed-off-by: Dibyendu Majumdar --- compiler/optimizer/LoadExtensions.cpp | 2 +- compiler/optimizer/OMROptimizer.cpp | 11 ++++++++-- compiler/optimizer/OMROptimizer.hpp | 30 ++++++++++++++++++++++++++ compiler/optimizer/UseDefInfo.cpp | 22 ++++++++++++++++++- compiler/optimizer/UseDefInfo.hpp | 24 +++++++++++++++++++-- compiler/optimizer/ValueNumberInfo.cpp | 4 ++-- 6 files changed, 85 insertions(+), 8 deletions(-) diff --git a/compiler/optimizer/LoadExtensions.cpp b/compiler/optimizer/LoadExtensions.cpp index 8ee1ce6f3d9..8c3b7939691 100644 --- a/compiler/optimizer/LoadExtensions.cpp +++ b/compiler/optimizer/LoadExtensions.cpp @@ -79,7 +79,7 @@ int32_t TR_LoadExtensions::perform() optimizer()->setUseDefInfo(NULL); - TR_UseDefInfo* useDefInfo = new (comp()->allocator()) TR_UseDefInfo(comp(), comp()->getFlowGraph(), optimizer(), false, false, false, true, true); + TR_UseDefInfo* useDefInfo = optimizer()->createUseDefInfo(comp(), false, false, false, true, true); if (useDefInfo->infoIsValid()) { diff --git a/compiler/optimizer/OMROptimizer.cpp b/compiler/optimizer/OMROptimizer.cpp index 41f2670895b..f7e6d0bdb9a 100644 --- a/compiler/optimizer/OMROptimizer.cpp +++ b/compiler/optimizer/OMROptimizer.cpp @@ -987,6 +987,13 @@ TR_ValueNumberInfo *OMR::Optimizer::setValueNumberInfo(TR_ValueNumberInfo *v) return (_valueNumberInfo = v); } +TR_UseDefInfo* OMR::Optimizer::createUseDefInfo(TR::Compilation* comp, + bool requiresGlobals, bool prefersGlobals, bool loadsShouldBeDefs, bool cannotOmitTrivialDefs, + bool conversionRegsOnly, bool doCompletion) + { + return new (comp->allocator()) TR_UseDefInfo(comp, comp->getFlowGraph(), self(), requiresGlobals, prefersGlobals, loadsShouldBeDefs, + cannotOmitTrivialDefs, conversionRegsOnly, doCompletion, getCallsAsUses()); + } TR_ValueNumberInfo *OMR::Optimizer::createValueNumberInfo(bool requiresGlobals, bool preferGlobals, bool noUseDefInfo) { @@ -1807,7 +1814,7 @@ int32_t OMR::Optimizer::performOptimization(const OptimizationStrategy *optimiza LexicalTimer t("use defs (for globals definitely)", comp()->phaseTimer()); TR::LexicalMemProfiler mp("use defs (for globals definitely)", comp()->phaseMemProfiler()); - useDefInfo = new (comp()->allocator()) TR_UseDefInfo(comp(), comp()->getFlowGraph(), self(), + useDefInfo = createUseDefInfo(comp(), true, // requiresGlobals false,// prefersGlobals !manager->getDoesNotRequireLoadsAsDefsInUseDefs(), @@ -1853,7 +1860,7 @@ int32_t OMR::Optimizer::performOptimization(const OptimizationStrategy *optimiza #endif LexicalTimer t("use defs (for globals possibly)", comp()->phaseTimer()); TR::LexicalMemProfiler mp("use defs (for globals possibly)", comp()->phaseMemProfiler()); - useDefInfo = new (comp()->allocator()) TR_UseDefInfo(comp(), comp()->getFlowGraph(), self(), + useDefInfo = createUseDefInfo(comp(), false, // requiresGlobals manager->getPrefersGlobalsUseDefInfo() || manager->getPrefersGlobalsValueNumbering(), !manager->getDoesNotRequireLoadsAsDefsInUseDefs(), diff --git a/compiler/optimizer/OMROptimizer.hpp b/compiler/optimizer/OMROptimizer.hpp index 60059ff3137..d382b645b20 100644 --- a/compiler/optimizer/OMROptimizer.hpp +++ b/compiler/optimizer/OMROptimizer.hpp @@ -204,6 +204,23 @@ class Optimizer void setCantBuildGlobalsUseDefInfo(bool v) { _cantBuildGlobalsUseDefInfo = v; } void setCantBuildLocalsUseDefInfo(bool v) { _cantBuildLocalsUseDefInfo = v; } + /** + * Constructs a TR_UseDefInfo instance with appropriate flags enabled. + * + * @param comp The compilation instance + * @param cfg The CFG being analysed + * @param opt The optimizer instance + * @param requiresGlobals + * @param prefersGlobals + * @param loadsShouldBeDefs + * @param cannotOmitTrivialDefs + * @param conversionRegsOnly + * @param doCompletion + */ + TR_UseDefInfo* createUseDefInfo(TR::Compilation* comp, + bool requiresGlobals = true, bool prefersGlobals = true, bool loadsShouldBeDefs = true, bool cannotOmitTrivialDefs = false, + bool conversionRegsOnly = false, bool doCompletion = true); + // Get value number information TR_ValueNumberInfo * getValueNumberInfo() { return _valueNumberInfo; } TR_ValueNumberInfo *createValueNumberInfo(bool requiresGlobals = false, bool preferGlobals = true, bool noUseDefInfo = false ); @@ -217,6 +234,19 @@ class Optimizer bool canRunBlockByBlockOptimizations() { return _canRunBlockByBlockOptimizations; } void setCanRunBlockByBlockOptimizations(bool v) { _canRunBlockByBlockOptimizations = v; } + /** + * Controls inclusion of calls as uses so that the alias analysis can detect + * when local (stack) variable has been aliased by a function call. + * This defaults to false which is fine for Java like languages where + * local (stack) variables cannot be passed by reference to function calls + * and hence cannot be aliased. However for C like languages this flag should be + * over-ridden by the optimizer in the front-end. + * + * This parameter is passed to the TR_UseDefInfo constructor. + * See createUseDefInfo(). + */ + virtual bool getCallsAsUses() { return false; } + bool prepareForNodeRemoval(TR::Node *node , bool deferInvalidatingUseDefInfo = false); void prepareForTreeRemoval(TR::TreeTop *treeTop) { prepareForNodeRemoval(treeTop->getNode()); } diff --git a/compiler/optimizer/UseDefInfo.cpp b/compiler/optimizer/UseDefInfo.cpp index e16fde02fd3..9fcd8dc30b5 100644 --- a/compiler/optimizer/UseDefInfo.cpp +++ b/compiler/optimizer/UseDefInfo.cpp @@ -74,8 +74,27 @@ const char* const TR_UseDefInfo::allocatorName = "UseDefInfo"; +/** + * Constructs TR_UseDefInfo instance. Note that this should not be called directly. + * Instead construction is handled by OMR::Optimizer::createUseDefInfo() method. + * + * @param cfg The compilation instance + * @param requiresGlobals + * @param prefersGlobals + * @param loadsShouldBeDefs + * @param cannotOmitTrivialDefs + * @param conversionRegsOnly + * @param doCompletion + * @param callsShouldBeUses Enables inclusion of calls as uses so that the alias analysis can detect + * when local (stack) variable has been aliased by a function call. + * A value of false is fine for Java like languages where + * local (stack) variables cannot be passed by reference to function calls + * and hence cannot be aliased. However for C like languages this flag should be + * set to true. + */ TR_UseDefInfo::TR_UseDefInfo(TR::Compilation *comp, TR::CFG *cfg, TR::Optimizer *optimizer, - bool requiresGlobals, bool prefersGlobals, bool loadsShouldBeDefs, bool cannotOmitTrivialDefs, bool conversionRegsOnly, bool doCompletion) + bool requiresGlobals, bool prefersGlobals, bool loadsShouldBeDefs, bool cannotOmitTrivialDefs, bool conversionRegsOnly, + bool doCompletion, bool callsShouldBeUses) : _region(comp->trMemory()->heapMemoryRegion()), _compilation(comp), _optimizer(optimizer), @@ -91,6 +110,7 @@ TR_UseDefInfo::TR_UseDefInfo(TR::Compilation *comp, TR::CFG *cfg, TR::Optimizer _tempsOnly(false), _trace(comp->getOption(TR_TraceUseDefs)), _hasLoadsAsDefs(loadsShouldBeDefs), + _hasCallsAsUses(callsShouldBeUses), _useDefs(0, _region), _numMemorySymbols(0), _valueNumbersToMemorySymbolsMap(0, static_cast(NULL), _region), diff --git a/compiler/optimizer/UseDefInfo.hpp b/compiler/optimizer/UseDefInfo.hpp index aee44153624..95691a9375f 100644 --- a/compiler/optimizer/UseDefInfo.hpp +++ b/compiler/optimizer/UseDefInfo.hpp @@ -43,6 +43,7 @@ namespace TR { class CFG; } namespace TR { class ILOpCode; } namespace TR { class Optimizer; } namespace TR { class TreeTop; } +namespace OMR { class Optimizer; } /** * Use/def information. @@ -153,12 +154,31 @@ class TR_UseDefInfo friend class TR_OSRDefInfo; }; + friend class OMR::Optimizer; + protected: + /** + * Constructs TR_UseDefInfo instance. Note that this should not be called directly. + * Instead construction is handled by OMR::Optimizer::createUseDefInfo() method. + * + * @param cfg The compilation instance + * @param requiresGlobals + * @param prefersGlobals + * @param loadsShouldBeDefs + * @param cannotOmitTrivialDefs + * @param conversionRegsOnly + * @param doCompletion + * @param callsShouldBeUses Enables inclusion of calls as uses so that the alias analysis can detect + * when local (stack) variable has been aliased by a function call. + * A value of false is fine for Java like languages where + * local (stack) variables cannot be passed by reference to function calls + * and hence cannot be aliased. However for C like languages this flag should be + * set to true. + */ TR_UseDefInfo(TR::Compilation *, TR::CFG * cfg, TR::Optimizer *, bool requiresGlobals = true, bool prefersGlobals = true, bool loadsShouldBeDefs = true, bool cannotOmitTrivialDefs = false, - bool conversionRegsOnly = false, bool doCompletion = true); + bool conversionRegsOnly = false, bool doCompletion = true, bool callsShouldBeUses = false); - protected: void prepareUseDefInfo(bool requiresGlobals, bool prefersGlobals, bool cannotOmitTrivialDefs, bool conversionRegsOnly); void invalidateUseDefInfo(); virtual bool performAnalysis(AuxiliaryData &aux); diff --git a/compiler/optimizer/ValueNumberInfo.cpp b/compiler/optimizer/ValueNumberInfo.cpp index 0267ddf8740..9c2e7f48c84 100644 --- a/compiler/optimizer/ValueNumberInfo.cpp +++ b/compiler/optimizer/ValueNumberInfo.cpp @@ -93,7 +93,7 @@ TR_ValueNumberInfo::TR_ValueNumberInfo(TR::Compilation *comp, TR::Optimizer *opt { TR::LexicalMemProfiler mp("use defs (value numbers - I)", comp->phaseMemProfiler()); - _useDefInfo = new (comp->allocator()) TR_UseDefInfo(comp, comp->getFlowGraph(), optimizer, requiresGlobals, prefersGlobals); + _useDefInfo = optimizer->createUseDefInfo(comp, requiresGlobals, prefersGlobals); if (_useDefInfo->infoIsValid()) { _optimizer->setUseDefInfo(_useDefInfo); @@ -1533,7 +1533,7 @@ TR_HashValueNumberInfo::TR_HashValueNumberInfo(TR::Compilation *comp, TR::Optimi if (!(requiresGlobals && _optimizer->cantBuildGlobalsUseDefInfo())) { TR::LexicalMemProfiler mp("use defs (value numbers - II)", comp->phaseMemProfiler()); - _useDefInfo = new (comp->allocator()) TR_UseDefInfo(comp, comp->getFlowGraph(), optimizer, requiresGlobals, prefersGlobals); + _useDefInfo = optimizer->createUseDefInfo(comp, requiresGlobals, prefersGlobals); if (_useDefInfo->infoIsValid()) { _optimizer->setUseDefInfo(_useDefInfo);