[mlir][dataflow] Allow re-run all analyses in DataFlowSolver #120881
Conversation
|
@llvm/pr-subscribers-mlir Author: Hongren Zheng (ZenithalHourlyRate) ChangesIn downstream (check google/heir#1228, especially this commit; also check google/heir#1154) we often need to re-run the analysis during the transformation pass as IR get changed based on the analysis result and analysis continuously get invalidated. There are solutions to it like Just call To correctly re-run the analysis, either a new Cc @j2kun for downstream. Cc @Mogball for review. Full diff: https://github.com/llvm/llvm-project/pull/120881.diff 1 Files Affected:
diff --git a/mlir/include/mlir/Analysis/DataFlowFramework.h b/mlir/include/mlir/Analysis/DataFlowFramework.h
index 969664dc7a4fe3..dfd358e7017a4e 100644
--- a/mlir/include/mlir/Analysis/DataFlowFramework.h
+++ b/mlir/include/mlir/Analysis/DataFlowFramework.h
@@ -308,6 +308,10 @@ class DataFlowConfig {
/// according to their dependency relations until a fixed point is reached.
/// 3. Query analysis state results from the solver.
///
+/// Steps to re-run a data-flow analysis when IR changes:
+/// 1. Erase all analysis states as they are no longer valid.
+/// 2. Re-run the analysis using `initializeAndRun`.
+///
/// TODO: Optimize the internal implementation of the solver.
class DataFlowSolver {
public:
@@ -346,6 +350,9 @@ class DataFlowSolver {
}
}
+ // Erase all analysis states
+ void eraseAllStates() { analysisStates.clear(); }
+
/// Get a uniqued lattice anchor instance. If one is not present, it is
/// created with the provided arguments.
template <typename AnchorT, typename... Args>
|
…lowSolver (#120885) Detailed writeup is in google/heir#1153. See also #120881. In short, `propagateIfChanged` is used outside of the `DataFlowAnalysis` scope, because it is public, but it does not propagate as expected as the `DataFlowSolver` has stopped running. To solve such misuse, `propagateIfChanged` should be made protected/private. For downstream users affected by this, to correctly propagate the change, the Analysis should be re-run (check #120881) instead of just a `propagateIfChanged` The change to `IntegerRangeAnalysis` is just a expansion of the `solver->propagateIfChanged`. The `Lattice` has already been updated by the `join`. Propagation is done by `onUpdate`. Cc @Mogball for review
…d for DataFlowSolver (#120885) Detailed writeup is in google/heir#1153. See also llvm/llvm-project#120881. In short, `propagateIfChanged` is used outside of the `DataFlowAnalysis` scope, because it is public, but it does not propagate as expected as the `DataFlowSolver` has stopped running. To solve such misuse, `propagateIfChanged` should be made protected/private. For downstream users affected by this, to correctly propagate the change, the Analysis should be re-run (check #120881) instead of just a `propagateIfChanged` The change to `IntegerRangeAnalysis` is just a expansion of the `solver->propagateIfChanged`. The `Lattice` has already been updated by the `join`. Propagation is done by `onUpdate`. Cc @Mogball for review
In downstream (check google/heir#1228, especially this commit; also check google/heir#1154) we often need to re-run the analysis during the transformation pass as IR get changed based on the analysis result and analysis continuously get invalidated.
There are solutions to it like
getOrCreateStatefor newly createdValue(AnchorT), but warning is that the new state does not propagate! This is quite unexpected as user of analysis would expect it to propagate. We downstream used to usesolver->propagateIfChangedbut that turned out to be not working, see detailed writeup in google/heir#1153.Just call
initializeAndRunrepeatedly also does not solve the problem asanalysisStatesis not cleared and the monotonicity ofAnalysisStatewill make the analysis invalid asjoinwill not work as expected (the first join is no longerjoin(uninitialized, init value), instead it becomesjoin(higher value, init value).To correctly re-run the analysis, either a new
DataFlowSolveris created, or we can just clear theanalysisState.Cc @j2kun for downstream. Cc @Mogball for review.