[Mlir-commits] [mlir] [mlir] Added `Convergent` trait that matches LLVM's semantics (PR #152358)

[Nikolay Panchenko]

npanchen wrote:

> All the discussions we're having above looks like things to close on.
> 
> Right now you're adding convergent on a bunch of operations which are not convergent as far as I know.
> What concrete problem is this patch solving for you right now?

The exact issue is 
```
fn callee() {
  nvvm.barrier0
}

fn caller() {
  if (condition) {
    call callee()
  }
}
```

I'm ok-ish to add new trait to `nvvm.barrier0`. But it will be great if you can poke your colleagues to revisit remaining intrinsics so that remaining MLIR ops can be updated properly.

> 
> Also adding a trait on something for which we don't have a good definition or an idea of how it'll work with our structured control-flow does not seem like something I'm comfortable to do: I think the plan needs to be more fleshed out.
> 

On one hand I agree with you that adding something that might change in future isn't good. On the other that's hard to reason about proper design without having convergent CF examples to support. 
Since the change is quite minimal I don't think it's a big problem to be replaced with a proper design later (that assumes `RecursivelyConvergent` won't be right approach and trait I'm adding won't fit in that right approach too)

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