[PATCH] D114159: [flang][codegen] Add a conversion for `fir.coordinate_of` - part 1

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added inline comments.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:26
 #include "llvm/ADT/ArrayRef.h"
+#include <mlir/IR/OperationSupport.h>
 
----------------
clementval wrote:
> Is this include needed? Can you use `"`?
> Is this include needed?

Yes, one of my plugins adds these whenever I switch from `auto` to a specific type. But this will be included by some other file anyway. I'll just remove it.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:201
+  static mlir::Type getBaseAddrTypeFromBox(mlir::Type type) {
+    return getBoxEleTy(type, {0});
+  }
----------------
clementval wrote:
> Replace `0` with `kAddrPosInBox`
Thanks!


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:238-240
+    auto c7 = genConstantOffset(loc, rewriter, 7);
+    auto dimValue = genConstantIndex(loc, idxTy, rewriter, dim);
+    return loadFromOffset(loc, box, c0, c7, dimValue, 2, idxTy, rewriter);
----------------
clementval wrote:
> 
Thanks!


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:2125
+    if (fir::hasDynamicSize(fir::unwrapSequenceType(objectTy)))
+      return mlir::emitError(
+          loc,
----------------
kiranchandramohan wrote:
> Nit: use NotifyMatchfailure
Makes sense, but then we won't be able to check for this diagnostic in "convert-to-llvm-invalid.fir". I'm starting to think that we should `--debug` there. @clementval , what are your thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114159