Docuumentation updates per review

Author: krzysz00

mlir/docs/DeclarativeRewrites.md

@@ -431,10 +431,9 @@ NativeCodeCall<"someFn($1, $2, $0)">` and use it like `(SomeCall $in0, $in1,
 $in2)`, then this will be translated into C++ call `someFn($in1, $in2, $in0)`.
 
 In the case of properties, the placeholder will be bound to a value of the _interface_
-type of the property. Passing in a `StringProp` as an argument to a `NativeCodeCall`
-will pass a `StringRef` (as if the getter of the matched operation were called)
-and not a `std::string`. See `mlir/include/mlir/IR/Properties.td` for details
-on interface vs. storage type.
+type of the property. For example, passing in a `StringProp` as an argument to a `NativeCodeCall` will pass a `StringRef` (as if the getter of the matched
+operation were called) and not a `std::string`. See
+`mlir/include/mlir/IR/Properties.td` for details on interface vs. storage type.
 
 Positional range placeholders will be substituted by multiple `dag` object
 parameters at the `NativeCodeCall` use site. For example, if we define

mlir/include/mlir/IR/Properties.td

@@ -405,6 +405,9 @@ class ConfinedProperty<Property p, Pred pred, string newSummary = "">
 /// equal to the given value (and matches any other constraints on the property).
 /// The constant value is given as a string and should be of the _interface_ type
 /// of the attribute.
+///
+/// This requires that the given property's inference type be comparable to the
+/// given value with `==`, and does require specify a concrete property type.
 class ConstantProp<Property prop, string val>
     : ConfinedProp<prop,
         CPred<"$_self == " # val>,

mlir/include/mlir/TableGen/Pattern.h

@@ -300,10 +300,9 @@ class SymbolInfoMap {
     //   the DAG of the operation, `operandIndexOrNumValues` specifies the
     //   operand index, and `variadicSubIndex` must be set to `std::nullopt`.
     //
-    // * Properties not associated with an operation (ex. as arguments to
+    // * Properties not associated with an operation (e.g. as arguments to
     //   native code) have their corresponding PropConstraint stored in the
-    //   `dag` field. (They'll and set `operandIndexOrNumValues` to -1 to
-    //   indicate they aren't part of an operation.)
+    //   `dag` field. This constraint is only used when
     //
     // * If a symbol is defined in a `variadic` DAG, `dag` specifies the DAG
     //   of the parent operation, `operandIndexOrNumValues` specifies the
@@ -423,9 +422,9 @@ class SymbolInfoMap {
                         DagAndConstant(nullptr, index, std::nullopt));
     }
     static SymbolInfo getProp(const PropConstraint *constraint) {
+      // -1 for anthe `operandIndexOrNumValues` is a sentinel value.
       return SymbolInfo(nullptr, Kind::Prop,
                         DagAndConstant(constraint, -1, std::nullopt));
-      ;
     }
     static SymbolInfo
     getOperand(DagNode node, const Operator *op, int operandIndex,

mlir/lib/TableGen/Pattern.cpp

@@ -830,8 +830,8 @@ void Pattern::collectBoundSymbols(DagNode tree, SymbolInfoMap &infoMap,
       if (!treeArgName.empty() && treeArgName != "_") {
         DagLeaf leaf = tree.getArgAsLeaf(i);
 
-        // In (NativeCodeCall<"Foo($_self, $0, $1, $2)"> I8Attr:$a, I8:$b, $c,
-        // I8Prop:$d),
+        // In (NativeCodeCall<"Foo($_self, $0, $1, $2, $3)"> I8Attr:$a, I8:$b,
+        //     $c, I8Prop:$d),
         if (leaf.isUnspecified()) {
           // This is case of $c, a Value without any constraints.
           verifyBind(infoMap.bindValue(treeArgName), treeArgName);