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

[rriddle]

Summary

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.

[jpienaar]

mlir/test/mlir-pdll/Parser/expr.pdll

99–113

Could you add comments describing the tests?

mlir/include/mlir/Tools/PDLL/AST/Nodes.h

 // ValueRangeConstraintDecl
 
 /// The class represents a ValueRange constraint, and constrains a variable to
 /// be a ValueRange.
 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; }
   const Expr *getTypeExpr() const { return typeExpr; }
 
 protected:
   ValueRangeConstraintDecl(SMRange loc, Expr *typeExpr)
       : Base(loc), typeExpr(typeExpr) {}

mlir/lib/Tools/PDLL/Parser/Parser.cpp

FailureOr<ast::Expr *> Parser::parseOperationExpr() {
   // operation refers to "any" operation(i.e. a difference between `MyOp` and
   // `Operation*`). Operation names within a rewrite context must be named.
   bool allowEmptyName = parserContext != ParserContext::Rewrite;
   FailureOr<ast::OpNameDecl *> opNameDecl =
       parseWrappedOperationName(allowEmptyName);
   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))
         return failure();
       operands.push_back(*operand);
     } while (consumeIf(Token::comma));
 
     if (failed(parseToken(Token::r_paren,
FailureOr<ast::Expr *> Parser::parseOperationExpr() {
 
   // Check for the optional list of result types.
   SmallVector<ast::Expr *> resultTypes;
   if (consumeIf(Token::arrow)) {
     if (failed(parseToken(Token::l_paren,
                           "expected `(` before operation result type list")))
       return failure();
 
+    // 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();
     }
+  } 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,
                              resultTypes);
 }
 
 FailureOr<ast::Expr *> Parser::parseTupleExpr() {
   SMRange loc = curToken.getLoc();
   consumeToken(Token::l_paren);

mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll

 // RUN: mlir-pdll %s -I %S -split-input-file -x mlir | FileCheck %s
 
 //===----------------------------------------------------------------------===//
 // AttributeExpr
 //===----------------------------------------------------------------------===//
 
 // CHECK: pdl.pattern @AttrExpr
 // CHECK: %[[ATTR:.*]] = attribute 10
-// CHECK: operation {"attr" = %[[ATTR]]}
+// CHECK: operation({{.*}}) {"attr" = %[[ATTR]]}
 Pattern AttrExpr => erase op<> { attr = attr<"10"> };
 
 // -----
 
 //===----------------------------------------------------------------------===//
 // CallExpr
 //===----------------------------------------------------------------------===//
 

 // -----
 
 //===----------------------------------------------------------------------===//
 // TypeExpr
 //===----------------------------------------------------------------------===//
 
 // CHECK: pdl.pattern @TypeExpr
 // CHECK: %[[TYPE:.*]] = type : i32
-// CHECK: operation -> (%[[TYPE]] : !pdl.type)
+// CHECK: operation({{.*}}) -> (%[[TYPE]] : !pdl.type)
 Pattern TypeExpr => erase op<> -> (type<"i32">);

mlir/test/mlir-pdll/Parser/expr.pdll

 }
 
 // -----
 
 //===----------------------------------------------------------------------===//
 // 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>
 Pattern {
   erase op<my_dialect.foo>;
 }
 
 // -----