[MLIR] Replace a not of a comparison with appropriate comparison

[rriddle]

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

2757

Please use m_Constant to match constants.

2766

Are all of these mlir:: necessary? If not, please drop them.

mlir/test/Dialect/Standard/canonicalize.mlir

312

Can you add tests for the other cases?

[wsmoses]

Test all integer comparisons

[chelini]

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

2751

I would make this top-level comment more generic, something like Replace a not of a comparison operation, for example: not(cmp eq A, B) => cmp ne A, B.

mlir/test/Dialect/Standard/canonicalize.mlir

321

No newline.

[ftynse]

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

2751

And also mention that "not" is in practice "xor 1".

2764

The i / 1 - i trick deserves a comment. (Personally, I would have written a function and just called it twice).

2766

I suppose you want to continue here rather than error out. Otherwise you seem to be missing on a valid an optimizable case of xor %value, 1 : i1.

[wsmoses]

Address comments

[mehdi_amini]

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

2759

Please avoid i and other single letter variable name and find something meaningful instead. Even operand_id would be better.

Also, in general we don't need to have such loop. Instead we should canonicalize commutative operations to have constant always on the same side.

[rriddle]

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

2759

(operandId given we use camelCase variables)

[wsmoses]

Remove for loop

[ftynse]

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

2752

Nit: trailing dot please.

2760

Nit: let's mention that we expect constants to have been canonicalized to be the second operand.

mlir/include/mlir/Dialect/StandardOps/IR/Ops.td

let description = [{
     // SIMD vector element-wise bitwise integer xor.
     %f = xor %g, %h : vector<4xi32>
 
     // Tensor element-wise bitwise integer xor.
     %x = xor %y, %z : tensor<4x?xi8>
     ```
   }];
   let hasFolder = 1;
+  let hasCanonicalizer = 1;
 }
 
 //===----------------------------------------------------------------------===//
 // ZeroExtendIOp
 //===----------------------------------------------------------------------===//
 
 def ZeroExtendIOp : Std_Op<"zexti", [NoSideEffect,
     DeclareOpInterfaceMethods<VectorUnrollOpInterface>] #

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

OpFoldResult XOrOp::fold(ArrayRef<Attribute> operands) {
   /// xor(x,x) -> 0
   if (lhs() == rhs())
     return Builder(getContext()).getZeroAttr(getType());
 
   return constFoldBinaryOp<IntegerAttr>(operands,
                                         [](APInt a, APInt b) { return a ^ b; });
 }
 
+namespace {
+/// not(cmp eq A, B) => cmp ne A, B
+struct NotICmp : public OpRewritePattern<XOrOp> {
+  using OpRewritePattern<XOrOp>::OpRewritePattern;
+
+  LogicalResult matchAndRewrite(XOrOp op,
+                                PatternRewriter &rewriter) const override {
+    auto c1 = op.rhs().getDefiningOp<ConstantOp>();
+    if (!c1)
+      return failure();
+    if (c1.getValue().cast<IntegerAttr>().getValue() != 1)
+      return failure();
+    auto prev = op.lhs().getDefiningOp<CmpIOp>();
+    if (!prev)
+      return failure();
+    switch (prev.predicate()) {
+    case mlir::CmpIPredicate::eq:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::ne,
+                                          prev.lhs(), prev.rhs());
+      return success();
+    case mlir::CmpIPredicate::ne:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::eq,
+                                          prev.lhs(), prev.rhs());
+      return success();
+
+    case mlir::CmpIPredicate::slt:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::sge,
+                                          prev.lhs(), prev.rhs());
+      return success();
+    case mlir::CmpIPredicate::sle:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::sgt,
+                                          prev.lhs(), prev.rhs());
+      return success();
+    case mlir::CmpIPredicate::sgt:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::sle,
+                                          prev.lhs(), prev.rhs());
+      return success();
+    case mlir::CmpIPredicate::sge:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::slt,
+                                          prev.lhs(), prev.rhs());
+      return success();
+
+    case mlir::CmpIPredicate::ult:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::uge,
+                                          prev.lhs(), prev.rhs());
+      return success();
+    case mlir::CmpIPredicate::ule:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::ugt,
+                                          prev.lhs(), prev.rhs());
+      return success();
+    case mlir::CmpIPredicate::ugt:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::ule,
+                                          prev.lhs(), prev.rhs());
+      return success();
+    case mlir::CmpIPredicate::uge:
+      rewriter.replaceOpWithNewOp<CmpIOp>(op, mlir::CmpIPredicate::ult,
+                                          prev.lhs(), prev.rhs());
+      return success();
+    }
+    return failure();
+  }
+};
+} // namespace
+
+void XOrOp::getCanonicalizationPatterns(OwningRewritePatternList &results,
+                                        MLIRContext *context) {
+  results.insert<NotICmp>(context);
+}
+
 //===----------------------------------------------------------------------===//
 // ZeroExtendIOp
 //===----------------------------------------------------------------------===//
 
 static LogicalResult verify(ZeroExtendIOp op) {
   auto srcType = getElementTypeOrSelf(op.getOperand().getType());
   auto dstType = getElementTypeOrSelf(op.getType());
 

mlir/test/Dialect/Standard/canonicalize.mlir

 
 // CHECK-LABEL: @select_cmp_ne_select
 //       CHECK:   return %arg0
 func @select_cmp_ne_select(%arg0: i64, %arg1: i64) -> i64 {
   %0 = cmpi ne, %arg0, %arg1 : i64
   %1 = select %0, %arg0, %arg1 : i64
   return %1 : i64
 }
+
+// -----
+
+// CHECK-LABEL: @notCmp
+//       CHECK:   %[[cres:.+]] = cmpi ne, %arg0, %arg1 : i8
+//       CHECK:   return %[[cres]]
+func @notCmp(%arg0: i8, %arg1: i8) -> i1 {
+  %true = constant true
+  %cmp = cmpi "eq", %arg0, %arg1 : i8
+  %ncmp = xor %cmp, %true : i1
+  return %ncmp : i1
+}
 No newline at end of file