[mlir] DCA optimistically assuming function not part of DataFlowAnalysis is unreachable

[zero9178]

Reproducer that can be run via mlir-opt %s --pass-pipeline="func.func(sccp)":

func.func private @foo() -> index {
    %0 = arith.constant 10 : index
    return %0 : index
}

func.func private @bar(%arg0: index) -> index {
  %c0 = arith.constant 0 : index
  %1 = arith.constant 420 : index
  %7 = arith.cmpi eq, %arg0, %c0 : index
  cf.cond_br %7, ^bb1(%1 : index), ^bb2

^bb1(%8: index):  // 2 preds: ^bb0, ^bb4
  return %8 : index

^bb2:
  %13 = call @foo() : () -> index
  cf.br ^bb1(%13 : index)
}

this will incorrectly fold the return %8 : index to return %c420 : index, despite ^bb2 branching to ^bb1 with a different value as block argument.

The underlying issue is that:

llvm-project/mlir/lib/Analysis/DataFlow/SparseAnalysis.cpp

Lines 109 to 117 in ab70197

	const auto *predecessors = getOrCreateFor<PredecessorState>(op, call);
	// If not all return sites are known, then conservatively assume we can't
	// reason about the data-flow.
	if (!predecessors->allPredecessorsKnown())
	return markAllPessimisticFixpoint(resultLattices);
	for (Operation *predecessor : predecessors->getKnownPredecessors())
	for (auto it : llvm::zip(predecessor->getOperands(), resultLattices))
	join(std::get<1>(it), *getLatticeElementFor(op, std::get<0>(it)));
	return;

queries for a predecessor state of a call that is never created/written to by DCE. Since PredecessorsState optimistically assumes all predecessors are know, and also has no known predecessors at the same time, it does nothing but return at line 117.
I can only guess, but it seems to me the intention was that this predecessor state should be set in either:

llvm-project/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp

Line 410 in ab70197

auto *callsites = getOrCreateFor<PredecessorState>(op, callable);

or

llvm-project/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp

Line 300 in ab70197

auto *callsites = getOrCreate<PredecessorState>(callableOp);

or maybe during priming of the analysis.
The former does not work as the analysis is scheduled on func.func operations, hence it never seeing the func.return of @foo.
The latter only sets the call as predecessor to the callable.
I think what is needed is to mark the predecessors of the call as unknown if the callable is not part of the analysis.

[llvmbot]

@llvm/issue-subscribers-mlir-core