How to add addtional traits to existing Ops?

Hello,

We are working on a dialect that depends on the Arithmetic dialect. We want arith::AddFOp to have a trait that verifies the encoding (a member of RankedTensorType) of all operands. Other than copying all Ops from the Arithmetic dialect and adding new traits, are there any simpler ways to do this? Thanks!

AFAIK, we have dialect extension to add the dependent dialects and interfaces, you can do this through it. But we don’t have a way to invoke the dialect interface in verification, i.e., something like “verifyDialect”. That means you may need to find a proper place to do the verification through dialect interface. The drawback is it’s separated from standard verification path, you may not be able to benefit from certain verification mechanism like verification after pass.

I’m not sure if having “verifyDialect” or “verifyDialectInterface” is a thing we want to support. Need more inputs here.

1 Like

You aren’t really supposed to add traits without modifying the op definition. The point of traits is being statically checkable as well as making extra methods available on the Op class.

However, it doesn’t look like you need traits. What you need it a verification hook that is stricter than what the ops already have. (Note that we wouldn’t want to use traits for this even if we could hypothetically add them dynamically, because it would suddenly make valid IR invalid.) I would suggest using the verifier of some ancestor op that contains the ops you care about: just walk the child ops, check that they satisfy the conditions and report errors at the child op location if they don’t.

Alternatively, you can add an attribute from your dialect to the relevant ops and have a dialect verifier for the attribute that checks the op instead, the dialect attribute verification hook gets a pointer to the op. Note, however, that such attributes may be (and likely are) discarded by transformations.

I’d say we don’t really want that. As described above, it would make some ops valid or invalid depending on what interface implementation is loaded.

1 Like

Technically you can attach an external interface to ops in the arith dialect and have a verifier on the interface. That has issues when integrating in a whole system though, as you’d enforce invariants that the system does not know about.

1 Like

Add one more thing here, you may want to add a region verifier in the ancestor op so that the nested ops will be verified first and it’ll be safer (e.g. won’t manipulate on malformed op) to do additional verification.

If we have it, then the invalid case will be like a verification failure of an interface (except that it’s not an op interface). I think if we scope the validity of ops to include non-op interface verification, then “make some ops valid or invalid depending on what interface implementation is loaded” may not be confused, right?

It may suddenly make the IR invalid but I would think that’s the thing the user needs to take care. Wrapping ops seems a workaround to me, it’s doable but may not be ideal. I’m trying to think about that we have dialect extension, is it reasonable to “extend” the validity of op as well?

Thanks! I will take a look at op verifiers.

I couldn’t entirely follow your reasoning in the first paragraph… In any case, having the op validity depend on something that is not visible in the IR (and thus difficult, if not impossible, to properly test with FileCheck) sounds highly undesirable. Verifiers are meant to ensure that the op has certain semantics. Changing verifiers essentially means changing semantics of the op on-the-fly. Also, on the practical side, this leads to the situation where a downstream user says “arith.addf” fails to verify at head, but it is not reproducible in core, despite “arith” being a core dialect, because the downstream user, or any of other downstream libraries it depends on, added a verifier. Good luck debugging that!

On the contrary, having a wrapping op clearly indicates the additional semantics of the IR, such as “within this region, all values must be tensors” without touching the semantics of nested ops. It also provides an anchor to report new verification errors – the wrapper op, not the actual op that is supposed to work without wrapper’s restrictions.

1 Like

Dialect interface adds the capability which is not stated in the IR, for example, you can define a folder for an op, but you can also define the DialectFoldInterface to fold an op. I just try to use the same concept to apply it on verification here, like “can DialectExtension also extend the validity things?” If we were allowed to do that, that’s just another step for doing verification, a verification failure will also show why it’s failed. If it’s not, then that’s the “interface verifier” developer’s responsibility, just like the if we see an op verification failure without any message, then we should check what’s wrong in the implementation of verifier.

I agree this works well in current design. It’s like tf_executor dialect, but that means we may need to wrap a lot of ops and which may have some side effect like using more memory, or having their optimizations be aware of the wrapping structure.

Again, I just tried to think, if “extend the verification” is also in the scope of dialect extension. Now I know it’s a bad idea.