Skip to content

Commit

Permalink
Fix s2i improperly clobbering its child's register
Browse files Browse the repository at this point in the history
Previously, the Power codegen's implementation of the s2i opcode would
attempt to reuse the child node's register if its reference count was 1.
While this works most of the time, this causes issues if the child node
itself reused a register from its child. In this case, the clobbered
register may belong to a node whose reference count is not 1, which can
result in incorrect behaviour.

In general, there is no reason to try and reuse the register of the
child node in s2i, since an extsh instruction is always required. Doing
so does not avoid any register shuffles or additional instructions.
Accordingly, the s2i evaluator will now always allocate its own
register.

Signed-off-by: Ben Thomas <ben@benthomas.ca>
  • Loading branch information
aviansie-ben committed Feb 11, 2021
1 parent eccafd9 commit b1024d0
Showing 1 changed file with 8 additions and 43 deletions.
51 changes: 8 additions & 43 deletions compiler/p/codegen/UnaryEvaluator.cpp
Expand Up @@ -275,53 +275,18 @@ TR::Register *OMR::Power::TreeEvaluator::b2aEvaluator(TR::Node *node, TR::CodeGe
TR::Register *OMR::Power::TreeEvaluator::s2iEvaluator(TR::Node *node, TR::CodeGenerator *cg)
{
TR::Node *child = node->getFirstChild();
TR::Register *trgReg = 0;

TR_ASSERT(node->getOpCodeValue() == TR::s2i, "s2iEvaluator called for a %s node",
node->getOpCode().getName());
TR::Register *trgReg = cg->allocateRegister();

if (node->getOpCodeValue() == TR::s2i)// s2i sign extend
if (child->getOpCode().isLoad() && !child->getRegister() && child->getReferenceCount() == 1)
{
trgReg = child->getRegister();
if (!trgReg && child->getOpCode().isMemoryReference() && child->getReferenceCount() == 1)
{
trgReg = cg->allocateRegister();
TR::MemoryReference *tempMR = TR::MemoryReference::createWithRootLoadOrStore(cg, child, 2);
generateTrg1MemInstruction(cg, TR::InstOpCode::lha, node, trgReg, tempMR);
tempMR->decNodeReferenceCounts(cg);
}
else
{
if(!trgReg)
{
// child reg is null. need to evaluate child
trgReg = cg->evaluate(child);
if(child->getReferenceCount() == 1)
{
generateTrg1Src1Instruction(cg, TR::InstOpCode::extsh, node, trgReg, trgReg);
}
else
{
TR::Register * trgRegOld = trgReg;
trgReg = cg->allocateRegister();
generateTrg1Src1Instruction(cg, TR::InstOpCode::extsh, node, trgReg, trgRegOld);
}
}
else if(child->getReferenceCount() > 1)
{
// child has been evaluated, but will be used later
TR::Register * trgRegOld = trgReg;
trgReg = cg->allocateRegister();
generateTrg1Src1Instruction(cg, TR::InstOpCode::extsh, node, trgReg, trgRegOld);
}
else
{
generateTrg1Src1Instruction(cg, TR::InstOpCode::extsh, node, trgReg, trgReg);
}
}
TR::MemoryReference *tempMR = TR::MemoryReference::createWithRootLoadOrStore(cg, child, 2);
generateTrg1MemInstruction(cg, TR::InstOpCode::lha, node, trgReg, tempMR);
tempMR->decNodeReferenceCounts(cg);
}
else
trgReg = cg->gprClobberEvaluate(child);
{
generateTrg1Src1Instruction(cg, TR::InstOpCode::extsh, node, trgReg, cg->evaluate(child));
}

node->setRegister(trgReg);
cg->decReferenceCount(child);
Expand Down

0 comments on commit b1024d0

Please sign in to comment.