Skip to content

Commit

Permalink
Fixes in RegDepCopyRemoval for postGRA block splitter
Browse files Browse the repository at this point in the history
RegDepCopyRemoval is the last ran optimization which intends to reduce
register shuffling due to global register dependencies. It was
generating tree where we have a PassThrough Child under PassThrough to
copy node to new virtual register. This tree was not working well with
the post GRA block splitter. Issue was that PassThrough child
encountered in GlRegDeps was anchored to tree top. In postGRA block
splitter, it records the regStores encountered before the GlRegDeps in
extended basic block to guide decision on which register to chose for
the node after split point.In this commit, adding changes to both
splitPostGRA block splitter and RegDepCopyRemoval optimization so that
we can split the blocks after later has run.

Signed-off-by: Rahil Shah <rahil@ca.ibm.com>
  • Loading branch information
r30shah committed Sep 2, 2020
1 parent c7312b8 commit 104d2c3
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 10 deletions.
10 changes: 7 additions & 3 deletions compiler/il/OMRBlock.cpp
Expand Up @@ -1020,6 +1020,9 @@ static void gatherUnavailableRegisters(TR::Compilation *comp, TR::Node *regDeps,
if (dep->getOpCodeValue() == TR::PassThrough)
{
TR::Node *value = dep->getFirstChild();
TR::Node *originalNode = value;
while (originalNode->getOpCodeValue() == TR::PassThrough)
originalNode = originalNode->getFirstChild();
auto nodeInfoEntry = nodeInfo->find(value);
// If a node referenced under the PassThrough node post split point requires uncommoning
// We need to check if the we can use the information to allocate the register for the node or not.
Expand All @@ -1045,7 +1048,7 @@ static void gatherUnavailableRegisters(TR::Compilation *comp, TR::Node *regDeps,
if (checkIfRegisterIsAvailable(comp, storeNode, unavailableRegisters))
{
// Whether the PassThrough uses same register or not, use the last recorded regStore to assign register to the node.
TR::Node *regLoad = TR::Node::create(value, comp->il.opCodeForRegisterLoad(value->getDataType()));
TR::Node *regLoad = TR::Node::create(value, comp->il.opCodeForRegisterLoad(originalNode->getDataType()));
regLoad->setRegLoadStoreSymbolReference(storeNode->getRegLoadStoreSymbolReference());
regLoad->setGlobalRegisterNumber(storeNode->getGlobalRegisterNumber());
unavailableRegisters.set(storeNode->getGlobalRegisterNumber());
Expand All @@ -1068,15 +1071,16 @@ static void gatherUnavailableRegisters(TR::Compilation *comp, TR::Node *regDeps,
dep->getLowGlobalRegisterNumber() == nodeInfoEntry->second.second->getLowGlobalRegisterNumber() &&
dep->getHighGlobalRegisterNumber() == nodeInfoEntry->second.second->getHighGlobalRegisterNumber())
{
// Passthrough uses same register as the replacement, do not need to do anything
regDeps->setAndIncChild(i, nodeInfoEntry->second.second);
dep->recursivelyDecReferenceCount();
}
else if (needToCreateRegStore && (!needToCheckStoreRegPostSplitPoint || !checkStoreRegNodeListForNode(dep, storeRegNodePostSplitPoint)))
{
// We have either used different register for replacement to value node after split point or Passthrough has corresponding regStore before spilt point, but registers were unavailable.
// In these cases we need to create a regStore for the PassThrough.
// If we have replacement then need to store the replacement to register, else, value needs to be stored.
TR::Node *nodeToBeStored = nodeInfoEntry->second.second != NULL ? nodeInfoEntry->second.second : value;
TR::Node *regStore = TR::Node::create(value, comp->il.opCodeForRegisterStore(value->getDataType()), 1, nodeToBeStored);
TR::Node *regStore = TR::Node::create(value, comp->il.opCodeForRegisterStore(originalNode->getDataType()), 1, nodeToBeStored);
currentTT->insertBefore(TR::TreeTop::create(comp, regStore));
regStore->setGlobalRegisterNumber(dep->getGlobalRegisterNumber());
if (nodeToBeStored->requiresRegisterPair(comp))
Expand Down
52 changes: 45 additions & 7 deletions compiler/optimizer/RegDepCopyRemoval.cpp
Expand Up @@ -98,6 +98,14 @@ TR::RegDepCopyRemoval::perform()
processRegDeps(lastChild, tt);
}
}
else if (node->getOpCode().isStoreReg()
&& node->getHighGlobalRegisterNumber() == static_cast<TR_GlobalRegisterNumber>(-1)
&& (node->getType().isIntegral() || node->getType().isAddress()))
{
TR_GlobalRegisterNumber lowReg = node->getLowGlobalRegisterNumber();
NodeChoice &choice = getNodeChoice(lowReg);
choice.regStoreNode = node;
}
break;
}
}
Expand Down Expand Up @@ -136,7 +144,7 @@ void
TR::RegDepCopyRemoval::discardAllNodeChoices()
{
for (TR_GlobalRegisterNumber reg = _regBegin; reg < _regEnd; reg++)
discardNodeChoice(reg);
clearNodeChoice(reg);
}

void
Expand All @@ -147,6 +155,15 @@ TR::RegDepCopyRemoval::discardNodeChoice(TR_GlobalRegisterNumber reg)
choice.selected = NULL;
}

void
TR::RegDepCopyRemoval::clearNodeChoice(TR_GlobalRegisterNumber reg)
{
NodeChoice &choice = getNodeChoice(reg);
choice.original = NULL;
choice.selected = NULL;
choice.regStoreNode = NULL;
}

void
TR::RegDepCopyRemoval::rememberNodeChoice(TR_GlobalRegisterNumber reg, TR::Node *selected)
{
Expand Down Expand Up @@ -218,15 +235,24 @@ TR::RegDepCopyRemoval::readRegDeps()
continue;
}

// Only process integral and address-type nodes; they'll go into GPRs

TR_GlobalRegisterNumber reg = depNode->getGlobalRegisterNumber();
TR::DataType depType = depValue->getType();
if (!depType.isIntegral() && !depType.isAddress())
// Only process integral and address-type nodes; they'll go into GPRs
if (!depType.isIntegral() && !depType.isAddress())
{
ignoreRegister(reg);
continue;
}

NodeChoice &choice = getNodeChoice(reg);
// In a rare case, for the given register last regStore we have seen used different node then the node under PassThrough
// Skipping such cases.
if (choice.regStoreNode != NULL && choice.regStoreNode->getFirstChild() != depValue)
{
ignoreRegister(reg);
continue;
}

RegDepInfo &dep = getRegDepInfo(reg);
TR_ASSERT(dep.state == REGDEP_ABSENT, "register %s is multiply-specified\n", registerName(reg));
dep.node = depNode;
Expand Down Expand Up @@ -408,9 +434,21 @@ TR::RegDepCopyRemoval::makeFreshCopy(TR_GlobalRegisterNumber reg)
copyNode = TR::Node::create(TR::PassThrough, 1, dep.value);
copyNode->setCopyToNewVirtualRegister();
}

TR::Node *copyTreetopNode = TR::Node::create(TR::treetop, 1, copyNode);
_treetop->insertBefore(TR::TreeTop::create(comp(), copyTreetopNode));
NodeChoice &choice = getNodeChoice(reg);
if (choice.regStoreNode == NULL)
{
// As we walk down in Extended Basic Block, for each register, if exists, we record a regStore node, if we do not have one found for given register, node should be regLoad.
TR_ASSERT_FATAL(dep.node->getOpCode().isLoadReg(), "Only PassThrough (with a corresponding regStore appeared before) or regLoad is expected as children of GlRegDeps, Unexpected Node is n%dn OpCode %s",dep.node->getGlobalIndex(), dep.node->getOpCode().getName());
choice.regStoreNode = TR::Node::create(dep.node, comp()->il.opCodeForRegisterStore(dep.node->getDataType()), 1, copyNode);
_treetop->insertBefore(TR::TreeTop::create(comp(), choice.regStoreNode));
choice.regStoreNode->setGlobalRegisterNumber(dep.node->getGlobalRegisterNumber());
choice.regStoreNode->setRegLoadStoreSymbolReference(dep.node->getRegLoadStoreSymbolReference());
}
else
{
choice.regStoreNode->setAndIncChild(0, copyNode);
dep.value->recursivelyDecReferenceCount();
}
if (trace())
traceMsg(comp(), "\tcopy is n%un\n", copyNode->getGlobalIndex());

Expand Down
2 changes: 2 additions & 0 deletions compiler/optimizer/RegDepCopyRemoval.hpp
Expand Up @@ -97,6 +97,7 @@ class RegDepCopyRemoval : public TR::Optimization
{
TR::Node *original;
TR::Node *selected;
TR::Node *regStoreNode;
};

const char *registerName(TR_GlobalRegisterNumber reg);
Expand All @@ -106,6 +107,7 @@ class RegDepCopyRemoval : public TR::Optimization

void discardAllNodeChoices();
void discardNodeChoice(TR_GlobalRegisterNumber reg);
void clearNodeChoice(TR_GlobalRegisterNumber reg);
void rememberNodeChoice(TR_GlobalRegisterNumber reg, TR::Node *selected);

void processRegDeps(TR::Node *deps, TR::TreeTop *depTT);
Expand Down

0 comments on commit 104d2c3

Please sign in to comment.