-
Notifications
You must be signed in to change notification settings - Fork 18
Region Sequentializer #839
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,205 @@ | ||
| /* | ||
| * Copyright 2025 Nico Reißmann <nico.reissmann@gmail.com> | ||
| * See COPYING for terms of redistribution. | ||
| */ | ||
|
|
||
| #include <jlm/llvm/backend/RegionSequentializer.hpp> | ||
| #include <jlm/rvsdg/region.hpp> | ||
| #include <jlm/rvsdg/traverser.hpp> | ||
| #include <jlm/util/HashSet.hpp> | ||
|
|
||
| namespace jlm::llvm | ||
| { | ||
|
|
||
| RegionSequentializer::~RegionSequentializer() noexcept = default; | ||
|
|
||
| RegionSequentializer::RegionSequentializer(rvsdg::Region & region) | ||
| : Region_(®ion) | ||
| {} | ||
|
|
||
| ExhaustiveRegionSequentializer::ExhaustiveRegionSequentializer(rvsdg::Region & region) | ||
| : RegionSequentializer(region) | ||
| { | ||
| util::HashSet<const rvsdg::Node *> visited; | ||
| std::vector<const rvsdg::Node *> sequentializedNodes; | ||
| ComputeSequentializations(region, visited, sequentializedNodes); | ||
| } | ||
|
|
||
| void | ||
| ExhaustiveRegionSequentializer::ComputeNextSequentialization() | ||
| { | ||
| if (HasMoreSequentializations()) | ||
| CurrentSequentialization_++; | ||
| else | ||
| CurrentSequentialization_ = 0; | ||
| } | ||
|
|
||
| Sequentialization | ||
| ExhaustiveRegionSequentializer::GetSequentialization() | ||
| { | ||
| if (!HasMoreSequentializations()) | ||
| ComputeNextSequentialization(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not understand this code. Why can it not just return |
||
|
|
||
| return Sequentializations_[CurrentSequentialization_]; | ||
| } | ||
|
|
||
| bool | ||
| ExhaustiveRegionSequentializer::HasMoreSequentializations() const noexcept | ||
| { | ||
| JLM_ASSERT(CurrentSequentialization_ <= Sequentializations_.size()); | ||
| return CurrentSequentialization_ != Sequentializations_.size(); | ||
| } | ||
|
|
||
| void | ||
| ExhaustiveRegionSequentializer::ComputeSequentializations( | ||
| const rvsdg::Region & region, | ||
| util::HashSet<const rvsdg::Node *> & visited, | ||
| std::vector<const rvsdg::Node *> & sequentializedNodes) noexcept | ||
| { | ||
| for (auto & node : region.Nodes()) | ||
| { | ||
| if (AllPredecessorsVisited(node, visited) && !visited.Contains(&node)) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would swap the order of the conditions, to skip visited nodes more quickly |
||
| { | ||
| sequentializedNodes.push_back(&node); | ||
| visited.Insert(&node); | ||
|
|
||
| ComputeSequentializations(region, visited, sequentializedNodes); | ||
|
|
||
| visited.Remove(&node); | ||
| sequentializedNodes.pop_back(); | ||
| } | ||
| } | ||
|
|
||
| if (sequentializedNodes.size() == region.nnodes()) | ||
| { | ||
| Sequentializations_.emplace_back(sequentializedNodes); | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This if should not be inside the loop, no? |
||
| } | ||
|
|
||
| bool | ||
| ExhaustiveRegionSequentializer::AllPredecessorsVisited( | ||
| const rvsdg::Node & node, | ||
| const util::HashSet<const rvsdg::Node *> & visited) | ||
| { | ||
| for (size_t n = 0; n < node.ninputs(); n++) | ||
| { | ||
| auto & origin = *node.input(n)->origin(); | ||
| if (const auto predecessor = rvsdg::TryGetOwnerNode<rvsdg::Node>(origin)) | ||
| { | ||
| if (!visited.Contains(predecessor)) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| IdempotentRegionSequentializer::~IdempotentRegionSequentializer() noexcept = default; | ||
|
|
||
| IdempotentRegionSequentializer::IdempotentRegionSequentializer(rvsdg::Region & region) | ||
| : RegionSequentializer(region) | ||
| {} | ||
|
|
||
| void | ||
| IdempotentRegionSequentializer::ComputeNextSequentialization() | ||
| {} | ||
|
|
||
| Sequentialization | ||
| IdempotentRegionSequentializer::GetSequentialization() | ||
| { | ||
| auto sequentialization = Sequentialization(); | ||
| for (const auto node : rvsdg::TopDownTraverser(&GetRegion())) | ||
| { | ||
| sequentialization.push_back(node); | ||
| } | ||
| return sequentialization; | ||
| } | ||
|
|
||
| bool | ||
| IdempotentRegionSequentializer::HasMoreSequentializations() const noexcept | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| RegionTreeSequentializer::~RegionTreeSequentializer() noexcept = default; | ||
|
|
||
| RegionTreeSequentializer::RegionTreeSequentializer(SequentializerMap sequentializerMap) | ||
| : Sequentializers_(std::move(sequentializerMap)) | ||
| {} | ||
|
|
||
| void | ||
| RegionTreeSequentializer::ComputeNextSequentializations() | ||
| { | ||
| for (auto & [region, sequentializer] : Sequentializers_) | ||
| { | ||
| sequentializer->ComputeNextSequentialization(); | ||
| } | ||
| } | ||
|
|
||
| bool | ||
| RegionTreeSequentializer::HasMoreSequentializations() const noexcept | ||
| { | ||
| for (auto & [_, sequentializer] : Sequentializers_) | ||
| { | ||
| if (sequentializer->HasMoreSequentializations()) | ||
| return true; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Due to the looping behavior of the exhaustive sequentializer, I would be a bit worried about the |
||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| Sequentialization | ||
| RegionTreeSequentializer::GetSequentialization(rvsdg::Region & region) | ||
| { | ||
| JLM_ASSERT(Sequentializers_.find(®ion) != Sequentializers_.end()); | ||
| return Sequentializers_[®ion]->GetSequentialization(); | ||
| } | ||
|
|
||
| template<class TRegionSequentializer> | ||
| void | ||
| InitializeUniformRegionTreeSequentializer( | ||
| rvsdg::Region & region, | ||
| SequentializerMap & sequentializerMap) | ||
| { | ||
| auto sequentializer = std::make_unique<TRegionSequentializer>(region); | ||
|
|
||
| for (auto & node : region.Nodes()) | ||
| { | ||
| if (const auto structuralNode = dynamic_cast<const rvsdg::StructuralNode *>(&node)) | ||
| { | ||
| for (size_t n = 0; n < structuralNode->nsubregions(); n++) | ||
| { | ||
| const auto subregion = structuralNode->subregion(n); | ||
| InitializeUniformRegionTreeSequentializer<TRegionSequentializer>( | ||
| *subregion, | ||
| sequentializerMap); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| sequentializerMap[®ion] = std::move(sequentializer); | ||
| } | ||
|
|
||
| RegionTreeSequentializer | ||
| CreateIdempotentRegionTreeSequentializer(rvsdg::Region & rootRegion) | ||
| { | ||
| SequentializerMap sequentializerMap; | ||
| InitializeUniformRegionTreeSequentializer<IdempotentRegionSequentializer>( | ||
| rootRegion, | ||
| sequentializerMap); | ||
| return RegionTreeSequentializer(std::move(sequentializerMap)); | ||
| } | ||
|
|
||
| RegionTreeSequentializer | ||
| CreateExhaustiveRegionTreeSequentializer(rvsdg::Region & rootRegion) | ||
| { | ||
| SequentializerMap sequentializerMap; | ||
| InitializeUniformRegionTreeSequentializer<ExhaustiveRegionSequentializer>( | ||
| rootRegion, | ||
| sequentializerMap); | ||
| return RegionTreeSequentializer(std::move(sequentializerMap)); | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looping behavior is a bit surprising. It sort of allows overflow, but then automatically fixes the overflow again once you actually call
GetSequentialization().The way it is currently, you have to query
HasMoreSequentialization()afterComputeNextSequentialization(), to find out that it actually didn't have a "next sequentialization".I feel like that naming is a bit backwards, as I would expect to call
HasMoreSequentializations()before callingComputeNextSequentialization().If would consider remaning
HasMoreSequentializations()to justHasSequentialization().