Skip to content

Commit

Permalink
Consider guard or instanceof as escape point in Field Privatizer
Browse files Browse the repository at this point in the history
A reference or store to a field that came from an inlined method might
be guarded by a virtual guard.  In any particular execution of the loop,
such a field might not actually exist in the object that's being
referenced.  To avoid that situation, this change adds a simple check
for the existence of a virtual guard within the loop and considers it
to be an escape point in the containsEscapePoints method.

Similarly, instanceof tests might guard casts that will be used to
access fields that wouldn't necessarily exist every time a loop is
executed, so they are considered escape points as well.

Signed-off-by:  Henry Zongaro <zongaro@ca.ibm.com>
  • Loading branch information
hzongaro committed Oct 8, 2021
1 parent 2255420 commit c51edad
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 2 deletions.
55 changes: 53 additions & 2 deletions compiler/optimizer/FieldPrivatizer.cpp
Expand Up @@ -72,7 +72,9 @@ TR_FieldPrivatizer::TR_FieldPrivatizer(TR::OptimizationManager *manager)
_privatizedFieldSymRefs(manager->trMemory(), stackAlloc),
_privatizedRegCandidates(manager->trMemory()),
_appendCalls(manager->trMemory()),
_privatizedFieldNodes(manager->trMemory())
_privatizedFieldNodes(manager->trMemory()),
_subtreeCheckedForInstanceOf(manager->comp()),
_subtreeHasInstanceOf(manager->comp())
{
}

Expand Down Expand Up @@ -412,6 +414,44 @@ int32_t TR_FieldPrivatizer::detectCanonicalizedPredictableLoops(TR_Structure *lo



bool TR_FieldPrivatizer::subtreeHasInstanceOf(TR::Node *node)
{
bool hasInstanceOf = false;

if (_subtreeCheckedForInstanceOf.contains(node))
{
hasInstanceOf = _subtreeHasInstanceOf.contains(node);
}
else
{
if (node->getOpCodeValue() == TR::instanceof)
{
hasInstanceOf = true;
}
else
{
for (int i = 0; i < node->getNumChildren(); i++)
{
if (subtreeHasInstanceOf(node->getChild(i)))
{
hasInstanceOf = true;
}
}
}

_subtreeCheckedForInstanceOf.add(node);
if (hasInstanceOf)
{
_subtreeHasInstanceOf.add(node);
}
}

return hasInstanceOf;
}





bool TR_FieldPrivatizer::containsEscapePoints(TR_Structure *structure, bool &containsStringPeephole)
{
Expand All @@ -427,8 +467,19 @@ bool TR_FieldPrivatizer::containsEscapePoints(TR_Structure *structure, bool &con
{
TR::Node *currentNode = currentTree->getNode();

if (currentNode->exceptionsRaised())
// Check for situations that can be a problem for privatization:
// (1) Exceptions that might be thrown
// (2) A virtual guard or instanceof, as it might guard access of a field
// that might not be valid in any particular execution of the loop.
// The checks for virtual guards and instanceof could be refined
// further, as they might be unrelated to the fields that are of
// interest.
if (currentNode->exceptionsRaised()
|| currentNode->isTheVirtualGuardForAGuardedInlinedCall()
|| subtreeHasInstanceOf(currentNode))
{
result = true;
}

// DISABLED for now, although an excellent general purpose opt,
// this causes a regression, because the loop runs <= 1 times
Expand Down
24 changes: 24 additions & 0 deletions compiler/optimizer/FieldPrivatizer.hpp
Expand Up @@ -140,6 +140,16 @@ class TR_FieldPrivatizer : public TR_LoopTransformer
void detectFieldsThatCannotBePrivatized(TR::Node *, vcount_t);
void detectFieldsThatCannotBePrivatized(TR_Structure *, vcount_t);
bool canPrivatizeFieldSymRef(TR::Node *);

/**
* Checks whether \c currentNode represents a \ref TR::instanceof
* operation or whether any of its subtrees do.
*
* @param currentNode the node to check
* @return \c true if the subtree contains a \c TR::instanceof
* operation; \c false, otherwise.
*/
bool subtreeHasInstanceOf(TR::Node *currentNode);
bool containsEscapePoints(TR_Structure *, bool &);
void addPrivatizedRegisterCandidates(TR_Structure *);
bool isStringPeephole(TR::Node *, TR::TreeTop *);
Expand All @@ -163,6 +173,20 @@ class TR_FieldPrivatizer : public TR_LoopTransformer
List<TR::TreeTop> _appendCalls;

TR_PostDominators *_postDominators;

/**
* Tracks whether a node and its subtrees have been checked to
* see whether they contain a \ref TR::instanceof operation.
* If so, \ref _subtreeHasInstanceOf can be checked for the
* presence of an \c TR::instanceof in the subtree.
*/
TR::NodeChecklist _subtreeCheckedForInstanceOf;

/**
* Tracks whether a node or one or its subtrees performs a
* \ref TR::instanceof operation.
*/
TR::NodeChecklist _subtreeHasInstanceOf;
};


Expand Down

0 comments on commit c51edad

Please sign in to comment.