Skip to content

Commit

Permalink
Refactor the optimizer to allocate UseDefInfo through a central location
Browse files Browse the repository at this point in the history
This change implements the recommended refactoring suggested by
@andrewcraik in the review comments for
#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 <mobile@majumdar.org.uk>
  • Loading branch information
dibyendumajumdar committed Oct 22, 2019
1 parent bfe4c50 commit 775d7cf
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 8 deletions.
2 changes: 1 addition & 1 deletion compiler/optimizer/LoadExtensions.cpp
Expand Up @@ -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())
{
Expand Down
11 changes: 9 additions & 2 deletions compiler/optimizer/OMROptimizer.cpp
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down
30 changes: 30 additions & 0 deletions compiler/optimizer/OMROptimizer.hpp
Expand Up @@ -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 );
Expand All @@ -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()); }

Expand Down
22 changes: 21 additions & 1 deletion compiler/optimizer/UseDefInfo.cpp
Expand Up @@ -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),
Expand All @@ -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<MemorySymbolList *>(NULL), _region),
Expand Down
24 changes: 22 additions & 2 deletions compiler/optimizer/UseDefInfo.hpp
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions compiler/optimizer/ValueNumberInfo.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 775d7cf

Please sign in to comment.