[mlir][dataflow] disallow outside use of propagateIfChanged for DataFlowSolver (#120885)

Detailed writeup is in https://github.com/google/heir/issues/1153. See
also https://github.com/llvm/llvm-project/pull/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
This commit is contained in:
Hongren Zheng 2025-01-28 13:32:28 +08:00 committed by GitHub
parent 3c64f86314
commit 3a439e2caf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 12 additions and 0 deletions

View File

@ -401,6 +401,8 @@ public:
/// Propagate an update to an analysis state if it changed by pushing
/// dependent work items to the back of the queue.
/// This should only be used when DataFlowSolver is running.
/// Otherwise, the solver won't process the work items.
void propagateIfChanged(AnalysisState *state, ChangeResult changed);
/// Get the configuration of the solver.
@ -410,6 +412,9 @@ private:
/// Configuration of the dataflow solver.
DataFlowConfig config;
/// The solver is working on the worklist.
bool isRunning = false;
/// The solver's work queue. Work items can be inserted to the front of the
/// queue to be processed greedily, speeding up computations that otherwise
/// quickly degenerate to quadratic due to propagation of state updates.

View File

@ -10,6 +10,7 @@
#include "mlir/IR/Location.h"
#include "mlir/IR/Operation.h"
#include "mlir/IR/Value.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/iterator.h"
#include "llvm/Config/abi-breaking.h"
#include "llvm/Support/Casting.h"
@ -104,6 +105,10 @@ Location LatticeAnchor::getLoc() const {
//===----------------------------------------------------------------------===//
LogicalResult DataFlowSolver::initializeAndRun(Operation *top) {
// Enable enqueue to the worklist.
isRunning = true;
auto guard = llvm::make_scope_exit([&]() { isRunning = false; });
// Initialize the analyses.
for (DataFlowAnalysis &analysis : llvm::make_pointee_range(childAnalyses)) {
DATAFLOW_DEBUG(llvm::dbgs()
@ -134,6 +139,8 @@ LogicalResult DataFlowSolver::initializeAndRun(Operation *top) {
void DataFlowSolver::propagateIfChanged(AnalysisState *state,
ChangeResult changed) {
assert(isRunning &&
"DataFlowSolver is not running, should not use propagateIfChanged");
if (changed == ChangeResult::Change) {
DATAFLOW_DEBUG(llvm::dbgs() << "Propagating update to " << state->debugName
<< " of " << state->anchor << "\n"