Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AArch64: Properly find compressed refs sequence in implicit NULLCHK #12351

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 34 additions & 22 deletions runtime/compiler/aarch64/codegen/J9TreeEvaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3301,13 +3301,11 @@ J9::ARM64::TreeEvaluator::evaluateNULLCHKWithPossibleResolve(TR::Node *node, boo
// pattern match the sequence under the l2a
// NULLCHK NULLCHK <- node
// aloadi f l2a
// aload O ladd
// lshl
// i2l
// iloadi/irdbari f <- n
// aload O <- reference
// iconst shftKonst
// lconst HB
// aload O lshl
// iu2l
// iloadi/irdbari f <- n
// aload O <- reference
// iconst shftKonst
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the compressed sequence tree in the comment to be consistent with the current IL.
We do not have ladd anymore since heapbase has been eliminated in #11688

//
hasCompressedPointers = true;
TR::ILOpCodes loadOp = cg->comp()->il.opCodeForIndirectLoad(TR::Int32);
Expand All @@ -3331,6 +3329,7 @@ J9::ARM64::TreeEvaluator::evaluateNULLCHKWithPossibleResolve(TR::Node *node, boo

bool needExplicitCheck = true;
bool needLateEvaluation = true;
bool firstChildEvaluated = false;

// Add the explicit check after this instruction
//
Expand All @@ -3339,9 +3338,8 @@ J9::ARM64::TreeEvaluator::evaluateNULLCHKWithPossibleResolve(TR::Node *node, boo
// determine if an explicit check is needed
if (cg->getHasResumableTrapHandler())
{
if (opCode.isLoadVar()
|| (opCode.getOpCodeValue() == TR::l2i)
|| (hasCompressedPointers && firstChild->getFirstChild()->getOpCode().getOpCodeValue() == TR::i2l))
if (n->getOpCode().isLoadVar()
|| (opCode.getOpCodeValue() == TR::l2i))
{
TR::SymbolReference *symRef = NULL;

Expand All @@ -3362,19 +3360,29 @@ J9::ARM64::TreeEvaluator::evaluateNULLCHKWithPossibleResolve(TR::Node *node, boo

// at this point, n is the raw iloadi (created by lowerTrees) and
// reference is the aload of the object. node->getFirstChild is the
// l2a sequence; as a result, n's refCount will always be 1
// and node->getFirstChild's refCount will be at least 2 (one under the nullchk
// and the other under the translate treetop)
// l2a sequence; as a result, n's refCount will always be 1.
//
if (hasCompressedPointers
&& node->getFirstChild()->getReferenceCount() > 2)
&& node->getFirstChild()->getReferenceCount() >= 2)
{
// In this case, the result of load is used in other places, so we need to evaluate it here
//
needLateEvaluation = true;

// Check if offset from a NULL reference will fall into the inaccessible bytes,
// resulting in an implicit trap being raised.
if (symRef
&& ((symRef->getSymbol()->getOffset() + symRef->getOffset()) < cg->getNumberBytesReadInaccessible()))
{
needExplicitCheck = false;
}
}
}

// Check if offset from a NULL reference will fall into the inaccessible bytes,
// resulting in an implicit trap being raised.
else if (symRef
&& ((symRef->getSymbol()->getOffset() + symRef->getOffset()) < cg->getNumberBytesReadInaccessible()))
&& ((symRef->getSymbol()->getOffset() + symRef->getOffset()) < cg->getNumberBytesReadInaccessible()))
{
needExplicitCheck = false;

Expand Down Expand Up @@ -3407,7 +3415,6 @@ J9::ARM64::TreeEvaluator::evaluateNULLCHKWithPossibleResolve(TR::Node *node, boo
{
needLateEvaluation = false;
needExplicitCheck = true;
reference->incReferenceCount(); // will be decremented again later
}
}
}
Expand Down Expand Up @@ -3461,17 +3468,15 @@ J9::ARM64::TreeEvaluator::evaluateNULLCHKWithPossibleResolve(TR::Node *node, boo
// The child may generate inline code that provides an implicit null check
// but we won't know until the child is evaluated.
//
reference->incReferenceCount(); // will be decremented again later
needLateEvaluation = false;
cg->evaluate(reference);
appendTo = cg->getAppendInstruction();
cg->evaluate(firstChild);

firstChildEvaluated = true;
if (cg->getImplicitExceptionPoint()
&& (cg->getNumberBytesReadInaccessible() > cg->fe()->getOffsetOfContiguousArraySizeField()))
{
needExplicitCheck = false;
cg->decReferenceCount(reference);
}
}
}
Expand Down Expand Up @@ -3503,12 +3508,19 @@ J9::ARM64::TreeEvaluator::evaluateNULLCHKWithPossibleResolve(TR::Node *node, boo
if (needLateEvaluation)
{
cg->evaluate(firstChild);
firstChildEvaluated = true;
}
else if (needExplicitCheck)
// If the firstChild is evaluated, we simply call decReferenceCount.
// Otherwise, we need to call recursivelyDecReferenceCount so that the ref count of
// child nodes of the firstChild is properly decremented when the ref count of the firstChild is 1.
if (firstChildEvaluated)
{
cg->decReferenceCount(reference);
cg->decReferenceCount(firstChild);
}
else
{
cg->recursivelyDecReferenceCount(firstChild);
}
cg->decReferenceCount(firstChild);

// If an explicit check has not been generated for the null check, there is
// an instruction that will cause a hardware trap if the exception is to be
Expand Down