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

[Nikolay Panchenko]

npanchen wrote:

> > The exact issue is
> > ...
> 
> You're showing me some pseudo source program, but I'm not sure what is the actual issue in MLIR and how this patch solves it right now.

Ok, let me reiterate previous comments: the MLIR pseudo-code I provided as-is leads to invalid optimization done by jump threading, because `callee` is not marked with `convergent` attribute. The goal of that PR is to add special trait that some  special pass can easily rely on to propagate convergency through a call graph. That is, to do something like that:
```
if (op->hasTrait<Convergent>() || /*if calls llvm.func that is convergent*/) {
  op->getParentOfType<LLVMFuncOp>()->setConvergent();
}
```
Intentionally use LLVMFuncOp as it does have `convergent` attr that can be used in "the special pass".

> This isn't exactly what I am saying I believe: I'm saying that it isn't good to add something that isn't well defined and documented, for which we can't tell users about how to use it.
> Aren't the example from the presentation and the Clang codegen a good reference about the examples to support?

I'm not sure I follow you. What exactly is not defined in added trait ? The added trait is [defined](https://github.com/llvm/llvm-project/pull/152358/files) in [LLVM's term](https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/IR/Intrinsics.td#L172-L173) with a[ LLVM's link to convergent semantics](https://llvm.org/docs/ConvergentOperations.html). Is there anything else you want me to clarify/add to the doc ?


> Of course we could work around it by saying that "Convergent" is defined exactly as LLVM, and actually restrict it to LLVM function with flat CFGs only. But then that becomes a property of the LLVM layer and not something central in MLIR.

IMO defining it "as LLVM" is totally fine first step. IIRC, `SideEffect` trait was decomposed into several other traits, so if convergent trait is "too generic" that can be split into other traits.
But if you do have some other ideas in mind that will be more generic and still cover LLVM's convergent, I'll be glad to hear them.

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