[PATCH] D114125: [Flang] Add a factory class for creating Complex Ops

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan marked 2 inline comments as done.
kiranchandramohan added inline comments.


================
Comment at: flang/include/flang/Optimizer/Builder/Complex.h:43
+
+  mlir::Value extractComplexPart(mlir::Value cplx, bool isImagPart) {
+    return isImagPart ? extract<Part::Imag>(cplx) : extract<Part::Real>(cplx);
----------------
rovka wrote:
> Nit: Why not pass the Part directly, instead of a bool?
I think this is a convenience function for external users. i.e they don't have to know about Part.


================
Comment at: flang/unittests/Optimizer/Builder/ComplexTest.cpp:88
+  EXPECT_TRUE(fir::isa_real(helper->getComplexPartType(cVal4)));
+}
+
----------------
rovka wrote:
> Can you add another test case to check the values that are inserted and extracted? (I.e. not just the types).
I think since there are insertsOps and ExtractOps, it will be difficult to do that in the unittest. If I am missing a point, please let me know.

IR testing will come along with lowering later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114125