Skip to content

Commit

Permalink
Fix the check on isStoreIndirect in LocalAnalysis
Browse files Browse the repository at this point in the history
In `hasOldExpressionOnRhs()`, we temporarily change wrtbar to aloadi
for the syntactic comparison in `areSyntacticallyEquivalent()`.
When it’s an indirect store, the number of children of the node is
set to 1, otherwise 0. Because the check on whether not the node
is an indirect store happens after the wrtbar node is recreated as aloadi,
`node->getOpCode().isStoreIndirect()` is false for aloadi, and the
number of children for wrtbar node ends up as 0. It prevents
`areSyntacticallyEquivalent()` from comparing the first child of the two
nodes. It concludes that different expressions as the same.
The fix is to check if it’s an indirect store from the original node.

Fixes eclipse-openj9/openj9#11569

Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
  • Loading branch information
a7ehuo committed Jan 12, 2021
1 parent bded46c commit b7ee89d
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions compiler/optimizer/LocalAnalysis.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2020 IBM Corp. and others
* Copyright (c) 2000, 2021 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -488,7 +488,7 @@ int TR_LocalAnalysisInfo::hasOldExpressionOnRhs(TR::Node *node, bool recalcConta
#endif
bool seenWriteBarrier = false;
int32_t storeNumChildren = node->getNumChildren();

bool isStoreIndirect = node->getOpCode().isStoreIndirect();

// If node is a null check, compare the
// null check reference only to establish syntactic equivalence
Expand Down Expand Up @@ -528,7 +528,7 @@ int TR_LocalAnalysisInfo::hasOldExpressionOnRhs(TR::Node *node, bool recalcConta
#endif

TR::Node::recreate(node, _compilation->il.opCodeForCorrespondingLoadOrStore(node->getOpCodeValue()));
if (node->getOpCode().isStoreIndirect())
if (isStoreIndirect)
{
node->setNumChildren(1);
}
Expand All @@ -548,6 +548,7 @@ int TR_LocalAnalysisInfo::hasOldExpressionOnRhs(TR::Node *node, bool recalcConta
int32_t hashValue = _hashTable->hash(node);
HashTable::Cursor cursor(_hashTable, hashValue);
TR::Node *other;
bool tmpIsStoreIndirect = false;
for (other = cursor.firstNode(); other; other = cursor.nextNode())
{
// Convert other node's opcode to be a load temporarily
Expand All @@ -566,12 +567,13 @@ int TR_LocalAnalysisInfo::hasOldExpressionOnRhs(TR::Node *node, bool recalcConta
if (other->getOpCode().isWrtBar())
seenOtherWriteBarrier = true;

tmpIsStoreIndirect = other->getOpCode().isStoreIndirect();
#ifdef J9_PROJECT_SPECIFIC
seenOtherIndirectBCDStore = other->getType().isBCD();
#endif
TR::Node::recreate(other, _compilation->il.opCodeForCorrespondingLoadOrStore(other->getOpCodeValue()));

if (other->getOpCode().isStoreIndirect())
if (tmpIsStoreIndirect)
{
other->setNumChildren(1);
}
Expand Down

0 comments on commit b7ee89d

Please sign in to comment.