Skip to content

Commit

Permalink
Merge pull request #7509 from VermaSh/off-heap-unsafe-setMemory_fix
Browse files Browse the repository at this point in the history 
Fix loop strider reassociation and hoisting logic for off-heap
  • Loading branch information
@vijaysun-omr
vijaysun-omr authored Nov 6, 2024
2 parents aeffcb5 + c875d17 commit e59f482
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 39 deletions.
56 changes: 26 additions & 30 deletions compiler/optimizer/InductionVariable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
#include "optimizer/LoopCanonicalizer.hpp"
#include "optimizer/VPConstraint.hpp"
#include "ras/Debug.hpp"
#include "optimizer/TransformUtil.hpp"

#define OPT_DETAILS "O^O INDUCTION VARIABLE ANALYSIS: "

Expand Down Expand Up @@ -926,22 +927,8 @@ int32_t TR_LoopStrider::detectCanonicalizedPredictableLoops(TR_Structure *loopSt
traceMsg(comp(), "Found an reassociated induction var chance in %s\n", comp()->signature());

TR::SymbolReference *origAuto = symRefTab->getSymRef(j);

TR::Node *arrayRefNode;
int32_t hdrSize = (int32_t)TR::Compiler->om.contiguousArrayHeaderSizeInBytes();
if (usingAladd)
{
TR::Node *constantNode = TR::Node::create(byteCodeInfoNode, TR::lconst);
//constantNode->setLongInt(16);
constantNode->setLongInt((int64_t)hdrSize);
arrayRefNode = TR::Node::create(TR::aladd, 2,
TR::Node::createWithSymRef(byteCodeInfoNode,
TR::aload, 0, origAuto), constantNode);
}
else
arrayRefNode = TR::Node::create(TR::aiadd, 2,
TR::Node::createWithSymRef(byteCodeInfoNode, TR::aload, 0, origAuto),
TR::Node::create(byteCodeInfoNode, TR::iconst, 0, hdrSize));
TR::Node *newArrayObjNode = TR::Node::createWithSymRef(byteCodeInfoNode, TR::aload, 0, origAuto);
TR::Node *arrayRefNode = TR::TransformUtil::generateArrayElementAddressTrees(comp(), newArrayObjNode, NULL, byteCodeInfoNode);
arrayRefNode->setIsInternalPointer(true);

if (!origAuto->getSymbol()->isInternalPointer())
Expand Down Expand Up @@ -3397,6 +3384,20 @@ bool TR_LoopStrider::reassociateAndHoistComputations(TR::Block *loopInvariantBlo
int32_t originalInternalPointerSymbol = 0;
TR::Node *originalNode = NULL;
TR::Node *pinningArrayNode = NULL;
/* Tree structures eligible for reassociation and hoisting
non off-heap mode:
aladd (internal pointer)
array_obj (pinning array pointer)
add/sub offset
index
header_size/-header_size
off-heap mode:
aladd (internal pointer)
contiguousArrayDataAddrFieldSymbol (dataAddrPointer, internal pointer)
array_obj (pinning array pointer)
mul/shift/integer offset
*/
if (cg()->supportsInternalPointers() && reassociateAndHoistNonPacked() &&
node->isInternalPointer() && !node->isDataAddrPointer())
{
Expand All @@ -3405,6 +3406,8 @@ bool TR_LoopStrider::reassociateAndHoistComputations(TR::Block *loopInvariantBlo
else
pinningArrayNode = node->getFirstChild();

dumpOptDetails(comp(), "%s: Using %p as pinningArrayNode for internal pointer node %p\n", OPT_DETAILS, pinningArrayNode, node);

if (pinningArrayNode->getOpCode().isLoadVar() &&
pinningArrayNode->getSymbolReference()->getSymbol()->isAutoOrParm() &&
_neverWritten->get(pinningArrayNode->getSymbolReference()->getReferenceNumber()))
Expand Down Expand Up @@ -3652,10 +3655,12 @@ bool TR_LoopStrider::reassociateAndHoistComputations(TR::Block *loopInvariantBlo
}

#ifdef J9_PROJECT_SPECIFIC
// For OffHeap runs, the array access trees don't include headerSize addition
// Offset tree opcode is lmul/lshl to calculate offset from the index node.
if (TR::Compiler->om.isOffHeapAllocationEnabled() &&
(node->getOpCodeValue() == TR::lmul || node->getOpCodeValue() == TR::lshl))
/* For OffHeap runs, the array access trees don't include headerSize addition.
* Offset tree opcode can be either mul/shift or a number, if array stride is 1.
* If first child of originalNode is not dataAddr pointer we have already
* hoisted the array aload, no need to do it again.
*/
if (TR::Compiler->om.isOffHeapAllocationEnabled() && originalNode && originalNode->getFirstChild()->isDataAddrPointer())
{
if ((isInternalPointer &&
(comp()->getSymRefTab()->getNumInternalPointers() < maxInternalPointers())) &&
Expand Down Expand Up @@ -3709,20 +3714,11 @@ bool TR_LoopStrider::reassociateAndHoistComputations(TR::Block *loopInvariantBlo

TR::SymbolReference *internalPointerSymRef = (*_reassociatedAutos)[originalInternalPointerSymbol];
originalNode->getFirstChild()->recursivelyDecReferenceCount();
node->decReferenceCount();
_reassociatedNodes.add(node);

if (node->getReferenceCount() == 0)
{
node->getFirstChild()->decReferenceCount();
node->getSecondChild()->decReferenceCount();
}

TR::Node *newLoad = TR::Node::createWithSymRef(node, TR::aload, 0, internalPointerSymRef);
newLoad->setLocalIndex(~0);
originalNode->setAndIncChild(0, newLoad);
originalNode->setAndIncChild(1, node->getFirstChild());
reassociatedComputation = true;
dumpOptDetails(comp(), "%s: Replaced array load in %p with %p\n", OPT_DETAILS, originalNode, newLoad);
}
}
#endif /* J9_PROJECT_SPECIFIC */
Expand Down
16 changes: 8 additions & 8 deletions compiler/optimizer/OMRTransformUtil.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ OMR::TransformUtil::generateDataAddrLoadTrees(TR::Compilation *comp, TR::Node *a
#endif /* OMR_GC_SPARSE_HEAP_ALLOCATION */

TR::Node *
OMR::TransformUtil::generateArrayElementAddressTrees(TR::Compilation *comp, TR::Node *arrayNode, TR::Node *offsetNode)
OMR::TransformUtil::generateArrayElementAddressTrees(TR::Compilation *comp, TR::Node *arrayNode, TR::Node *offsetNode, TR::Node *originatingByteCodeNode)
{
TR::Node *arrayAddressNode = NULL;
TR::Node *totalOffsetNode = NULL;
Expand All @@ -468,24 +468,24 @@ OMR::TransformUtil::generateArrayElementAddressTrees(TR::Compilation *comp, TR::
{
arrayAddressNode = generateDataAddrLoadTrees(comp, arrayNode);
if (offsetNode)
arrayAddressNode = TR::Node::create(TR::aladd, 2, arrayAddressNode, offsetNode);
arrayAddressNode = TR::Node::create(originatingByteCodeNode, TR::aladd, 2, arrayAddressNode, offsetNode);
}
else if (comp->target().is64Bit())
#else
if (comp->target().is64Bit())
#endif /* OMR_GC_SPARSE_HEAP_ALLOCATION */
{
totalOffsetNode = TR::Node::lconst(TR::Compiler->om.contiguousArrayHeaderSizeInBytes());
totalOffsetNode = TR::Node::lconst(originatingByteCodeNode, TR::Compiler->om.contiguousArrayHeaderSizeInBytes());
if (offsetNode)
totalOffsetNode = TR::Node::create(TR::ladd, 2, offsetNode, totalOffsetNode);
arrayAddressNode = TR::Node::create(TR::aladd, 2, arrayNode, totalOffsetNode);
totalOffsetNode = TR::Node::create(originatingByteCodeNode, TR::ladd, 2, offsetNode, totalOffsetNode);
arrayAddressNode = TR::Node::create(originatingByteCodeNode, TR::aladd, 2, arrayNode, totalOffsetNode);
}
else
{
totalOffsetNode = TR::Node::iconst(static_cast<int32_t>(TR::Compiler->om.contiguousArrayHeaderSizeInBytes()));
totalOffsetNode = TR::Node::iconst(originatingByteCodeNode, static_cast<int32_t>(TR::Compiler->om.contiguousArrayHeaderSizeInBytes()));
if (offsetNode)
totalOffsetNode = TR::Node::create(TR::iadd, 2, offsetNode, totalOffsetNode);
arrayAddressNode = TR::Node::create(TR::aiadd, 2, arrayNode, totalOffsetNode);
totalOffsetNode = TR::Node::create(originatingByteCodeNode, TR::iadd, 2, offsetNode, totalOffsetNode);
arrayAddressNode = TR::Node::create(originatingByteCodeNode, TR::aiadd, 2, arrayNode, totalOffsetNode);
}

return arrayAddressNode;
Expand Down
3 changes: 2 additions & 1 deletion compiler/optimizer/OMRTransformUtil.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ class OMR_EXTENSIBLE TransformUtil
static TR::Node *generateArrayElementAddressTrees(
TR::Compilation *comp,
TR::Node *arrayNode,
TR::Node *offsetNode = NULL);
TR::Node *offsetNode = NULL,
TR::Node *originatingByteCodeNode = NULL);

/**
* \brief
Expand Down

0 comments on commit e59f482

Please sign in to comment.