[mlir:PDLL] Fix handling of unspecified operands/results on operation expressions

If the operand list or result list of an operation expression is not specified, we interpret
this as meaning that the operands/results are "unconstraint" (i.e. "could be anything").
We currently don't properly handle differentiating this case from the case of
"no operands/results". This commit adds the insertion of implicit value/type range
variables when these lists are unspecified. This allows for adding proper support
for when zero operands or results are expected.

Differential Revision: https://reviews.llvm.org/D119780

4 files changed, 69 insertions, 13 deletions

diff --git a/mlir/include/mlir/Tools/PDLL/AST/Nodes.h b/mlir/include/mlir/Tools/PDLL/AST/Nodes.h
index 03c0215..3eb3caa 100644
--- a/mlir/include/mlir/Tools/PDLL/AST/Nodes.h
+++ b/mlir/include/mlir/Tools/PDLL/AST/Nodes.h
@@ -786,7 +786,7 @@ class ValueRangeConstraintDecl
     : public Node::NodeBase<ValueRangeConstraintDecl, CoreConstraintDecl> {
 public:
   static ValueRangeConstraintDecl *create(Context &ctx, SMRange loc,
-                                          Expr *typeExpr);
+                                          Expr *typeExpr = nullptr);
 
   /// Return the optional type the value range is constrained to.
   Expr *getTypeExpr() { return typeExpr; }
diff --git a/mlir/lib/Tools/PDLL/Parser/Parser.cpp b/mlir/lib/Tools/PDLL/Parser/Parser.cpp
index 7339757..e69e8ff 100644
--- a/mlir/lib/Tools/PDLL/Parser/Parser.cpp
+++ b/mlir/lib/Tools/PDLL/Parser/Parser.cpp
@@ -1588,9 +1588,28 @@ FailureOr<ast::Expr *> Parser::parseOperationExpr() {
   if (failed(opNameDecl))
     return failure();
 
+  // Functor used to create an implicit range variable, used for implicit "all"
+  // operand or results variables.
+  auto createImplicitRangeVar = [&](ast::ConstraintDecl *cst, ast::Type type) {
+    FailureOr<ast::VariableDecl *> rangeVar =
+        defineVariableDecl("_", loc, type, ast::ConstraintRef(cst, loc));
+    assert(succeeded(rangeVar) && "expected range variable to be valid");
+    return ast::DeclRefExpr::create(ctx, loc, *rangeVar, type);
+  };
+
   // Check for the optional list of operands.
   SmallVector<ast::Expr *> operands;
-  if (consumeIf(Token::l_paren)) {
+  if (!consumeIf(Token::l_paren)) {
+    // If the operand list isn't specified and we are in a match context, define
+    // an inplace unconstrained operand range corresponding to all of the
+    // operands of the operation. This avoids treating zero operands the same
+    // way as "unconstrained operands".
+    if (parserContext != ParserContext::Rewrite) {
+      operands.push_back(createImplicitRangeVar(
+          ast::ValueRangeConstraintDecl::create(ctx, loc), valueRangeTy));
+    }
+  } else if (!consumeIf(Token::r_paren)) {
+    // If the operand list was specified and non-empty, parse the operands.
     do {
       FailureOr<ast::Expr *> operand = parseExpr();
       if (failed(operand))
@@ -1625,16 +1644,26 @@ FailureOr<ast::Expr *> Parser::parseOperationExpr() {
                           "expected `(` before operation result type list")))
       return failure();
 
-    do {
-      FailureOr<ast::Expr *> resultTypeExpr = parseExpr();
-      if (failed(resultTypeExpr))
-        return failure();
-      resultTypes.push_back(*resultTypeExpr);
-    } while (consumeIf(Token::comma));
+    // Handle the case of an empty result list.
+    if (!consumeIf(Token::r_paren)) {
+      do {
+        FailureOr<ast::Expr *> resultTypeExpr = parseExpr();
+        if (failed(resultTypeExpr))
+          return failure();
+        resultTypes.push_back(*resultTypeExpr);
+      } while (consumeIf(Token::comma));
 
-    if (failed(parseToken(Token::r_paren,
-                          "expected `)` after operation result type list")))
-      return failure();
+      if (failed(parseToken(Token::r_paren,
+                            "expected `)` after operation result type list")))
+        return failure();
+    }
+  } else if (parserContext != ParserContext::Rewrite) {
+    // If the result list isn't specified and we are in a match context, define
+    // an inplace unconstrained result range corresponding to all of the results
+    // of the operation. This avoids treating zero results the same way as
+    // "unconstrained results".
+    resultTypes.push_back(createImplicitRangeVar(
+        ast::TypeRangeConstraintDecl::create(ctx, loc), typeRangeTy));
   }
 
   return createOperationExpr(loc, *opNameDecl, operands, attributes,
diff --git a/mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll b/mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll
index 4205e56..3e652ad 100644
--- a/mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll
+++ b/mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll
@@ -6,7 +6,7 @@
 
 // CHECK: pdl.pattern @AttrExpr
 // CHECK: %[[ATTR:.*]] = attribute 10
-// CHECK: operation {"attr" = %[[ATTR]]}
+// CHECK: operation({{.*}}) {"attr" = %[[ATTR]]}
 Pattern AttrExpr => erase op<> { attr = attr<"10"> };
 
 // -----
@@ -89,5 +89,5 @@ Pattern TupleMemberAccessName {
 
 // CHECK: pdl.pattern @TypeExpr
 // CHECK: %[[TYPE:.*]] = type : i32
-// CHECK: operation -> (%[[TYPE]] : !pdl.type)
+// CHECK: operation({{.*}}) -> (%[[TYPE]] : !pdl.type)
 Pattern TypeExpr => erase op<> -> (type<"i32">);
diff --git a/mlir/test/mlir-pdll/Parser/expr.pdll b/mlir/test/mlir-pdll/Parser/expr.pdll
index d645fea..9919fe5 100644
--- a/mlir/test/mlir-pdll/Parser/expr.pdll
+++ b/mlir/test/mlir-pdll/Parser/expr.pdll
@@ -75,15 +75,42 @@ Pattern {
 // OperationExpr
 //===----------------------------------------------------------------------===//
 
+// Test a non-constrained operation expression, and ensure that we don't treat
+// unconstrained as "not present"(e.g. zero operands).
+
 // CHECK: Module
 // CHECK: `-OperationExpr {{.*}} Type<Op>
 // CHECK:   `-OpNameDecl
+// CHECK:   `Operands`
+// CHECK:     `-DeclRefExpr {{.*}} Type<ValueRange>
+// CHECK:       `-VariableDecl {{.*}} Name<_> Type<ValueRange>
+// CHECK:         `Constraints`
+// CHECK:           `-ValueRangeConstraintDecl
+// CHECK:   `Result Types`
+// CHECK:     `-DeclRefExpr {{.*}} Type<TypeRange>
+// CHECK:       `-VariableDecl {{.*}} Name<_> Type<TypeRange>
+// CHECK:         `Constraints`
+// CHECK:           `-TypeRangeConstraintDecl
 Pattern {
   erase op<>;
 }
 
 // -----
 
+// Test explicitly empty operand/result/etc. lists, which are different from the 
+// "unconstrained" examples above.
+
+// CHECK: Module
+// CHECK: `-OperationExpr {{.*}} Type<Op>
+// CHECK:   `-OpNameDecl
+// CHECK-NOT: `Operands`
+// CHECK-NOT: `Result Types`
+Pattern {
+  erase op<>() -> ();
+}
+
+// -----
+
 // CHECK: Module
 // CHECK: `-OperationExpr {{.*}} Type<Op<my_dialect.foo>>
 // CHECK:   `-OpNameDecl {{.*}} Name<my_dialect.foo>