[Mlir-commits] [mlir] [mlir][Transforms] Make lookup without type converter unambiguous (PR #151747)

[Markus Böck]

================
@@ -202,17 +202,33 @@ struct MyConversionPattern : public ConversionPattern {
 
 #### Type Safety
 
-The types of the remapped operands provided to a conversion pattern must be of a
-type expected by the pattern. The expected types of a pattern are determined by
-a provided [TypeConverter](#type-converter). If no type converter is provided,
-the types of the remapped operands are expected to match the types of the
-original operands. If a type converter is provided, the types of the remapped
-operands are expected to be legal as determined by the converter. If the
-remapped operand types are not of an expected type, and a materialization to the
-expected type could not be performed, the pattern fails application before the
-`matchAndRewrite` hook is invoked. This ensures that patterns do not have to
-explicitly ensure type safety, or sanitize the types of the incoming remapped
-operands. More information on type conversion is detailed in the
+The types of the remapped operands provided to a conversion pattern (through
+the adaptor or `ArrayRef` of operands) depend on type conversion rules.
+
+If the pattern was initialized with a [type converter](#type-converter), the
+conversion driver passes values whose types match the legalized types of the
+operands of the matched operation as per the type converter. To that end, the
+conversion driver may insert target materializations to convert the most
+recently mapped values to the expected legalized types. The driver tries to
+reuse existing materializations on a best-effort basis, but this is not
+guaranteed by the infrastructure. If the operand types of the matched op could
+not be legalized, the pattern fails to apply before the `matchAndRewrite` hook
+is invoked.
+
+If the pattern was initialized without a type converter, the conversion driver
+passes the most recently mapped values to the pattern, excluding any
+materializations. Materializations are intentionally excluded because their
+presence may depend on other patterns. Passing materializationsm would make the
+conversion infrastructure fragile and unpredictable. Moreover, there could be
+multiple materializations to different types. (This can be the case when
+multiple patterns are running with different type converters.) In such a case,
+it would be unclear which materialization to pass. If a value with the same
+type as the original operand is desired, users can directly take the respective
+operand from the matched operation.
----------------
zero9178 wrote:

I feel the way this paragraph is written is more useful for dialect conversion developers than users.

I think it'd be useful to instead rephrase this as simply "the value that most recently replaced an SSA value within a pattern". I don't think users are generally aware of the internal mapping datastructures and that materialization has an effect on it.  

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