diff --git a/mlir/lib/Conversion/PDLToPDLInterp/PredicateTree.cpp b/mlir/lib/Conversion/PDLToPDLInterp/PredicateTree.cpp index 99bf73de59e37..5d249a37d50fd 100644 --- a/mlir/lib/Conversion/PDLToPDLInterp/PredicateTree.cpp +++ b/mlir/lib/Conversion/PDLToPDLInterp/PredicateTree.cpp @@ -768,17 +768,25 @@ struct OrderedPredicate { /// model. bool operator<(const OrderedPredicate &rhs) const { // Sort by: + // * not being a constraint. Rational: When writing constraints, it is + // sometimes assumed that checks for null or operation names are executed + // before the constraint. As there is no dependency between this + // operation, this is not always guaranteed, which can lead to bugs if the + // constraints is not checking inputs for null itself. By ordering + // constraints to the end, it is assured that implicit checks are nun + // before them // * higher first and secondary order sums // * lower depth // * lower position dependency // * lower predicate dependency // * lower tie breaking ID auto *rhsPos = rhs.position; - return std::make_tuple(primary, secondary, rhsPos->getOperationDepth(), + return std::make_tuple(!isa(question), primary, + secondary, rhsPos->getOperationDepth(), rhsPos->getKind(), rhs.question->getKind(), rhs.id) > - std::make_tuple(rhs.primary, rhs.secondary, - position->getOperationDepth(), position->getKind(), - question->getKind(), id); + std::make_tuple(!isa(rhs.question), rhs.primary, + rhs.secondary, position->getOperationDepth(), + position->getKind(), question->getKind(), id); } }; diff --git a/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir b/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir index aeb4c7233d1ff..42fadbc3a6a90 100644 --- a/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir +++ b/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir @@ -488,6 +488,27 @@ module @predicate_ordering { } } +// ----- + +// CHECK-LABEL: module @predicate_ordering_attr +module @predicate_ordering_attr { + // Check that the result is checked for null first, before applying the + // constraint. + + // CHECK: func @matcher(%[[ROOT:.*]]: !pdl.operation) + // CHECK: %[[RESULT:.*]] = pdl_interp.get_attribute "attr" of %[[ROOT]] + // CHECK-NEXT: pdl_interp.is_not_null %[[RESULT]] + // CHECK: pdl_interp.apply_constraint "constraint" + + + pdl.pattern : benefit(1) { + %attr = attribute + pdl.apply_native_constraint "constraint"(%attr: !pdl.attribute) + pdl.apply_native_constraint "constraint"(%attr: !pdl.attribute) + %root = operation "foo.op" {"attr" = %attr} + rewrite %root with "rewriter" + } +} // -----