[PATCH] D76329: [MLIR] Deduplicate dialect registration by ClassID

[Geoffrey Martin-Noble via Phabricator via llvm-commits]

GMNGeoffrey added a comment.

In D76329#1927997 <https://reviews.llvm.org/D76329#1927997>, @mehdi_amini wrote:

> LGTM, but I'd like to hear from River as well.


Yeah I'd also like Rivers' review

> I am not sure what you mean with:
> 
>> This potentially introduces some issues if callers using non-standard dialect registration functions passed to registerDialectAllocator and they are silently overridden.

If people call `registerDialect<Foo>()` we're all good as if there's a duplicate, it would have been the same thing anyway. If someone is directly calling `registerDialectAllocator` (which I don't think anyone is right now) and they pass in their custom allocator that they expect to do something and someone else also registers the same dialect (either with `registerDialect` or `registerDialectAllocator`) then we might have surprising behavior. We could move `registerDialectAllocator` out of the public API, although it's used by the template function, so a bit tricky (maybe just an impl/detail namespace?), but I wanted to hear from others before doing that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76329