[PATCH] D75507: [mlir] Extended Dominance analysis with a function to find the nearest common dominator of two given blocks.

[Stephan Herhut via Phabricator via llvm-commits]

herhut added inline comments.


================
Comment at: mlir/lib/Analysis/Dominance.cpp:162
+    if (!b)
+      return IsPostDom;
 
----------------
rriddle wrote:
> herhut wrote:
> > You did not introduce this. Still, I do not get the post-dominates here. If `a` and `b` are both in subregions, like
> > ```
> > top {
> >   op1 {
> >     a^:
> >   }
> >   op2 {
> >     b^:
> >   }
> > }
> > ```
> > then the above code will not find a region in the ancestry of `b` that is also the region of `a`. Still, `a` does not post-dominate `b`. 
> Nested dominance has a lot of trickyness. We generally model dominance here as being able to flow-in, but not flow-out. So in the above, a^ can't dominate anything defined above its region inside of op1. That being said, different users want different things so we could likely expand the API surface area to handle the different cases.
> 
@rriddle I was more wondering about the case of post-dominance here. In the current implementation, `a` post-dominates `b` if I read the code right. That seems wrong to me.

`a` also does not dominate `b`. 

> being able to flow-in, but not flow-out

I did not understand that comment. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75507