[llvm-branch-commits] [mlir] [MLIR][OpenMP] Unify device shared memory logic, NFCI (PR #182856)

[Sergio Afonso via llvm-branch-commits]

skatrak wrote:

> You could use "[NFCI](https://discourse.llvm.org/t/nfc-abuse/58479/4?u=meinersbur)" ("I" = Intended) fot such cases.

Thanks for the feedback @Meinersbur, I changed the PR title to reflect the intent of these changes.

> Instead of yet another utilities module, why not add it to an existing library that depend on it indifetly anyway, such as MLIROpenMPDialect/MLIROpenACCMPCommon?

It is a rather common practice in other dialects to put logic that can be shared across transformation / conversion passes into its own utils library (e.g. MLIROpenACCUtils, MLIRArithUtils, MLIRLinalgUtils, MLIRMemRefUtils, ...). Here, these functions are shared between a transformation pass and a translation pass, so if this utils library had existed in advance, that would have been the natural place to put them.

I would discard putting these in MLIROpenACCMPCommon because this logic is not shared with OpenACC. The one alternative I think we could indeed make work would be incorporating this into the main MLIROpenMPDialect library as you suggest, but I'm not sure it would be a better alternative. Then, we'd have to introduce a dependency to the main OpenMP dialect library for each dialect we use in these utils, which doesn't seem like a great idea as new features are added over time. Plus, we'd be forcing users of the dialect to link all these internal utilities and dialect dependencies they wouldn't need otherwise.

Let me know what you think.

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