Fix array load in transformNullRestrictedArrayCopy

Load the source array and the destination array from the saved temp instead of commoning the original nodes incorrectly across blocks. Related: eclipse-openj9/openj9#19913, eclipse-openj9/openj9#19914 Signed-off-by: Annabelle Huo <[email protected]>
eclipse · Oct 8, 2024 · 833607b · 833607b
1 parent 389628f
commit 833607b
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 12 deletions.
diff --git a/compiler/optimizer/OMRValuePropagation.hpp b/compiler/optimizer/OMRValuePropagation.hpp
@@ -616,15 +616,16 @@ class ValuePropagation : public TR::Optimization
  {
  TR_ALLOC(TR_Memory::ValuePropagation)
 
- TR_NeedRuntimeTestNullRestrictedArrayCopy(TR::Node *dstArrRef, TR::Node *srcArrRef,
+ TR_NeedRuntimeTestNullRestrictedArrayCopy(TR::SymbolReference *dstArrRefSymRef, TR::SymbolReference *srcArrRefSymRef,
  TR::TreeTop *ptt, TR::TreeTop *ntt,
  TR::Block *originBlock, TR::Block *slowBlock,
  bool testDstArray)
- : _dstArrayRefNode(dstArrRef), _srcArrayRefNode(srcArrRef), _prevTT(ptt), _nextTT(ntt), _originBlock(originBlock), _slowBlock(slowBlock), _needRuntimeTestDstArray(testDstArray)
+ : _dstArrRefSymRef(dstArrRefSymRef), _srcArrRefSymRef(srcArrRefSymRef), _prevTT(ptt), _nextTT(ntt),
+ _originBlock(originBlock), _slowBlock(slowBlock), _needRuntimeTestDstArray(testDstArray)
  {}
 
- TR::Node *_dstArrayRefNode;
- TR::Node *_srcArrayRefNode;
+ TR::SymbolReference * _dstArrRefSymRef;
+ TR::SymbolReference * _srcArrRefSymRef;
 
  TR::TreeTop *_prevTT;
  TR::TreeTop *_nextTT;

diff --git a/compiler/optimizer/ValuePropagationCommon.cpp b/compiler/optimizer/ValuePropagationCommon.cpp
@@ -1496,6 +1496,9 @@ void OMR::ValuePropagation::transformArrayCopyCall(TR::Node *node)
  if (trace())
  traceMsg(comp(),"Creating temps for children of the original call node n%dn %p. new call node n%dn %p\n", oldCallNode->getGlobalIndex(), oldCallNode, newCallNode->getGlobalIndex(), newCallNode);
 
+ TR::SymbolReference * dstArrRefSymRef = NULL;
+ TR::SymbolReference * srcArrRefSymRef = NULL;
+
  // Create temporaries for System.arraycopy arguments and replace the children of the new call node with the temps
  for (int32_t i = 0 ; i < oldCallNode->getNumChildren(); ++i)
  {
@@ -1516,10 +1519,17 @@ void OMR::ValuePropagation::transformArrayCopyCall(TR::Node *node)
  _curTree->insertBefore(savedChildTree);
 
  if (trace())
- traceMsg(comp(),"Created child n%dn %p for old child n%dn %p of the original call node\n", savedChildNode->getGlobalIndex(), savedChildNode, child->getGlobalIndex(), child);
+ traceMsg(comp(),"Created child n%dn %p #%d for old child n%dn %p of the original call node\n", savedChildNode->getGlobalIndex(), savedChildNode, newSymbolReference->getReferenceNumber(),
+ child->getGlobalIndex(), child);
 
  // Create the child for the new call node with a load of the new sym ref
  value = TR::Node::createLoad(newCallNode, newSymbolReference);
+
+ if (child == dstObjNode)
+ dstArrRefSymRef = newSymbolReference;
+
+ if (child == srcObjNode)
+ srcArrRefSymRef = newSymbolReference;
  }
 
  if (trace())
@@ -1539,13 +1549,16 @@ void OMR::ValuePropagation::transformArrayCopyCall(TR::Node *node)
  nextTT = nextTT->getNextTreeTop();
 
  if (trace())
- traceMsg(comp(), "%s: n%dn %p current block_%d slowBlock block_%d newCallTree n%dn %p srcObjNode n%dn %p dstObjNode n%dn %p prevTT n%dn %p nextTT n%dn %p\n",
- __FUNCTION__, node->getGlobalIndex(), node, _curTree->getEnclosingBlock()->getNumber(), slowBlock->getNumber(),
- newCallTree->getNode()->getGlobalIndex(), newCallTree->getNode(),
- srcObjNode->getGlobalIndex(), srcObjNode, dstObjNode->getGlobalIndex(), dstObjNode,
+ {
+ traceMsg(comp(), "%s: n%dn %p current block_%d slowBlock block_%d newCallTree n%dn %p prevTT n%dn %p nextTT n%dn %p\n", __FUNCTION__, node->getGlobalIndex(), node,
+ _curTree->getEnclosingBlock()->getNumber(), slowBlock->getNumber(), newCallTree->getNode()->getGlobalIndex(), newCallTree->getNode(),
  prevTT->getNode()->getGlobalIndex(), prevTT->getNode(), nextTT->getNode()->getGlobalIndex(), nextTT->getNode());
 
- _needRuntimeTestNullRestrictedArrayCopy.add(new (trStackMemory()) TR_NeedRuntimeTestNullRestrictedArrayCopy(dstObjNode, srcObjNode,
+ traceMsg(comp(), "%s: srcObjNode n%dn %p #%d dstObjNode n%dn %p #%d\n", __FUNCTION__, srcObjNode->getGlobalIndex(), srcObjNode, srcArrRefSymRef->getReferenceNumber(),
+ dstObjNode->getGlobalIndex(), dstObjNode, dstArrRefSymRef->getReferenceNumber());
+ }
+
+ _needRuntimeTestNullRestrictedArrayCopy.add(new (trStackMemory()) TR_NeedRuntimeTestNullRestrictedArrayCopy(dstArrRefSymRef, srcArrRefSymRef,
  prevTT, nextTT,
  _curTree->getEnclosingBlock(), slowBlock,
  needRuntimeTestDstArray));
@@ -3833,8 +3846,10 @@ slowBlock-> n39n BBStart <block_10> (freq 0) (cold)
 
  TR_ASSERT_FATAL(needTestSrcArray || needTestDstArray, "needTestSrcArray %d needTestDstArray %d should not both be false\n", needTestSrcArray, needTestDstArray);
 
- TR::Node *dstArrayRefNode = nullRestrictedArrayCopy->_dstArrayRefNode;
- TR::Node *srcArrayRefNode = nullRestrictedArrayCopy->_srcArrayRefNode;
+ TR::SymbolReference *dstArrRefSymRef = nullRestrictedArrayCopy->_dstArrRefSymRef;
+ TR::SymbolReference *srcArrRefSymRef = nullRestrictedArrayCopy->_srcArrRefSymRef;
+ TR::Node *dstArrayRefNode = TR::Node::createLoad(dstArrRefSymRef);
+ TR::Node *srcArrayRefNode = TR::Node::createLoad(srcArrRefSymRef);
 
  TR::Block *originBlock = nullRestrictedArrayCopy->_originBlock;
  TR::Block *slowBlock = nullRestrictedArrayCopy->_slowBlock;