Skip to content

Commit bafb3e2

Browse files
angelomatni1copybara-github
authored andcommitted
Add cycle detection to ResourceSharingPass::PerformFoldingActions.
This change modifies the PerformFoldingActions function to check that the introduction of priority_sel input selection would not introduce a cycle when the folded nodes are merged together. This is to ensure the pass does not produce an unrecoverable error that would otherwise occur when the next pass attempts to iterate nodes using DfsVisitor. This does not yet solve the underlying bug in folding action legalization or analysis that exists but surfaces that error in a more usable way. PiperOrigin-RevId: 895433062
1 parent a672865 commit bafb3e2

File tree

3 files changed

+28
-2
lines changed

3 files changed

+28
-2
lines changed

xls/passes/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,6 +1113,7 @@ xls_pass(
11131113
"@com_google_absl//absl/random",
11141114
"@com_google_absl//absl/status",
11151115
"@com_google_absl//absl/status:statusor",
1116+
"@com_google_absl//absl/strings:str_format",
11161117
"@com_google_absl//absl/types:span",
11171118
"@cppitertools",
11181119
],

xls/passes/resource_sharing_pass.cc

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "absl/random/random.h"
3838
#include "absl/status/status.h"
3939
#include "absl/status/statusor.h"
40+
#include "absl/strings/str_format.h"
4041
#include "absl/types/span.h"
4142
#include "cppitertools/combinations.hpp"
4243
#include "cppitertools/zip.hpp"
@@ -1696,6 +1697,7 @@ SelectFoldingActions(OptimizationContext& context, FoldingGraph* folding_graph,
16961697
absl::StatusOr<bool> ResourceSharingPass::PerformFoldingActions(
16971698
FunctionBase* f, int64_t next_node_id,
16981699
VisibilityBuilder* visibility_builder,
1700+
const NodeBackwardDependencyAnalysis& nda,
16991701
const std::vector<std::unique_ptr<NaryFoldingAction>>&
17001702
folding_actions_to_perform) {
17011703
bool modified = !folding_actions_to_perform.empty();
@@ -1913,6 +1915,26 @@ absl::StatusOr<bool> ResourceSharingPass::PerformFoldingActions(
19131915
}
19141916
XLS_RET_CHECK_EQ(new_operands.size(), 2);
19151917

1918+
// Ensure the priority selects do not depend on any of the folded nodes.
1919+
// This is to avoid creating a cycle and producing an unrecoverable error.
1920+
for (Node* new_operand : new_operands) {
1921+
auto make_error = [&](Node* folded_node, Node* select) {
1922+
return absl::InternalError(absl::StrFormat(
1923+
"Folding action input selection would create a cycle with node "
1924+
"because it depends on it: %s -> ... -> %s; there is a bug in "
1925+
"the legalization of folding actions or in visibility analysis",
1926+
folded_node->ToString(), select->ToString()));
1927+
};
1928+
for (const auto& [from_node, _] : froms_to_use) {
1929+
if (nda.IsDependent(from_node, new_operand)) {
1930+
return make_error(from_node, new_operand);
1931+
}
1932+
}
1933+
if (nda.IsDependent(to_node, new_operand)) {
1934+
return make_error(to_node, new_operand);
1935+
}
1936+
}
1937+
19161938
// - Step 2: Replace the operands of the @to_node to use the results of the
19171939
// new selectors computed at Step 1.
19181940
VLOG(3) << " Step 2: update the target of the folding transformation";
@@ -2064,8 +2086,9 @@ absl::StatusOr<bool> ResourceSharingPass::RunOnFunctionBaseInternal(
20642086

20652087
// Perform the folding
20662088
XLS_ASSIGN_OR_RETURN(
2067-
modified, PerformFoldingActions(f, next_node_id, &visibility_estimator,
2068-
folding_actions_to_perform));
2089+
modified,
2090+
PerformFoldingActions(f, next_node_id, &visibility_estimator,
2091+
nda_backwards, folding_actions_to_perform));
20692092

20702093
return modified;
20712094
}

xls/passes/resource_sharing_pass.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "xls/ir/node.h"
3333
#include "xls/ir/node_util.h"
3434
#include "xls/passes/folding_graph.h"
35+
#include "xls/passes/node_dependency_analysis.h"
3536
#include "xls/passes/optimization_pass.h"
3637
#include "xls/passes/pass_base.h"
3738
#include "xls/passes/visibility_analysis.h"
@@ -307,6 +308,7 @@ class ResourceSharingPass : public OptimizationFunctionBasePass {
307308
static absl::StatusOr<bool> PerformFoldingActions(
308309
FunctionBase* f, int64_t next_node_id,
309310
VisibilityBuilder* visibility_builder,
311+
const NodeBackwardDependencyAnalysis& nda,
310312
const std::vector<std::unique_ptr<NaryFoldingAction>>&
311313
folding_actions_to_perform);
312314

0 commit comments

Comments
 (0)