[PATCH] D79922: [MLIR] Allow unreachable blocks to violate dominance property.

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/lib/IR/Dominance.cpp:186
+  if (baseInfoIt == dominanceInfos.end())
+    return true;
+  return baseInfoIt->second->isReachableFromEntry(a);
----------------
Is there a reason we assume unknown blocks are reachable?


================
Comment at: mlir/lib/IR/Verifier.cpp:228
+    // Dominance is only reachable inside reachable blocks.
+    if (domInfo->isReachableFromEntry(&block))
+      for (auto &op : block) {
----------------
nit: These if/else are big enough that they should have braces.
```
if (...) {
  ...
  return success();
}

...
return success();
```


================
Comment at: mlir/test/IR/invalid.mlir:1539
+    ^bb1:
+// expected-error @+1 {{operand #0 does not dominate this use}}
+      %2:3 = "bar"(%1) : (i64) -> (i1,i1,i1)
----------------
nit: align the message with the code line.


================
Comment at: mlir/test/IR/invalid.mlir:1545
+    ^bb4:
+      %1 = "foo"() : ()->i64   // expected-note {{operand defined here}}
+  }) : () -> ()
----------------
nit: Move this expected message above.


================
Comment at: mlir/test/IR/parser.mlir:1248
+func @unreachable_dominance_violation_ok() -> i1 {
+  %c = constant 0 : i1       // CHECK: [[VAL:%.*]] = constant 0 : i1
+  return %c : i1    // CHECK:   return [[VAL]] : i1
----------------
nit: Format these checks like the other tests in this file. Right now this test is really hard to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79922