[Mlir-commits] [mlir] [mlir][IR] Add builtin `TokenType` (PR #195640)

[Fabian Mora]

================
@@ -1237,6 +1237,26 @@ def Builtin_RankedTensor : Builtin_Type<"RankedTensor", "tensor", [
   let genVerifyDecl = 1;
 }
 
+//===----------------------------------------------------------------------===//
+// TokenType
+//===----------------------------------------------------------------------===//
+
+def Builtin_Token : Builtin_Type<"Token", "token"> {
+  let summary = "Token type";
+  let description = [{
+    Syntax:
+
+    ```
+    token-type ::= `token`
+    ```
+
+    A use of a token SSA value is a pointer to an operation (in case of an
+    OpResult) or a pointer to a region (in case of an entry block argument).
+    A token carries no runtime data and cannot be forwarded. Tokens are
+    excluded from the `AnyType` type constraint.
----------------
fabianmcg wrote:

> We could add those two small checks for extra safety: #197131. (Doesn't handle all non-ODS ops.)

The above PR is a perfect example of what I'm concerned of. We are saying, this a type that cannot be used in most situations, it has a lot of structural constraints... Yet, we are adding special verification cases to upstream interfaces to prevent misuse, IMO this is not a scalable or robust approach in general. Or at the very least, is misleading calling these structural if there's no structural verification.

Also, if I recall correctly verification is never enforced on external models (we should consider making some interfaces never allowed to be external models), so there are scenarios where nothing will catch the misuse of tokens even for `RegionBranchOpInterface`.

> We are focused on ODS for years and C++ op definition is already "2nd class citizen".

There is also IRDL... While I agree with everything non-ODS is second-class (implicitly), this was not discussed as such in the RFC, as the consequence is now explicit.



To give an alternative. If we are calling all of this structural, I'm going back to my comment in the RFC and I think we should have Op traits `ProducesTokens` and `AcceptsTokens` (or the negatives) and make https://github.com/llvm/llvm-project/blob/main/mlir/lib/IR/Verifier.cpp enforce them (ODS can detect the type and add the traits automatically). Note, that we are already verifying 3 structural straits like that in the Verifier (IsTerminator, NoTerminator, IsolatedFromAbove), so not unprecedented.

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