[PATCH] D79479: [MLIR] Add complex addition and substraction to the standard dialect

[Alex Zinenko via Phabricator via llvm-commits]

ftynse accepted this revision.
ftynse added inline comments.


================
Comment at: mlir/include/mlir/Dialect/StandardOps/IR/Ops.td:221
+
+def AddCOp : ComplexArithmeticOp<"addc"> {
+  let summary = "complex number addition";
----------------
pifon2a wrote:
> @ftynse Alex, do you know if there is a need for arithmetic ops for complex<integer_type>? If yes, how should we call this one? `AddCFOp`? :)
I don't see an immediate need, but, e.g., C++ supports `complex<int>`.
Independently, `addc` is "add and carry" on x86.


================
Comment at: mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp:1409
+    auto structType = this->typeConverter.convertType(bop.getType());
+    auto result = ComplexStructBuilder ::undef(rewriter, loc, structType);
+
----------------
Nit: drop the extra whitespace before `::`


================
Comment at: mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp:1419
+
+  virtual void emitOpSpecificLLVMIR(ConversionPatternRewriter &rewriter,
+                                    Location loc, Value lhsReal, Value lhsImag,
----------------
I wonder if it wouldn't be better to just have a helper function that returns something like

```
struct PairOfComplex {
  Value lhsReal, lhsImag, rhsReal, rhsImag;
};
template <typename OpTy> PairOfComplex extractComplexComponents(OpTy op);
```
that you would call from two individual patterns. Feels like it would be less code and less concepts in the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79479