[Mlir-commits] [mlir] [mlir][liveness] fix bugs in liveness analysis (PR #133416)

[Jeff Niu]

================
@@ -106,51 +108,88 @@ void LivenessAnalysis::visitBranchOperand(OpOperand &operand) {
   // the forwarded branch operands or the non-branch operands. Thus they need
   // to be handled separately. This is where we handle them.
 
-  // This marks values of type (1.b) liveness as "live". A non-forwarded
+  // This marks values of type (1.b/1.c) liveness as "live". A non-forwarded
   // branch operand will be live if a block where its op could take the control
-  // has an op with memory effects.
+  // has an op with memory effects or could result in different results.
   // Populating such blocks in `blocks`.
+  bool mayLive = false;
   SmallVector<Block *, 4> blocks;
   if (isa<RegionBranchOpInterface>(op)) {
-    // When the op is a `RegionBranchOpInterface`, like an `scf.for` or an
-    // `scf.index_switch` op, its branch operand controls the flow into this
-    // op's regions.
-    for (Region &region : op->getRegions()) {
-      for (Block &block : region)
-        blocks.push_back(&block);
+    if (op->getNumResults() != 0) {
+      // This mark value of type 1.c liveness as may live, because the region
+      // branch operation has a return value, and the non-forwarded operand can
+      // determine the region to jump to, it can thereby control the result of
+      // the region branch operation.
+      // Therefore, if the result value is live, we conservatively consider the
+      // non-forwarded operand of the region branch operation with result may
+      // live and record all result.
+      for (Value result : op->getResults()) {
+        if (getLatticeElement(result)->isLive) {
----------------
Mogball wrote:

This seems too coarse grained to me. This logic is saying that ANY operand to a branch operation is live if ANY of its results are live. However, you should check whether the operand is actually a non-forwarded operand of the branch operation I think.

https://github.com/llvm/llvm-project/pull/133416