Skip to content

Commit 6102dee

Browse files
authored
Create missing state output in MSE for allocas (#1942)
I first tried to disqualify dynamic allocas from being simple in the ModRefSummarizer. At the most, a single file has 99 dynamic allocas, and 12 files have >= 10 dynamic allocas. 164 files have at least one dynamic alloca. Not all dynamic allocas were simple to begin with, however, so the actual reduction in simple allocas was at most 15, with only the top 10 files seeing >= 4 simple allocas removed. At the end of the sroa-raware pipeline, the change lead to **60 more loads** compared to master, across 14 files (12 up, 2 down). It also lead to **108 more stores**, but I think those might actually be mostly correct, and were always supposed to be there. Lastly it lead to **11 more allocas** being kept. Next I tried what this PR does: Creating an UndefValue for the memory state output if it does not exist yet when required by something that is not an alloca. This was a much smaller code change, and I think it makes much more sense, as it avoids adding even more special handling of the set of allocas that is or isn't included in the ModRefSummary of functions. I left a suggestion for an assert in the PR as a comment. The assert it not possible right now since the StateMap class not having access to the PointsToGraph, but the whole MSE class is being refactored, so I would rather just add it as a comment that can be fixed later. The total effect on the sroa-raware pipeline compared to master: **4 more loads, 105 more stores**. The load diffs are in 6 files: (+17, +17, +4, -1, -2, -31). The store diffs are in 8 files, which I again think are mostly supposed to be there. We keep **8 more allocas**.
1 parent 6f666ce commit 6102dee

1 file changed

Lines changed: 45 additions & 20 deletions

File tree

‎jlm/llvm/opt/alias-analyses/MemoryStateEncoder.cpp‎

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -435,20 +435,42 @@ class MemoryStateEncoder::StateMap final
435435
}
436436

437437
/**
438-
* Gets MemoryNodeStatePairs for each of the given memory nodes,
439-
* unless there is no memory state in the region representing the memory node.
438+
* Gets MemoryNodeStatePairs for each of the given memory nodes.
439+
* If no memory state output exists for a given memory node, an UndefValue node is created.
440+
* This only happens when a \ref ModRefSet contains alloca whose memory state is not routed
441+
* in as a function argument, and the alloca operation has not been encoded yet.
442+
* This can happen when allocas are inside subregions, or when the allocation count is a
443+
* runtime value that depends on a load.
444+
*
440445
* @param modRefSet the set of memory nodes to retrieve states for.
441-
* @return The MemoryNodeStatePairs for each given memory nodes, if one exists.
442-
* @see RegionalizedStateMap::GetExistingStates()
446+
* @param region the region in which the states are needed
447+
*
448+
* @return The MemoryNodeStatePairs for each given memory nodes.
443449
*/
444450
std::vector<MemoryNodeStatePair *>
445-
GetExistingStates(const ModRefSet & modRefSet)
451+
GetOrCreateStates(const ModRefSet & modRefSet, rvsdg::Region & region)
446452
{
447453
std::vector<MemoryNodeStatePair *> memoryNodeStatePairs;
448-
for (auto & [memoryNode, _] : modRefSet.getModRefNodes())
454+
for (auto & [memoryNode, modRefEffect] : modRefSet.getModRefNodes())
449455
{
456+
JLM_ASSERT(modRefEffect != ModRefEffect::NoEffect);
457+
450458
if (const auto statePair = TryGetState(memoryNode))
459+
{
451460
memoryNodeStatePairs.push_back(statePair);
461+
}
462+
else
463+
{
464+
// If no memory state output exists for the memory node, create an UndefValue for it
465+
466+
// Using undef for memory states that do not exist yet should only be done for allocas.
467+
// TODO: After refactoring, add an assert here like so:
468+
// JLM_ASSERT(modRefSummary_->getPointsToGraph().getKind(memoryNode) == NodeKind::Alloca);
469+
470+
auto & undefOutput = *UndefValueOperation::Create(region, MemoryStateType::Create());
471+
auto insertedPair = InsertState(memoryNode, undefOutput);
472+
memoryNodeStatePairs.push_back(insertedPair);
473+
}
452474
}
453475

454476
return memoryNodeStatePairs;
@@ -788,7 +810,7 @@ MemoryStateEncoder::EncodeLoad(const rvsdg::SimpleNode & node, StateMap & stateM
788810
JLM_ASSERT(is<LoadOperation>(node.GetOperation()));
789811

790812
const auto & modRefSet = modRefSummary_->GetSimpleNodeModRef(node);
791-
const auto memoryNodeStatePairs = stateMap.GetExistingStates(modRefSet);
813+
const auto memoryNodeStatePairs = stateMap.GetOrCreateStates(modRefSet, *node.region());
792814
const auto memoryStates = StateMap::MemoryNodeStatePair::States(memoryNodeStatePairs);
793815

794816
const auto & newLoadNode = ReplaceLoadNode(node, memoryStates);
@@ -803,7 +825,7 @@ MemoryStateEncoder::EncodeStore(const rvsdg::SimpleNode & node, StateMap & state
803825
JLM_ASSERT(is<StoreOperation>(node.GetOperation()));
804826

805827
const auto & modRefSet = modRefSummary_->GetSimpleNodeModRef(node);
806-
const auto memoryNodeStatePairs = stateMap.GetExistingStates(modRefSet);
828+
const auto memoryNodeStatePairs = stateMap.GetOrCreateStates(modRefSet, *node.region());
807829
const auto memoryStates = StateMap::MemoryNodeStatePair::States(memoryNodeStatePairs);
808830

809831
const auto & newStoreNode = ReplaceStoreNode(node, memoryStates);
@@ -820,7 +842,7 @@ MemoryStateEncoder::EncodeFree(const rvsdg::SimpleNode & freeNode, StateMap & st
820842
auto addressOperand = FreeOperation::getAddressInput(freeNode).origin();
821843
auto ioStateOperand = FreeOperation::getIOStateInput(freeNode).origin();
822844
auto memoryNodeStatePairs =
823-
stateMap.GetExistingStates(modRefSummary_->GetSimpleNodeModRef(freeNode));
845+
stateMap.GetOrCreateStates(modRefSummary_->GetSimpleNodeModRef(freeNode), *freeNode.region());
824846
auto memStateOperands = StateMap::MemoryNodeStatePair::States(memoryNodeStatePairs);
825847

826848
auto & newFreeNode =
@@ -848,7 +870,7 @@ MemoryStateEncoder::EncodeCall(const rvsdg::SimpleNode & callNode, StateMap & st
848870
JLM_ASSERT(is<CallOperation>(callNode.GetOperation()));
849871

850872
const auto & modRefSet = modRefSummary_->GetSimpleNodeModRef(callNode);
851-
const auto statePairs = stateMap.GetExistingStates(modRefSet);
873+
const auto statePairs = stateMap.GetOrCreateStates(modRefSet, *callNode.region());
852874

853875
std::vector<rvsdg::Output *> inputStates;
854876
std::vector<MemoryNodeId> memoryNodeIds;
@@ -876,8 +898,9 @@ MemoryStateEncoder::EncodeMemcpy(const rvsdg::SimpleNode & memcpyNode, StateMap
876898
{
877899
JLM_ASSERT(is<MemCpyOperation>(memcpyNode.GetOperation()));
878900

879-
auto memoryNodeStatePairs =
880-
stateMap.GetExistingStates(modRefSummary_->GetSimpleNodeModRef(memcpyNode));
901+
auto memoryNodeStatePairs = stateMap.GetOrCreateStates(
902+
modRefSummary_->GetSimpleNodeModRef(memcpyNode),
903+
*memcpyNode.region());
881904
auto memoryStateOperands = StateMap::MemoryNodeStatePair::States(memoryNodeStatePairs);
882905

883906
const auto & newMemCpyNode = ReplaceMemcpyNode(memcpyNode, memoryStateOperands);
@@ -891,8 +914,9 @@ MemoryStateEncoder::EncodeMemset(const rvsdg::SimpleNode & memsetNode, StateMap
891914
{
892915
JLM_ASSERT(is<MemSetOperation>(memsetNode.GetOperation()));
893916

894-
auto memoryNodeStatePairs =
895-
stateMap.GetExistingStates(modRefSummary_->GetSimpleNodeModRef(memsetNode));
917+
auto memoryNodeStatePairs = stateMap.GetOrCreateStates(
918+
modRefSummary_->GetSimpleNodeModRef(memsetNode),
919+
*memsetNode.region());
896920
auto memoryStateOperands = StateMap::MemoryNodeStatePair::States(memoryNodeStatePairs);
897921

898922
auto & newMemSetNode = ReplaceMemsetNode(memsetNode, memoryStateOperands);
@@ -906,8 +930,9 @@ MemoryStateEncoder::EncodeMemmove(const rvsdg::SimpleNode & memmoveNode, StateMa
906930
{
907931
JLM_ASSERT(is<MemMoveOperation>(memmoveNode.GetOperation()));
908932

909-
auto memoryNodeStatePairs =
910-
stateMap.GetExistingStates(modRefSummary_->GetSimpleNodeModRef(memmoveNode));
933+
auto memoryNodeStatePairs = stateMap.GetOrCreateStates(
934+
modRefSummary_->GetSimpleNodeModRef(memmoveNode),
935+
*memmoveNode.region());
911936
auto memoryStateOperands = StateMap::MemoryNodeStatePair::States(memoryNodeStatePairs);
912937

913938
auto & newMemMoveNode = ReplaceMemmoveNode(memmoveNode, memoryStateOperands);
@@ -992,7 +1017,7 @@ MemoryStateEncoder::EncodeGamma(rvsdg::GammaNode & gammaNode, StateMap & stateMa
9921017
// Handle gamma entry
9931018
{
9941019
auto & modRefSet = modRefSummary_->GetGammaEntryModRef(gammaNode);
995-
auto memoryNodeStatePairs = stateMap.GetExistingStates(modRefSet);
1020+
auto memoryNodeStatePairs = stateMap.GetOrCreateStates(modRefSet, *gammaNode.region());
9961021
for (auto & memoryNodeStatePair : memoryNodeStatePairs)
9971022
{
9981023
auto gammaInput = gammaNode.AddEntryVar(&memoryNodeStatePair->State());
@@ -1009,7 +1034,7 @@ MemoryStateEncoder::EncodeGamma(rvsdg::GammaNode & gammaNode, StateMap & stateMa
10091034
// Handle gamma exit
10101035
{
10111036
auto & modRefSet = modRefSummary_->GetGammaExitModRef(gammaNode);
1012-
auto memoryNodeStatePairs = stateMap.GetExistingStates(modRefSet);
1037+
auto memoryNodeStatePairs = stateMap.GetOrCreateStates(modRefSet, *gammaNode.region());
10131038

10141039
for (auto & memoryNodeStatePair : memoryNodeStatePairs)
10151040
{
@@ -1038,7 +1063,7 @@ MemoryStateEncoder::EncodeTheta(rvsdg::ThetaNode & thetaNode, StateMap & stateMa
10381063
std::vector<rvsdg::Output *> thetaStateOutputs;
10391064
{
10401065
const auto & modRefSet = modRefSummary_->GetThetaModRef(thetaNode);
1041-
auto memoryNodeStatePairs = stateMap.GetExistingStates(modRefSet);
1066+
auto memoryNodeStatePairs = stateMap.GetOrCreateStates(modRefSet, *thetaNode.region());
10421067
for (auto & memoryNodeStatePair : memoryNodeStatePairs)
10431068
{
10441069
auto loopvar = thetaNode.AddLoopVar(&memoryNodeStatePair->State());
@@ -1052,7 +1077,7 @@ MemoryStateEncoder::EncodeTheta(rvsdg::ThetaNode & thetaNode, StateMap & stateMa
10521077
// Handle theta exit
10531078
{
10541079
const auto & memoryNodes = modRefSummary_->GetThetaModRef(thetaNode);
1055-
auto memoryNodeStatePairs = stateMap.GetExistingStates(memoryNodes);
1080+
auto memoryNodeStatePairs = stateMap.GetStates(memoryNodes);
10561081

10571082
JLM_ASSERT(memoryNodeStatePairs.size() == thetaStateOutputs.size());
10581083
for (size_t n = 0; n < thetaStateOutputs.size(); n++)

0 commit comments

Comments
 (0)