[Mlir-commits] [mlir] Update MLIR conversion to LLVMFunc to account better for properties (PR #67406)

[Tobias Gysi]

gysit wrote:

> I think we discussed making visibility a part of symbol interface and having it depend on the linkage attribute for LLVMFuncOp. That would be my preferred solution. However, @jpienaar had an argument about having private linkage yet public visibility (or vice versa) functions somehow, but I don't remember what it was.

It would be nice if we find a consistent way of deriving the visibility from the linkage and maybe the fact if a function is a declaration. I remember there have been discussions about it before and it think it was not completely trivial to find a consistent solution. I think the SymbolOpVerifier checks that a declaration does not have public visibility. LLVM on the other hand verifies that a declaration has external linkage (or extern_weak). That means public != extern as one could assume. On the other hand, setting the visibility of all extern symbols to private is confusing since private linkage is exactly the opposite of extern linkage 🤯 (I hope I got that right...).  

Another argument may be that we want to keep supporting scenarios where symbols need to have a configurable visibility. For example, when compiling for GPUs one may want to be able to have nested modules and symbols with a specific visibility? Coupling visibility and LLVM linkage may be too limiting in this case?





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