[PATCH] D72995: [MLIR] LLVM Dialect: add llvm.cmpxchg and improve llvm.atomicrmw custom parser

[Alex Zinenko via Phabricator via llvm-commits]

ftynse accepted this revision.
ftynse added a comment.
This revision is now accepted and ready to land.

Minor comments only.

Thanks!



================
Comment at: mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp:1350
 
-static void printAtomicRMWOp(OpAsmPrinter &p, AtomicRMWOp &op) {
-  p << op.getOperationName() << " ";
-  p << '"' << stringifyAtomicBinOp(op.bin_op()) << "\" ";
-  p << '"' << stringifyAtomicOrdering(op.ordering()) << "\" ";
-  p << op.ptr() << ", " << op.val();
-  p.printOptionalAttrDict(op.getAttrs(), {"bin_op", "ordering"});
-  p << " : (" << op.ptr().getType() << ", " << op.val().getType() << ") -> "
-    << op.res().getType();
-}
-
-// <operation> ::= `llvm.atomicrmw` string-literal string-literal
-//                 ssa-use `,` ssa-use attribute-dict? `:` type
-static ParseResult parseAtomicRMWOp(OpAsmParser &parser,
-                                    OperationState &result) {
-  Type type;
-  StringAttr binOp, ordering;
-  llvm::SMLoc binOpLoc, orderingLoc, trailingTypeLoc;
-  OpAsmParser::OperandType ptr, val;
-  if (parser.getCurrentLocation(&binOpLoc) ||
-      parser.parseAttribute(binOp, "bin_op", result.attributes) ||
-      parser.getCurrentLocation(&orderingLoc) ||
-      parser.parseAttribute(ordering, "ordering", result.attributes) ||
-      parser.parseOperand(ptr) || parser.parseComma() ||
-      parser.parseOperand(val) ||
-      parser.parseOptionalAttrDict(result.attributes) || parser.parseColon() ||
-      parser.getCurrentLocation(&trailingTypeLoc) || parser.parseType(type))
+static ParseResult parseAtomicBinOp(OpAsmParser &parser, OperationState &result,
+                                    StringRef attrName) {
----------------
Please add some documentation to this function.


================
Comment at: mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp:1506
+      !valType.isIntegerTy(16) && !valType.isIntegerTy(32) &&
+      !valType.isIntegerTy(64) && !valType.getUnderlyingType()->isHalfTy() &&
+      !valType.isFloatTy() && !valType.isDoubleTy())
----------------
Consider exposing `isHalfTy` on LLVMType


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72995/new/

https://reviews.llvm.org/D72995