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

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski marked an inline comment as done.
awarzynski added inline comments.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:2291
+    //   %resultAddr = coordinate_of %box, %idx1, %idx2 : !fir.ref<f32>
+    // 2.4 (`fir.derived` inside `fir.array`)
+    mlir::LLVM::ConstantOp c0 =
----------------
rovka wrote:
> Are these really the only 4 cases? Is it possible to have any combination of arrays/deriveds inside of arrays/deriveds, e.g. fir.array<M x fir.type<d1{field1:fir.array<N x fir.array <10 x i32>>, field_2:fir.type<d2{i32, i32}>}>>? Maybe a more generic sentence would work better here
Let me just mix 2.3 and 2.4 then.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:2311
+          mlir::Value stride =
+              loadStrideFromBox(loc, operands[0], index - i, rewriter);
+          auto sc = rewriter.create<mlir::LLVM::MulOp>(
----------------
rovka wrote:
> So, IIUC, if you have coordinate_of on a fir.array<M x N x fir.array<P x i32>> with operands idx1, idx2, idx3, then you end up multiplying idx1 * **stride(operands[0], 0)**, then idx2 * stride(operands[0], 1), then idx3 * **stride(operands[0], 0)**. That sounds like you expect the same stride for the inner array as for the outer array. That doesn't seem right. Am I missing something?
> 
> Also, is it even possible to have an inner fir.array without a box? Or would you actually have fir.array<M x N x fir.box<fir.array<P x i32>>? 
> 
> That doesn't seem right. Am I missing something?
> 

I'll double-check with @schweitz. IIUC, it's either a bug or an indication that this `fir.box` contains one of: `!fir.array<!fir.derived<>>` or `!fir.derived<>`.

> Also, is it even possible to have an inner fir.array without a box? Or would you actually have fir.array<M x N x fir.box<fir.array<P x i32>>? 
> 

Well, there's `fir::emitFatalError(loc, "unexpected type in coordinate_of");` towards the end of this method which is hit if the requested element inside the original `fir.array` or `fir.type` is neither ... `fir.array` nor `fir.type`.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:2312
+              loadStrideFromBox(loc, operands[0], index - i, rewriter);
+          auto sc = rewriter.create<mlir::LLVM::MulOp>(
+              loc, idxTy, operands[index], stride);
----------------
clementval wrote:
> Can you fix the clang-format here?
Yup, sorry about that! Still haven't figured why `arc lint` does nothing for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114159