[PATCH] D77714: Extended Liveness analysis to support nested regions.

[Stephan Herhut via Phabricator via llvm-commits]

herhut added a comment.

This still does not handle the case where values flow through operations (and that is fine, we lack the interface). A good example might be a `loop.for` with `iter_args`

I am OK with this to land with the two comments addressed but wait for @rriddle to comment and approve.



================
Comment at: mlir/lib/Analysis/Liveness.cpp:73
+            [](Operation *op) { return op->getBlock(); });
+        if (ownerBlock && ownerBlock != block) {
+          outValues.insert(value);
----------------
What does it mean if there is no `ownerBlock`. Can this ever happen? It would mean that the use was in a block that does not have the current operation as parent. Values cannot escape regions, though. Maybe rather assert than hiding this case.


================
Comment at: mlir/lib/Analysis/Liveness.cpp:134
   /// values with the current live-out values.
-  template <typename SourceT> void updateLiveOut(SourceT &source) {
+  template <typename SourceT>
+  void updateLiveOut(SourceT &source) {
----------------
Is this a spurious reformat?


================
Comment at: mlir/lib/Analysis/Liveness.cpp:332
     if (Operation *defOp = value.getDefiningOp())
       os << "val_" << defOp->getName();
     else {
----------------
Use valueIds here?


================
Comment at: mlir/test/Analysis/test-liveness.mlir:312
+    // CHECK: Block: 3
+    // CHECK-NEXT: LiveIn: arg6 at 0 val_std.addi val_std.addi
+    // CHECK-NEXT: LiveOut:{{ *$}}
----------------
I think this would be easier to read if the function arguments were already named %argX in the input. Also, please make the value names unique, either by using the counter or  you could use an instruction position trace, using block numbers and trace back via parent ops.

`i1 at b0@i5` for the second instruction in block zero of the fifth instruction. These are easier to map back to the actual code than global numbers.

In any case, something to make the printout non-ambiguous.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77714/new/

https://reviews.llvm.org/D77714