Skip to content

Commit

Permalink
Fix handling of points-to flags for memcpy in Steensgaard (#373)
Browse files Browse the repository at this point in the history 
1. Fixes the handling of flags for memcpy in Steensgaard. The method
ignored flags so far.
2. Added a unit test that exposes the problem
3. Some minor adjustments for debugging Steensgaard

Closes #334
  • Loading branch information
@phate
phate authored Feb 1, 2024
1 parent 8fd565f commit b4f02a4
Show file tree
Hide file tree
Showing 7 changed files with 196 additions and 52 deletions.
55 changes: 26 additions & 29 deletions jlm/llvm/opt/alias-analyses/Steensgaard.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ class LocationSet final
};

std::string str;
str.append("digraph PointsToGraph {\n");
str.append("digraph DisjointLocationSetGraph {\n");

for (auto & set : DisjointLocationSet_)
{
Expand Down Expand Up @@ -1379,45 +1379,39 @@ Steensgaard::AnalyzeMemcpy(const jlm::rvsdg::simple_node & node)
{
JLM_ASSERT(is<Memcpy>(&node));

// FIXME: handle unknown

// FIXME: write some documentation about the implementation

auto & dstAddress = LocationSet_->Find(*node.input(0)->origin());
auto & srcAddress = LocationSet_->Find(*node.input(1)->origin());

// We implement memcpy by pointing srcAddress and dstAddress to the same underlying memory:
//
// srcAddress -> underlyingMemory
// dstAddress -> underlyingMemory
//
// Preferably, I would have liked to implement it as follows:
//
// srcAddress -> srcMemory
// dstAddress -> dstMemory (which is a copy of srcMemory)
// srcMemory and dstMemory -> underlyingMemory
//
// However, this was not possible due to points-to flags propagation. We have no guarantee that
// srcAddress and dstAddress are annotated with the right flags BEFORE PropagatePointsToFlags()
// ran. In this scheme, dstMemory is a copy of srcMemory, which means that it should also get a
// copy of its flags (which again are the same as the points-to flags of srcAddress).
if (srcAddress.GetPointsTo() == nullptr)
{
// If we do not know where the source address points to yet(!),
// insert a dummy location such that we have something to work with.
auto & dummyLocation = LocationSet_->InsertDummyLocation();
srcAddress.SetPointsTo(dummyLocation);
auto & underlyingMemory = LocationSet_->InsertDummyLocation();
srcAddress.SetPointsTo(underlyingMemory);
}

if (dstAddress.GetPointsTo() == nullptr)
{
// If we do not know where the destination address points to yet(!),
// insert a dummy location such that we have something to work with.
auto & dummyLocation = LocationSet_->InsertDummyLocation();
dstAddress.SetPointsTo(dummyLocation);
}

auto & srcMemory = LocationSet_->GetRootLocation(*srcAddress.GetPointsTo());
auto & dstMemory = LocationSet_->GetRootLocation(*dstAddress.GetPointsTo());

if (srcMemory.GetPointsTo() == nullptr)
{
auto & dummyLocation = LocationSet_->InsertDummyLocation();
srcMemory.SetPointsTo(dummyLocation);
dstAddress.SetPointsTo(*srcAddress.GetPointsTo());
}

if (dstMemory.GetPointsTo() == nullptr)
else
{
auto & dummyLocation = LocationSet_->InsertDummyLocation();
dstMemory.SetPointsTo(dummyLocation);
// Unifies the underlying memory of srcMemory and dstMemory
LocationSet_->Join(*srcAddress.GetPointsTo(), *dstAddress.GetPointsTo());
}

LocationSet_->Join(*srcMemory.GetPointsTo(), *dstMemory.GetPointsTo());
}

void
Expand Down Expand Up @@ -1721,6 +1715,9 @@ Steensgaard::Analyze(
const RvsdgModule & module,
jlm::util::StatisticsCollector & statisticsCollector)
{
// std::unordered_map<const rvsdg::output *, std::string> outputMap;
// std::cout << jlm::rvsdg::view(module.Rvsdg().root(), outputMap) << std::flush;

LocationSet_ = LocationSet::Create();
auto statistics = Statistics::Create(module.SourceFileName());

Expand All @@ -1738,7 +1735,7 @@ Steensgaard::Analyze(
// Construct PointsTo graph
statistics->StartPointsToGraphConstructionStatistics(*LocationSet_);
auto pointsToGraph = ConstructPointsToGraph();
// std::cout << PointsToGraph::ToDot(*pointsToGraph) << std::flush;
// std::cout << PointsToGraph::ToDot(*pointsToGraph, outputMap) << std::flush;
statistics->StopPointsToGraphConstructionStatistics(*pointsToGraph);

// Redirect unknown memory node sources
Expand Down
59 changes: 59 additions & 0 deletions tests/TestRvsdgs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3127,6 +3127,65 @@ MemcpyTest2::SetupRvsdg()
return rvsdgModule;
}

std::unique_ptr<jlm::llvm::RvsdgModule>
MemcpyTest3::SetupRvsdg()
{
using namespace jlm::llvm;

auto rvsdgModule = RvsdgModule::Create(jlm::util::filepath(""), "", "");
auto rvsdg = &rvsdgModule->Rvsdg();

auto nf = rvsdg->node_normal_form(typeid(jlm::rvsdg::operation));
nf->set_mutable(false);

PointerType pointerType;
auto & declaration =
rvsdgModule->AddStructTypeDeclaration(StructType::Declaration::Create({ &pointerType }));
auto structType = StructType::Create("myStruct", false, declaration);

iostatetype iOStateType;
MemoryStateType memoryStateType;
loopstatetype loopStateType;
FunctionType functionType(
{ &pointerType, &iOStateType, &memoryStateType, &loopStateType },
{ &iOStateType, &memoryStateType, &loopStateType });

Lambda_ = lambda::node::create(rvsdg->root(), functionType, "f", linkage::internal_linkage);
auto pArgument = Lambda_->fctargument(0);
auto iOStateArgument = Lambda_->fctargument(1);
auto memoryStateArgument = Lambda_->fctargument(2);
auto loopStateArgument = Lambda_->fctargument(3);

auto cFalse = jlm::rvsdg::create_bitconstant(Lambda_->subregion(), 1, 0);
auto eight = jlm::rvsdg::create_bitconstant(Lambda_->subregion(), 64, 8);
auto zero = jlm::rvsdg::create_bitconstant(Lambda_->subregion(), 32, 0);
auto minusFive = jlm::rvsdg::create_bitconstant(Lambda_->subregion(), 64, -5);
auto three = jlm::rvsdg::create_bitconstant(Lambda_->subregion(), 64, 3);

auto allocaResults = alloca_op::create(*structType, eight, 8);
auto memoryState = MemStateMergeOperator::Create({ allocaResults[1], memoryStateArgument });

auto memcpyResults = Memcpy::create(allocaResults[0], pArgument, eight, cFalse, { memoryState });

auto gep1 =
GetElementPtrOperation::Create(allocaResults[0], { zero, zero }, *structType, pointerType);
auto ld = LoadNode::Create(gep1, { memcpyResults[0] }, pointerType, 8);

auto gep2 =
GetElementPtrOperation::Create(allocaResults[0], { minusFive }, *structType, pointerType);

memcpyResults = Memcpy::create(ld[0], gep2, three, cFalse, { ld[1] });

auto lambdaOutput = Lambda_->finalize({ iOStateArgument, memcpyResults[0], loopStateArgument });

rvsdg->add_export(lambdaOutput, { PointerType(), "f" });

Alloca_ = rvsdg::node_output::node(allocaResults[0]);
Memcpy_ = rvsdg::node_output::node(memcpyResults[0]);

return rvsdgModule;
}

std::unique_ptr<jlm::llvm::RvsdgModule>
LinkedListTest::SetupRvsdg()
{
Expand Down
54 changes: 54 additions & 0 deletions tests/TestRvsdgs.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1868,6 +1868,60 @@ class MemcpyTest2 final : public RvsdgTest
jlm::rvsdg::node * Memcpy_ = {};
};

/**
* This class sets up an RVSDG representing the following code snippet:
*
* \code{.c}
* #include <stdint.h>
* #include <string.h>
*
* typedef struct {
* uint8_t * buf;
* } myStruct;
*
* void
* f(myStruct * p)
* {
* myStruct s = *p;
* memcpy(s.buf, s.buf - 5, 3);
* }
* \endcode
*/
class MemcpyTest3 final : public RvsdgTest
{
public:
[[nodiscard]] const jlm::llvm::lambda::node &
Lambda() const noexcept
{
JLM_ASSERT(Lambda_ != nullptr);
return *Lambda_;
}

[[nodiscard]] const jlm::rvsdg::node &
Alloca() const noexcept
{
JLM_ASSERT(Alloca_ != nullptr);
return *Alloca_;
}

[[nodiscard]] const jlm::rvsdg::node &
Memcpy() const noexcept
{
JLM_ASSERT(Memcpy_ != nullptr);
return *Memcpy_;
}

private:
std::unique_ptr<jlm::llvm::RvsdgModule>
SetupRvsdg() override;

jlm::llvm::lambda::node * Lambda_ = {};

jlm::rvsdg::node * Alloca_ = {};

jlm::rvsdg::node * Memcpy_ = {};
};

/** \brief LinkedListTest class
*
* This class sets up an RVSDG representing the following code snippet:
Expand Down
4 changes: 2 additions & 2 deletions tests/jlm/llvm/opt/alias-analyses/TestAgnosticMemoryNodeProvider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -797,8 +797,8 @@ TestMemcpy()
assert(numLambdaExitNodes == pointsToGraph.NumMemoryNodes());
assert(numCallFEntryNodes == pointsToGraph.NumMemoryNodes());
assert(numCallFExitNodes == pointsToGraph.NumMemoryNodes());
assert(numMemcpyDestNodes == 1);
assert(numMemcpySrcNodes == 1);
assert(numMemcpyDestNodes == 2);
assert(numMemcpySrcNodes == 2);
}
};

Expand Down
23 changes: 10 additions & 13 deletions tests/jlm/llvm/opt/alias-analyses/TestMemoryStateEncoder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1102,10 +1102,10 @@ ValidateMemcpySteensgaardAgnostic(const jlm::tests::MemcpyTest & test)
assert(is<aa::LambdaExitMemStateOperator>(*lambdaExitMerge, 5, 1));

auto load = jlm::rvsdg::node_output::node(test.LambdaF().fctresult(0)->origin());
assert(is<LoadOperation>(*load, 2, 2));
assert(is<LoadOperation>(*load, 3, 3));

auto store = jlm::rvsdg::node_output::node(load->input(1)->origin());
assert(is<StoreOperation>(*store, 3, 1));
assert(is<StoreOperation>(*store, 4, 2));

auto lambdaEntrySplit = jlm::rvsdg::node_output::node(store->input(2)->origin());
assert(is<aa::LambdaEntryMemStateOperator>(*lambdaEntrySplit, 1, 5));
Expand Down Expand Up @@ -1135,7 +1135,7 @@ ValidateMemcpySteensgaardAgnostic(const jlm::tests::MemcpyTest & test)
memcpy = node;
}
assert(memcpy != nullptr);
assert(is<Memcpy>(*memcpy, 6, 2));
assert(is<Memcpy>(*memcpy, 8, 4));

auto lambdaEntrySplit = jlm::rvsdg::node_output::node(memcpy->input(5)->origin());
assert(is<aa::LambdaEntryMemStateOperator>(*lambdaEntrySplit, 1, 5));
Expand All @@ -1152,16 +1152,16 @@ ValidateMemcpySteensgaardRegionAware(const jlm::tests::MemcpyTest & test)
*/
{
auto lambdaExitMerge = jlm::rvsdg::node_output::node(test.LambdaF().fctresult(2)->origin());
assert(is<aa::LambdaExitMemStateOperator>(*lambdaExitMerge, 1, 1));
assert(is<aa::LambdaExitMemStateOperator>(*lambdaExitMerge, 2, 1));

auto load = jlm::rvsdg::node_output::node(test.LambdaF().fctresult(0)->origin());
assert(is<LoadOperation>(*load, 2, 2));
assert(is<LoadOperation>(*load, 3, 3));

auto store = jlm::rvsdg::node_output::node(load->input(1)->origin());
assert(is<StoreOperation>(*store, 3, 1));
assert(is<StoreOperation>(*store, 4, 2));

auto lambdaEntrySplit = jlm::rvsdg::node_output::node(store->input(2)->origin());
assert(is<aa::LambdaEntryMemStateOperator>(*lambdaEntrySplit, 1, 1));
assert(is<aa::LambdaEntryMemStateOperator>(*lambdaEntrySplit, 1, 2));
}

/*
Expand All @@ -1172,23 +1172,20 @@ ValidateMemcpySteensgaardRegionAware(const jlm::tests::MemcpyTest & test)
assert(is<CallOperation>(*callNode, 4, 4));

auto callEntryMerge = jlm::rvsdg::node_output::node(callNode->input(2)->origin());
assert(is<aa::CallEntryMemStateOperator>(*callEntryMerge, 1, 1));
assert(is<aa::CallEntryMemStateOperator>(*callEntryMerge, 2, 1));

auto callExitSplit = input_node(*callNode->output(2)->begin());
assert(is<aa::CallExitMemStateOperator>(*callExitSplit, 1, 1));
assert(is<aa::CallExitMemStateOperator>(*callExitSplit, 1, 2));

auto memcpyNode = jlm::rvsdg::node_output::node(callEntryMerge->input(0)->origin());
assert(is<Memcpy>(*memcpyNode, 6, 2));
assert(is<Memcpy>(*memcpyNode, 8, 4));

auto lambdaEntrySplit = jlm::rvsdg::node_output::node(memcpyNode->input(4)->origin());
assert(is<aa::LambdaEntryMemStateOperator>(*lambdaEntrySplit, 1, 2));
assert(jlm::rvsdg::node_output::node(memcpyNode->input(5)->origin()) == lambdaEntrySplit);

auto lambdaExitMerge = input_node(*callExitSplit->output(0)->begin());
assert(is<aa::LambdaExitMemStateOperator>(*lambdaExitMerge, 2, 1));
assert(
jlm::rvsdg::node_output::node(lambdaExitMerge->input(0)->origin()) == memcpyNode
|| jlm::rvsdg::node_output::node(lambdaExitMerge->input(1)->origin()) == memcpyNode);
}
}

Expand Down
8 changes: 4 additions & 4 deletions tests/jlm/llvm/opt/alias-analyses/TestRegionAwareMemoryNodeProvider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1179,10 +1179,10 @@ TestMemcpy()
*/
{
auto & lambdaEntryNodes = provisioning.GetLambdaEntryNodes(test.LambdaF());
AssertMemoryNodes(lambdaEntryNodes, { &globalArrayMemoryNode });
AssertMemoryNodes(lambdaEntryNodes, { &globalArrayMemoryNode, &localArrayMemoryNode });

auto & lambdaExitNodes = provisioning.GetLambdaExitNodes(test.LambdaF());
AssertMemoryNodes(lambdaExitNodes, { &globalArrayMemoryNode });
AssertMemoryNodes(lambdaExitNodes, { &globalArrayMemoryNode, &localArrayMemoryNode });
}

/*
Expand All @@ -1193,10 +1193,10 @@ TestMemcpy()
AssertMemoryNodes(lambdaEntryNodes, { &localArrayMemoryNode, &globalArrayMemoryNode });

auto & callEntryNodes = provisioning.GetCallEntryNodes(test.CallF());
AssertMemoryNodes(callEntryNodes, { &globalArrayMemoryNode });
AssertMemoryNodes(callEntryNodes, { &globalArrayMemoryNode, &localArrayMemoryNode });

auto & callExitNodes = provisioning.GetCallExitNodes(test.CallF());
AssertMemoryNodes(callExitNodes, { &globalArrayMemoryNode });
AssertMemoryNodes(callExitNodes, { &globalArrayMemoryNode, &localArrayMemoryNode });

auto & lambdaExitNodes = provisioning.GetLambdaExitNodes(test.LambdaG());
AssertMemoryNodes(lambdaExitNodes, { &localArrayMemoryNode, &globalArrayMemoryNode });
Expand Down
Loading

0 comments on commit b4f02a4

Please sign in to comment.