Skip to content

Enable visibility expression builder usage while filtering BFS on def-use DAG#4506

Draft
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_941218304
Draft

Enable visibility expression builder usage while filtering BFS on def-use DAG#4506
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_941218304

Conversation

@copybara-service

Copy link
Copy Markdown

Enable visibility expression builder usage while filtering BFS on def-use DAG

This change ensures BuildVisibilityIRExpr always returns a valid visibility expression regardless of conditional_edges provided. If none are provided, then the function would return always_visible. If every edge in the def-use DAG from node down to terminal nodes is provided, then the expression would be exact, assuming no BDD overflow, and conservative otherwise. Providing only a subset of the entire DAG would produce a smaller and conservative expression, perhaps less conservative if that subset is chosen wisely.

BuildVisibilityIRExpr had a bug where the short-circuit path when you provided a single-element conditional_edges set assumed that that single edge was a sufficient visibility expression to account for every path through the DAG. This assumption was made because resource sharing was using BuildVisibilityIRExpr in this way if it could prove mutual exclusivity via SingleSelectVisibilityAnalysis.

For more context:

  • BuildVisibilityIRExprFromEdges computes visibility as OR(vis_node_through_user_i AND vis_user_i) where if the edge (node->user) is NOT in conditional_edges we assume vis_node_through_user_i is always true. This function is conservative. Using conditional_edgescan help prevent BDD overflow.
  • The test helper BuildDefaultVisibilityExpr uses GetEdgesForMutuallyExclusiveVisibilityExpr to determine a subset of conditional_edges that helps BuildVisibilityIRExprFromEdgesnot overflow.

…-use DAG

This change ensures `BuildVisibilityIRExpr` always returns a valid visibility expression regardless of `conditional_edges` provided. If none are provided, then the function would return always_visible. If every edge in the def-use DAG from `node` down to terminal nodes is provided, then the expression would be exact, assuming no BDD overflow, and conservative otherwise. Providing only a subset of the entire DAG would produce a smaller and conservative expression, perhaps less conservative if that subset is chosen wisely.

BuildVisibilityIRExpr had a bug where the short-circuit path when you provided a single-element conditional_edges set assumed that that single edge was a sufficient visibility expression to account for every path through the DAG. This assumption was made because resource sharing was using BuildVisibilityIRExpr in this way if it could prove mutual exclusivity via SingleSelectVisibilityAnalysis.

For more context:

* BuildVisibilityIRExprFromEdges computes visibility as OR(vis_node_through_user_i AND vis_user_i) where if the edge (node->user) is NOT in conditional_edges we assume vis_node_through_user_i is always true. This function is conservative. Using conditional_edgescan help prevent BDD overflow.
* The test helper BuildDefaultVisibilityExpr uses GetEdgesForMutuallyExclusiveVisibilityExpr to determine a subset of conditional_edges that helps BuildVisibilityIRExprFromEdgesnot overflow.

PiperOrigin-RevId: 941218304
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant