Remove invariant state edges between Lambda[Entry|Exit]MemoryState nodes #785
Conversation
sjalander
force-pushed
the
invariant-state
branch
from
cb93b88 to
d317a25
Compare
|
| // We only apply this for memory state edges that is invariant between | ||
| // LambdaEntryMemoryStateSplit and LambdaExitMemoryStateMerge nodes. | ||
| // So we first check if we have a LambdaExitMemoryStateMerge node. | ||
| if (memoryState->origin()->nusers() == 1) |
There was a problem hiding this comment.
This if could be an early return, which would help avoid the deep nesting below
There was a problem hiding this comment.
The second check is enough, so this one has been removed.
| if (memoryState->origin()->nusers() == 1) | ||
| { | ||
| auto exitNode = rvsdg::output::GetNode(*memoryState->origin()); | ||
| if (rvsdg::is<const llvm::LambdaExitMemoryStateMergeOperation>(exitNode->GetOperation())) |
There was a problem hiding this comment.
This too could be an early return
There was a problem hiding this comment.
Changed to an early return.
jlm/hls/backend/rvsdg2rhls/InvariantLambdaMemoryStateRemoval.cpp
Outdated
Show resolved
Hide resolved
jlm/hls/backend/rvsdg2rhls/InvariantLambdaMemoryStateRemoval.cpp
Outdated
Show resolved
Hide resolved
| * @brief Remove invariant memory state edges between Lambda[Entry/Exit]MemoryState nodes. | ||
| * | ||
| * The pass checks all lambdas in the module for invariant memory state edges between a | ||
| * LambdaEntreyMemoryStateSplit and LambdaExitMemoryStateMerge node and removes them. The memory |
|
|
sjalander
changed the title
|
jlm/hls/backend/rvsdg2rhls/InvariantLambdaMemoryStateRemoval.hpp
Outdated
Show resolved
Hide resolved
|
@phate I see what you mean about being a part of invariant value redirection. One thing to note there is that IVR only performs redirections, and does not remove dead state edges. This means IVR can remove invariant states from the |
|
@haved @sjalander : Let's hold back on this one until after the meeting tomorrow. I thought a little bit about it last night. There is more to it than immediately meets the eye. It is better to have a whiteboard available. |
|
| #ifndef JLM_HLS_OPT_INVARIANTLAMBDAMEMORYSTATEREMOVAL_HPP | ||
| #define JLM_HLS_OPT_INVARIANTLAMBDAMEMORYSTATEREMOVAL_HPP | ||
|
|
||
| #include <jlm/rvsdg/region.hpp> |
There was a problem hiding this comment.
I think the only reason why you need this header is because of rvsdg::RegionResult, but that can be forward declared and the header is not necessary.
| * @brief Remove invariant memory state edges between Lambda[Entry/Exit]MemoryState nodes. | ||
| * | ||
| * The pass applies RemoveInvariantMemoryStateEdges(rvsdg::RegionResult * memoryState) to all | ||
| * memory states of all lambdas in the module. |
There was a problem hiding this comment.
of all lambdas in the module that are only exported.
| * @param rvsdgModule The RVSDG moduled for which invariant memory state edges in all lambda nodes | ||
| * are to be removed. |
There was a problem hiding this comment.
for which invariant memory state edges are removed in lambdas.
| } | ||
| }; | ||
|
|
||
| /* InvariantLambdaMemoryStateRemoval class */ |
| InvariantLambdaMemoryStateRemoval::~InvariantLambdaMemoryStateRemoval() | ||
| {} |
| // The memory state edge is invariant, so we could in principle remove it from the lambda | ||
| // But the LLVM dialect expects to always have a memory state, so we connect the argument | ||
| // directly to the result | ||
| memoryState->divert_to(entryNode->input(0)->origin()); |
There was a problem hiding this comment.
You seem to assume here that you only have a single non-invariant state edge. What about if all state edges are non-invariant?
| else if (outputs.size() == 1) | ||
| { | ||
| // Single edge that is not invariant, so we can elmintate the two MemoryState nodes | ||
| memoryState->divert_to(outputs.front()); |
There was a problem hiding this comment.
outputs[0] is a bit more readable.
| { | ||
| // Single edge that is not invariant, so we can elmintate the two MemoryState nodes | ||
| memoryState->divert_to(outputs.front()); | ||
| entryNode->output(indexes.front())->divert_users(entryNode->input(0)->origin()); |
There was a problem hiding this comment.
This line should not be necessary, I believe. After the line above, the exit node and entry node should be dead.
|
|
||
| void | ||
| InvariantLambdaMemoryStateRemoval::RemoveInvariantMemoryStateEdges( | ||
| rvsdg::RegionResult * memoryState) |
There was a problem hiding this comment.
Maybe you can call it memoryStateResult so one does know what exactly it is when reading the code.
| int i = 0; | ||
| for (auto index : indexes) | ||
| { | ||
| entryNode->output(index)->divert_users(newEntryNodeOutputs.at(i)); |
If the pass results in a single noninvariant state edge, then the Lambda[Exit|Entry]MemoryState nodes are removed.