Patch dataflow framework

It had a bug whereby the regions of an
op are marked as dead code, and their
successors are not properly populated,
if any operand of a RegionBranchOpInterface
is not folded to a constant. The interface
already is meant to support partially-folded
operands, so we should use that instead of giving up.

Some changes are also just to make debug prints
more helpful. Some of them were printing pointer
addresses.

Author: oowekyala

mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h

@@ -223,6 +223,10 @@ class DeadCodeAnalysis : public DataFlowAnalysis {
   /// Get the constant values of the operands of the operation. Returns
   /// std::nullopt if any of the operand lattices are uninitialized.
   std::optional<SmallVector<Attribute>> getOperandValues(Operation *op);
+  
+  /// Get the constant values of the operands of the operation.
+  /// If the operand lattices are uninitialized, add a null attribute for those.
+  SmallVector<Attribute> getOperandValuesBestEffort(Operation *op);
 
   /// The top-level operation the analysis is running on. This is used to detect
   /// if a callable is outside the scope of the analysis and thus must be

mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp

@@ -341,23 +341,37 @@ void DeadCodeAnalysis::visitCallOperation(CallOpInterface call) {
 /// constant value lattices are uninitialized, return std::nullopt to indicate
 /// the analysis should bail out.
 static std::optional<SmallVector<Attribute>> getOperandValuesImpl(
-    Operation *op,
+    Operation *op, bool failIfAnyNull,
     function_ref<const Lattice<ConstantValue> *(Value)> getLattice) {
   SmallVector<Attribute> operands;
   operands.reserve(op->getNumOperands());
   for (Value operand : op->getOperands()) {
     const Lattice<ConstantValue> *cv = getLattice(operand);
     // If any of the operands' values are uninitialized, bail out.
-    if (cv->getValue().isUninitialized())
-      return {};
+    if (cv->getValue().isUninitialized()) {
+      if (failIfAnyNull)
+        return {};
+      operands.emplace_back();
+      continue;
+    }
+
     operands.push_back(cv->getValue().getConstantValue());
   }
   return operands;
 }
 
 std::optional<SmallVector<Attribute>>
 DeadCodeAnalysis::getOperandValues(Operation *op) {
-  return getOperandValuesImpl(op, [&](Value value) {
+  return getOperandValuesImpl(op, true, [&](Value value) {
+    auto *lattice = getOrCreate<Lattice<ConstantValue>>(value);
+    lattice->useDefSubscribe(this);
+    return lattice;
+  });
+}
+
+SmallVector<Attribute>
+DeadCodeAnalysis::getOperandValuesBestEffort(Operation *op) {
+  return *getOperandValuesImpl(op, false, [&](Value value) {
     auto *lattice = getOrCreate<Lattice<ConstantValue>>(value);
     lattice->useDefSubscribe(this);
     return lattice;
@@ -366,11 +380,9 @@ DeadCodeAnalysis::getOperandValues(Operation *op) {
 
 void DeadCodeAnalysis::visitBranchOperation(BranchOpInterface branch) {
   // Try to deduce a single successor for the branch.
-  std::optional<SmallVector<Attribute>> operands = getOperandValues(branch);
-  if (!operands)
-    return;
+  SmallVector<Attribute> operands = getOperandValuesBestEffort(branch);
 
-  if (Block *successor = branch.getSuccessorForOperands(*operands)) {
+  if (Block *successor = branch.getSuccessorForOperands(operands)) {
     markEdgeLive(branch->getBlock(), successor);
   } else {
     // Otherwise, mark all successors as executable and outgoing edges.
@@ -382,12 +394,10 @@ void DeadCodeAnalysis::visitBranchOperation(BranchOpInterface branch) {
 void DeadCodeAnalysis::visitRegionBranchOperation(
     RegionBranchOpInterface branch) {
   // Try to deduce which regions are executable.
-  std::optional<SmallVector<Attribute>> operands = getOperandValues(branch);
-  if (!operands)
-    return;
+  SmallVector<Attribute> operands = getOperandValuesBestEffort(branch);
 
   SmallVector<RegionSuccessor> successors;
-  branch.getEntrySuccessorRegions(*operands, successors);
+  branch.getEntrySuccessorRegions(operands, successors);
   for (const RegionSuccessor &successor : successors) {
     // The successor can be either an entry block or the parent operation.
     ProgramPoint *point =

mlir/lib/Analysis/DataFlowFramework.cpp

@@ -44,9 +44,9 @@ void AnalysisState::addDependency(ProgramPoint *dependent,
   (void)inserted;
   DATAFLOW_DEBUG({
     if (inserted) {
-      llvm::dbgs() << "Creating dependency between " << debugName << " of "
-                   << anchor << "\nand " << debugName << " on " << dependent
-                   << "\n";
+      llvm::dbgs() << "Creating dependency between \t" << debugName << " of "
+                   << anchor << "\n                      and\t" << debugName
+                   << " of " << *dependent << "\n";
     }
   });
 }
@@ -125,7 +125,7 @@ LogicalResult DataFlowSolver::initializeAndRun(Operation *top) {
     worklist.pop();
 
     DATAFLOW_DEBUG(llvm::dbgs() << "Invoking '" << analysis->debugName
-                                << "' on: " << point << "\n");
+                                << "' on: " << *point << "\n");
     if (failed(analysis->visit(point)))
       return failure();
   }