[PATCH] D76903: Add a flag on the context to protect against creation of operations in unregistered dialects

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula added a comment.

In D76903#1948937 <https://reviews.llvm.org/D76903#1948937>, @rriddle wrote:

> In D76903#1948918 <https://reviews.llvm.org/D76903#1948918>, @mehdi_amini wrote:
>
> > In D76903#1948894 <https://reviews.llvm.org/D76903#1948894>, @bondhugula wrote:
> >
> > > In D76903#1948876 <https://reviews.llvm.org/D76903#1948876>, @mehdi_amini wrote:
> > >
> > > > In D76903#1948842 <https://reviews.llvm.org/D76903#1948842>, @bondhugula wrote:
> > > >
> > > > > The commit summary opening "By default, the verifier won't allow operations with unregistered dialect. " reads as if this was the situation before.
> > > >
> > > >
> > > > I used the future tense here to indicate the change.
> > > >
> > > > > Besides "operations with unregistered dialect", what about unregistered operations, i.e., operations that aren't part of any registered dialect?
> > > >
> > > > To me “operations with unregistered dialect” is the same as “ operations that aren't part of any registered dialect”.
> > >
> > >
> > > But the two are clearly different. There could be an operation that is part of a dialect that is not registered, and you then could have operations that aren't part of any dialect like "foo"() : () -> () -- the latter isn't part of say any dialect.
> >
> >
> > OK I see what you mean, I'll write “operations that aren't part of any registered dialect” which covers both `f.oo` which if part of the unregistered dialect `f` and `foo` which is not part of any dialect. The verifier is covering both here.
> >
> > (I don't even know why we allow such operation name in the first place to be honest, other than saving a few chars when writing tests there isn't much interest)
>
>
> This is wrong. Operations are always part of a dialect. "foo" would be considered to be part of the builtin dialect, which currently has no namespace. IMO we should set the namespace of the builtin dialect to "mlir" and the assert that operation names
>  always have a dialect prefix.


I didn't know the unknown named ops were considered as part of the builtin dialect - they are anything but built-in!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76903