[flang][codegen] Add a conversion for `fir.coordinate_of` - part 2

[awarzynski]

Summary

This patch extends the FIRToLLVMLowering pass in Flang by extending
the hook to transform fir.coordinate_of into a sequence of LLVM MLIR
instructions (i.e. CoordinateOpConversion::doRewrite). The following
case is added:

3. the input object is inside !fir.ref (e.g. !fir.ref<!fir.array> or !fir.ref<!fir.derived>).

This is part of the upstreaming effort from the fir-dev branch in [1].

[1] https://github.com/flang-compiler/f18-llvm-project

Originally written by:
Co-authored-by: Jean Perier <[e-mail]>
Co-authored-by: Eric Schweitz <[e-mail]>
Co-authored-by: V Donaldson <[e-mail]>

Depends on: D114159

[clementval]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

2981

replace auto

2986

replace auto

3010

replace auto

3013

replace auto

3132

3152

3155

3181

3185

auto

[clementval]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

2943

Remove brace on simple single statement

2947

Remove brace on simple single statement

3010

3011

3157

3157

Maybe adding braces here would help readability.

3184

3218

Remove braces

[awarzynski]

Address auto and formatting comments

[clementval]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

2981

we use std::size_t in other places. Can you add it here as well and in other places?

2982

[awarzynski]

Replace size_t with std::size_t

[rovka]

Love the tests! I think the code allows more cases though, e.g. array of array, array inside derived etc. I think we should have lots of mixed testcases like that, since the code is particularly hard to follow :-/ Or if we don't support that we should consider erroring out early (sorry if we already do and I just missed it).

flang/lib/Optimizer/CodeGen/CodeGen.cpp

2980

Why is this here and not in the verifier? Or is this just the subset of coordinate stuff that's supported at the moment in the CodeGen? If so, then maybe we should call it supportedCoordinate instead and add a comment that we intend to support more in the future (and that eventually the whole function should disappear once everything is supported).

3003

The i >= sz part is strange. Unless I'm missing something, that can only happen in the case of a sequential type, but the docs say "When computing the coordinate of an array element, the rank of the array must be known and the number of indexing expressions must equal the rank of the array". IIUC that means i has to be exactly equal to sz, no?

3009

Same as above, shouldn't this be in the verify function? (There seems to be something along these lines, but it doesn't look very complete)

3134

Nit: Should we also support mlir::TupleType, like we do in hasSubDimensions?

3139

Nit: You could just have bool columnIsDeferred = !hasSubdimension;

3142

If it's incorrect, then it should be an error rather than a TODO. If it's just unsupported yet, then I think the message should say that instead :)

3144

Nit: This comment is superfluous (and ungrammatical), I think you can skip it. hasKnownShape is clear enough imo.

3188

Is this really a TODO? Sounds more like an error...

3234

Error?

flang/test/Fir/convert-to-llvm.fir

2457

Nit: I think it would be worthwhile to introduce a %arg2 here

2469

Nit: I think you can skip the definition for derived_4 on the other 2 lines.

[awarzynski]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

2980

Why is this here and not in the verifier? Or is this just the subset of coordinate stuff that's supported at the moment in the CodeGen?

If we move this to the verifier then we will effectively change the semantics of !fir.coordinate_of. That's something that should probably be discussed first. I did experiment with this idea and saw quite a few new regressions, so clearly other parts of the compiler do generate code that this method deems as unsupported.

IIUC, the purpose of this method is to capture cases not supported by this conversion rather then to refine the semantics of !fir.coordinate_of. I will rename it and add some comments to clarify this. I do feel that some cases captured here are genuinely invalid and should indeed be rejected by the verifier, but we'd probably have to do on a case-by-case basis. How about a TODO for now?

3003

The i >= sz part is strange. Unless I'm missing something, that can only happen in the case of a sequential type, but the docs say "When computing the coordinate of an array element, the rank of the array must be known and the number of indexing expressions must equal the rank of the array". IIUC that means i has to be exactly equal to sz, no?

Thanks for the link! Yeah, I agree that this is confusing. See this example. It's inconsistent with the documentation that you referred to in your comment. Perhaps the docs should be updated to read "(...) must not be greater than the rank of the array." instead of "(...) must equal the rank of the array.". WDTY?

3009

Hm, but both true and false might lead to a valid case. Unless I missed something.

3134

We should. In fact, this assert is not needed here. FIROps.cpp takes care of this. I will remove this.

3139

Good point, ta!

3142

Let me update the message then :)

3144

Indeed, thanks!

[awarzynski]

• Renamed validCoordinate as supportedCoordinate (also added comments, renamed variables)
• Added a test for fir.char and tuple
• Added tests for TODOs and diagnostics
• Removed an assert that was duplicating a similar assert in FIROps.cpp

[awarzynski]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

3188

Agree. I was trying to come up with an example that would get me here, but all of my invalid examples are detected earlier on.

3234

Probably. I was trying to come up with an example that would get me here, but all of my invalid examples are detected earlier on.

[awarzynski]

• Replace a couple of TODOs with an error
• Make sure that !fir.ptr is supported too (alongside !fir.ref)

[rovka]

In D115333#3195069, @awarzynski wrote:

• Replace a couple of TODOs with an error
• Make sure that !fir.ptr is supported too (alongside !fir.ref)

Is there a test for fir.ptr?

Other than that, LGTM (with microscopic nits).

flang/lib/Optimizer/CodeGen/CodeGen.cpp

2949

Nit: I'm not 100% sure, but can you use baseObjectTy.isa<fir::ReferenceType, fir::PointerType>()?

2977

Same as above.

3003

Ok, I was not aware of that example :) Let's update the docs then.
Also, thanks for the sz->numOfCoors, that's more readable.

3009

Yes, sorry, big patch, I think I had paged out some parts of it by the time I was done reviewing :D

flang/test/Fir/convert-to-llvm.fir

2511

[awarzynski]

Address the final comments from @rovka. Renamed doRewriteRef as doRewriteRefOrPtr.

[awarzynski]

In D115333#3197304, @rovka wrote:

Is there a test for fir.ptr?

Added in the latest update :) As fir.ptr and fir.ref are practically identical, I've only added one sanity-test for fir.ptr (to avoid duplication). If there are no further comments, I'll merge later this week.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

2949

That's a neat trick, thanks!

flang/lib/Optimizer/CodeGen/CodeGen.cpp

if (fir::isa_complex(objectTy)) {
       mlir::LLVM::ConstantOp c0 =
           genConstantIndex(loc, lowerTy().indexType(), rewriter, 0);
       SmallVector<mlir::Value> offs = {c0, operands[1]};
       mlir::Value gep = genGEP(loc, ty, rewriter, base, offs);
       rewriter.replaceOp(coor, gep);
       return success();
     }
 
-    // Box type - get the base pointer from the box
-    if (auto boxTy = baseObjectTy.dyn_cast<fir::BoxType>()) {
-      doRewriteBox(coor, ty, operands, loc, rewriter);
-      return success();
-    }
-
-    // Sequence type (e.g. fir.array)
-    if (auto arrTy = objectTy.dyn_cast<fir::SequenceType>()) {
-      doRewriteSequence(loc);
-      return success();
-    }
+    // Boxed type - get the base pointer from the box
+    if (baseObjectTy.dyn_cast<fir::BoxType>())
+      return doRewriteBox(coor, ty, operands, loc, rewriter);
+
+    // Reference or pointer type
+    if (baseObjectTy.isa<fir::ReferenceType, fir::PointerType>())
+      return doRewriteRefOrPtr(coor, ty, operands, loc, rewriter);
 
     return rewriter.notifyMatchFailure(
         coor, "fir.coordinate_of base operand has unsupported type");
   }
 
   unsigned getFieldNumber(fir::RecordType ty, mlir::Value op) const {
     return fir::hasDynamicSize(ty)
                ? op.getDefiningOp()
int64_t getIntValue(mlir::Value val) const {
     if (auto constOp = dyn_cast<mlir::arith::ConstantIntOp>(defop))
       return constOp.value();
     if (auto llConstOp = dyn_cast<mlir::LLVM::ConstantOp>(defop))
       if (auto attr = llConstOp.getValue().dyn_cast<mlir::IntegerAttr>())
         return attr.getValue().getSExtValue();
     fir::emitFatalError(val.getLoc(), "must be a constant");
   }
 
+  bool hasSubDimensions(mlir::Type type) const {
+    return type.isa<fir::SequenceType, fir::RecordType, mlir::TupleType>();
+  }
+
+  /// Check whether this form of `!fir.coordinate_of` is supported. These
+  /// additional checks are required, because we are not yet able to convert
+  /// all valid forms of `!fir.coordinate_of`.
+  /// TODO: Either implement the unsupported cases or extend the verifier
+  /// in FIROps.cpp instead.
+  bool supportedCoordinate(mlir::Type type, mlir::ValueRange coors) const {
+    const std::size_t numOfCoors = coors.size();
+    std::size_t i = 0;
+    bool subEle = false;
+    bool ptrEle = false;
+    for (; i < numOfCoors; ++i) {
+      mlir::Value nxtOpnd = coors[i];
+      if (auto arrTy = type.dyn_cast<fir::SequenceType>()) {
+        subEle = true;
+        i += arrTy.getDimension() - 1;
+        type = arrTy.getEleTy();
+      } else if (auto recTy = type.dyn_cast<fir::RecordType>()) {
+        subEle = true;
+        type = recTy.getType(getFieldNumber(recTy, nxtOpnd));
+      } else if (auto tupTy = type.dyn_cast<mlir::TupleType>()) {
+        subEle = true;
+        type = tupTy.getType(getIntValue(nxtOpnd));
+      } else {
+        ptrEle = true;
+      }
+    }
+    if (ptrEle)
+      return (!subEle) && (numOfCoors == 1);
+    return subEle && (i >= numOfCoors);
+  }
+
+  /// Walk the abstract memory layout and determine if the path traverses any
+  /// array types with unknown shape. Return true iff all the array types have a
+  /// constant shape along the path.
+  bool arraysHaveKnownShape(mlir::Type type, mlir::ValueRange coors) const {
+    const std::size_t sz = coors.size();
+    std::size_t i = 0;
+    for (; i < sz; ++i) {
+      mlir::Value nxtOpnd = coors[i];
+      if (auto arrTy = type.dyn_cast<fir::SequenceType>()) {
+        if (fir::sequenceWithNonConstantShape(arrTy))
+          return false;
+        i += arrTy.getDimension() - 1;
+        type = arrTy.getEleTy();
+      } else if (auto strTy = type.dyn_cast<fir::RecordType>()) {
+        type = strTy.getType(getFieldNumber(strTy, nxtOpnd));
+      } else if (auto strTy = type.dyn_cast<mlir::TupleType>()) {
+        type = strTy.getType(getIntValue(nxtOpnd));
+      } else {
+        return true;
+      }
+    }
+    return true;
+  }
+
 private:
-  void doRewriteBox(fir::CoordinateOp coor, mlir::Type ty,
-                    mlir::ValueRange operands, mlir::Location loc,
-                    mlir::ConversionPatternRewriter &rewriter) const {
+  mlir::LogicalResult
+  doRewriteBox(fir::CoordinateOp coor, mlir::Type ty, mlir::ValueRange operands,
+               mlir::Location loc,
     mlir::Type boxObjTy = coor.getBaseType();
     assert(boxObjTy.dyn_cast<fir::BoxType>() && "This is not a `fir.box`");
 
     mlir::Value boxBaseAddr = operands[0];
 
     // 1. SPECIAL CASE (uses `fir.len_param_index`):
     //   %box = ... : !fir.box<!fir.type<derived{len1:i32}>>
     //   %lenp = fir.len_param_index len1, !fir.type<derived{len1:i32}>
for (unsigned i = 1, last = operands.size(); i < last; ++i) {
         resultAddr =
             rewriter.create<mlir::LLVM::BitcastOp>(loc, voidPtrTy, gep);
       } else {
         fir::emitFatalError(loc, "unexpected type in coordinate_of");
       }
     }
 
     rewriter.replaceOpWithNewOp<mlir::LLVM::BitcastOp>(coor, ty, resultAddr);
-    return;
+    return success();
+  }
+
+  mlir::LogicalResult
+  doRewriteRefOrPtr(fir::CoordinateOp coor, mlir::Type ty,
+                    mlir::ValueRange operands, mlir::Location loc,
+                    mlir::ConversionPatternRewriter &rewriter) const {
+    mlir::Type baseObjectTy = coor.getBaseType();
+
+    mlir::Type currentObjTy = fir::dyn_cast_ptrOrBoxEleTy(baseObjectTy);
+    bool hasSubdimension = hasSubDimensions(currentObjTy);
+    bool columnIsDeferred = !hasSubdimension;
+
+    if (!supportedCoordinate(currentObjTy, operands.drop_front(1))) {
+      TODO(loc, "unsupported combination of coordinate operands");
     }
 
-  void doRewriteSequence(mlir::Location loc) const {
-    TODO(loc, "fir.coordinate_of codegen for sequence types");
+    const bool hasKnownShape =
+        arraysHaveKnownShape(currentObjTy, operands.drop_front(1));
+
+    // If only the column is `?`, then we can simply place the column value in
+    // the 0-th GEP position.
+    if (auto arrTy = currentObjTy.dyn_cast<fir::SequenceType>()) {
+      if (!hasKnownShape) {
+        const unsigned sz = arrTy.getDimension();
+        if (arraysHaveKnownShape(arrTy.getEleTy(),
+                                 operands.drop_front(1 + sz))) {
+          llvm::ArrayRef<int64_t> shape = arrTy.getShape();
+          bool allConst = true;
+          for (unsigned i = 0; i < sz - 1; ++i) {
+            if (shape[i] < 0) {
+              allConst = false;
+              break;
+            }
+          }
+          if (allConst)
+            columnIsDeferred = true;
+        }
+      }
+    }
+
+    if (fir::hasDynamicSize(fir::unwrapSequenceType(currentObjTy))) {
+      mlir::emitError(
+          loc, "fir.coordinate_of with a dynamic element size is unsupported");
+      return failure();
+    }
+
+    if (hasKnownShape || columnIsDeferred) {
+      SmallVector<mlir::Value> offs;
+      if (hasKnownShape && hasSubdimension) {
+        mlir::LLVM::ConstantOp c0 =
+            genConstantIndex(loc, lowerTy().indexType(), rewriter, 0);
+        offs.push_back(c0);
+      }
+      const std::size_t sz = operands.size();
+      Optional<int> dims;
+      SmallVector<mlir::Value> arrIdx;
+      for (std::size_t i = 1; i < sz; ++i) {
+        mlir::Value nxtOpnd = operands[i];
+
+        if (!currentObjTy) {
+          mlir::emitError(loc, "invalid coordinate/check failed");
+          return failure();
+        }
+
+        // check if the i-th coordinate relates to an array
+        if (dims.hasValue()) {
+          arrIdx.push_back(nxtOpnd);
+          int dimsLeft = *dims;
+          if (dimsLeft > 1) {
+            dims = dimsLeft - 1;
+            continue;
+          }
+          currentObjTy = currentObjTy.cast<fir::SequenceType>().getEleTy();
+          // append array range in reverse (FIR arrays are column-major)
+          offs.append(arrIdx.rbegin(), arrIdx.rend());
+          arrIdx.clear();
+          dims.reset();
+          continue;
+        }
+        if (auto arrTy = currentObjTy.dyn_cast<fir::SequenceType>()) {
+          int d = arrTy.getDimension() - 1;
+          if (d > 0) {
+            dims = d;
+            arrIdx.push_back(nxtOpnd);
+            continue;
+          }
+          currentObjTy = currentObjTy.cast<fir::SequenceType>().getEleTy();
+          offs.push_back(nxtOpnd);
+          continue;
+        }
+
+        // check if the i-th coordinate relates to a field
+        if (auto recTy = currentObjTy.dyn_cast<fir::RecordType>())
+          currentObjTy = recTy.getType(getFieldNumber(recTy, nxtOpnd));
+        else if (auto tupTy = currentObjTy.dyn_cast<mlir::TupleType>())
+          currentObjTy = tupTy.getType(getIntValue(nxtOpnd));
+        else
+          currentObjTy = nullptr;
+
+        offs.push_back(nxtOpnd);
+      }
+      if (dims.hasValue())
+        offs.append(arrIdx.rbegin(), arrIdx.rend());
+      mlir::Value base = operands[0];
+      mlir::Value retval = genGEP(loc, ty, rewriter, base, offs);
+      rewriter.replaceOp(coor, retval);
+      return success();
+    }
+
+    mlir::emitError(loc, "fir.coordinate_of base operand has unsupported type");
+    return failure();
   }
 };
 
 } // namespace
 
 namespace {
 /// Convert FIR dialect to LLVM dialect
 ///

flang/test/Fir/Todo/cordinate_of_5.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// CHECK: unsupported combination of coordinate operands
+func @test_coordinate_of(%arr : !fir.ref<!fir.array<2 x !fir.char<10, 2>>>, %arg1: index) {
+  %1 = arith.constant 10 : i32
+  %2 = fir.coordinate_of %arr, %arg1, %1 : (!fir.ref<!fir.array<2 x !fir.char<10, 2>>>, index, i32) -> !fir.ref<!fir.char<1,10>>
+  return
+}

flang/test/Fir/Todo/cordinate_of_6.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// CHECK: unsupported combination of coordinate operands
+
+func @test_coordinate_of(%arr : !fir.ref<!fir.array<2 x i32>>, %arg1: index) {
+  %2 = fir.coordinate_of %arr, %arg1, %arg1 : (!fir.ref<!fir.array<2 x i32>>, index, index) -> !fir.ref<i32>
+  return
+}

flang/test/Fir/convert-to-llvm-invalid.fir

 ^bb1(%a : i32) :
   return %a : i32
 ^bb2(%b : i32) :
   return %b : i32
 ^bb5 :
   %zero = arith.constant 0 : i32
   return %zero : i32
 }
+
+// -----
+
+// `fir.coordinate_of` - dynamically sized arrays are not supported
+func @coordinate_of_dynamic_array(%arg0: !fir.ref<!fir.array<1x!fir.char<4,?>>>, %arg1: index) {
+  // expected-error@+2{{fir.coordinate_of with a dynamic element size is unsupported}}
+  // expected-error@+1{{failed to legalize operation 'fir.coordinate_of'}}
+  %p = fir.coordinate_of %arg0, %arg1 : (!fir.ref<!fir.array<1x!fir.char<4,?>>>, index) -> !fir.ref<f32>
+  return
+}

flang/test/Fir/convert-to-llvm.fir

 //CHECK:   %[[RESULT_BOX:.*]] = llvm.insertvalue %[[RESULT_PTR_CAST]], %[[RBOX_TMP7_3]][0 : i32] : !llvm.struct<(ptr<i8>, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>
 //CHECK:   llvm.store %[[RESULT_BOX]], %[[RESULT_BOX_REF]] : !llvm.ptr<struct<(ptr<i8>, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>>
 //CHECK:   llvm.call @bar(%[[RESULT_BOX_REF]]) : (!llvm.ptr<struct<(ptr<i8>, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>>) -> ()
 //CHECK:   llvm.return
 //CHECK: }
 
 // -----
 
-// Test `fir.coordinate_of` conversion
+// Test `fir.coordinate_of` conversion (items inside `!fir.box`)
 
 // 1. COMPLEX TYPE (`fir.complex` is a special case)
 // Complex type wrapped in `fir.ref`
 func @coordinate_ref_complex(%arg0: !fir.ref<!fir.complex<16>>, %arg1: index) {
   %p = fir.coordinate_of %arg0, %arg1 : (!fir.ref<!fir.complex<16>>, index) -> !fir.ref<f32>
   return
 }
 // CHECK-LABEL: llvm.func @coordinate_ref_complex

 // CHECK-SAME:  %[[BOX:.*]]: !llvm.ptr<struct<(ptr<struct<(f128, f128)>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}})>>
 // CHECK-SAME:  %[[COORDINATE:.*]]: i64) {
 // CHECK:    %[[C0:.*]] = llvm.mlir.constant(0 : i64) : i64
 // CHECK:    %{{.*}} = llvm.getelementptr %[[BOX]][%[[C0]], %[[COORDINATE]]] : (!llvm.ptr<struct<(ptr<struct<(f128, f128)>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}})>>, i64, i64) -> !llvm.ptr<f32>
 // CHECK-NEXT:    llvm.return
 
 // -----
 
-// Test `fir.coordinate_of` conversion
+// Test `fir.coordinate_of` conversion (items inside `!fir.box`)
 
 // 2. BOX TYPE (objects wrapped in `fir.box`)
 // Derived type - basic case (1 index)
 func @coordinate_box_derived_1(%arg0: !fir.box<!fir.type<derived_1{field_1:i32, field_2:i32}>>) {
   %idx = fir.field_index field_2, !fir.type<derived_1{field_1:i32, field_2:i32}>
   %q = fir.coordinate_of %arg0, %idx : (!fir.box<!fir.type<derived_1{field_1:i32, field_2:i32}>>, !fir.field) -> !fir.ref<i32>
   return
 }

 // CHECK-NEXT:    %[[SUBOBJECT_AS_VOID_PTR:.*]] = llvm.bitcast %[[SUBOBJECT_ADDR]] : !llvm.ptr<f32> to !llvm.ptr<i8>
 // CHECK-NEXT:    %{{.*}} = llvm.bitcast %[[SUBOBJECT_AS_VOID_PTR]] : !llvm.ptr<i8> to !llvm.ptr<i32>
 // CHECK-NEXT:   llvm.return
 
 // TODO: Derived type - special case with `fir.len_param_index`
 
 // -----
 
-// Test `fir.coordinate_of` conversion
+// Test `fir.coordinate_of` conversion (items inside `!fir.box`)
 
 // 3. BOX TYPE - `fir.array` wrapped in `fir.box`
 // `fir.array` inside a `fir.box` (1d)
 func @coordinate_box_array_1d(%arg0: !fir.box<!fir.array<10 x f32>>, %arg1: index) {
   %p = fir.coordinate_of %arg0, %arg1 : (!fir.box<!fir.array<10 x f32>>, index) -> !fir.ref<f32>
   return
 }
 // CHECK-LABEL: llvm.func @coordinate_box_array_1d

 // CHECK-NEXT:  %[[SUBOBJECT_OFFSET_2:.*]] = llvm.add %[[BYTE_OFFSET_2]], %[[SUBOBJECT_OFFSET_1]]  : i64
 // CHECK-NEXT:  %[[ARRAY_OBJECT_AS_VOID_PTR:.*]] = llvm.bitcast %[[ARRAY_OBJECT]] : !llvm.ptr<array<10 x array<10 x f32>>> to !llvm.ptr<i8>
 // CHECK-NEXT:  %[[SUBOBJECT_ADDR:.*]] = llvm.getelementptr %[[ARRAY_OBJECT_AS_VOID_PTR]][%[[SUBOBJECT_OFFSET_2]]] : (!llvm.ptr<i8>, i64) -> !llvm.ptr<i8>
 // CHECK-NEXT:  %[[RETURN_VAL:.*]] = llvm.bitcast %[[SUBOBJECT_ADDR]] : !llvm.ptr<i8> to !llvm.ptr<f32>
 // CHECK-NEXT:  llvm.return
 
 // -----
 
-// Test `fir.coordinate_of` conversion
+// Test `fir.coordinate_of` conversion (items inside `!fir.box`)
 
 // 4. BOX TYPE - `fir.derived` inside `fir.array`
 func @coordinate_box_derived_inside_array(%arg0: !fir.box<!fir.array<10 x !fir.type<derived_3{field_1:f32, field_2:f32}>>>, %arg1 : index) {
    %idx0 = fir.field_index field_2, !fir.type<derived_3{field_1:f32, field_2:f32}>
    %q = fir.coordinate_of %arg0, %arg1, %idx0 : (!fir.box<!fir.array<10 x !fir.type<derived_3{field_1:f32, field_2:f32}>>>, index, !fir.field) -> !fir.ref<f32>
    return
 }
 // CHECK-LABEL:   llvm.func @coordinate_box_derived_inside_array(

 // CHECK:         %[[OFFSET:.*]] = llvm.add %[[VAL_15]], %[[VAL_8]]  : i64
 // CHECK:         %[[VAL_17:.*]] = llvm.bitcast %[[ARRAY]] : !llvm.ptr<array<10 x struct<"derived_3", (f32, f32)>>> to !llvm.ptr<i8>
 // CHECK:         %[[VAL_18:.*]] = llvm.getelementptr %[[VAL_17]][%[[OFFSET]]] : (!llvm.ptr<i8>, i64) -> !llvm.ptr<i8>
 // CHECK:         %[[DERIVED:.*]] = llvm.bitcast %[[VAL_18]] : !llvm.ptr<i8> to !llvm.ptr<struct<"derived_3", (f32, f32)>>
 // CHECK:         %[[VAL_20:.*]] = llvm.getelementptr %[[DERIVED]][%[[VAL_3]], %[[COORDINATE_2]]] : (!llvm.ptr<struct<"derived_3", (f32, f32)>>, i64, i32) -> !llvm.ptr<f32>
 // CHECK:         %[[VAL_21:.*]] = llvm.bitcast %[[VAL_20]] : !llvm.ptr<f32> to !llvm.ptr<i8>
 // CHECK:         %[[VAL_22:.*]] = llvm.bitcast %[[VAL_21]] : !llvm.ptr<i8> to !llvm.ptr<f32>
 // CHECK:         llvm.return
+
+// -----
+
+// Test `fir.coordinate_of` conversion (items inside `!fir.ref`)
+
+// 5.1. `fir.array`
+func @coordinate_array_unknown_size_1d(%arg0: !fir.ref<!fir.array<? x i32>>, %arg1 : index) {
+   %q = fir.coordinate_of %arg0, %arg1 : (!fir.ref<!fir.array<? x i32>>, index) -> !fir.ref<i32>
+   return
+}
+// CHECK-LABEL:   llvm.func @coordinate_array_unknown_size_1d(
+// CHECK-SAME:    %[[VAL_0:.*]]: !llvm.ptr<i32>,
+// CHECK-SAME:    %[[VAL_1:.*]]: i64) {
+// CHECK:           %[[VAL_2:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_1]]] : (!llvm.ptr<i32>, i64) -> !llvm.ptr<i32>
+// CHECK:           llvm.return
+// CHECK:         }
+
+func @coordinate_array_known_size_1d(%arg0: !fir.ref<!fir.array<10 x i32>>, %arg1 : index) {
+   %q = fir.coordinate_of %arg0, %arg1 : (!fir.ref<!fir.array<10 x i32>>, index) -> !fir.ref<i32>
+   return
+}
+// CHECK-LABEL:   llvm.func @coordinate_array_known_size_1d(
+// CHECK-SAME:    %[[VAL_0:.*]]: !llvm.ptr<array<10 x i32>>,
+// CHECK-SAME:    %[[VAL_1:.*]]: i64) {
+// CHECK:           %[[VAL_2:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK:           %[[VAL_3:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_2]], %[[VAL_1]]] : (!llvm.ptr<array<10 x i32>>, i64, i64) -> !llvm.ptr<i32>
+// CHECK:           llvm.return
+// CHECK:         }
+
+func @coordinate_array_known_size_2d(%arg0: !fir.ref<!fir.array<10 x 10 x i32>>, %arg1 : index, %arg2 : index) {
+   %q = fir.coordinate_of %arg0, %arg1, %arg2 : (!fir.ref<!fir.array<10 x 10 x i32>>, index, index) -> !fir.ref<i32>
+   return
+}
+// CHECK-LABEL:   llvm.func @coordinate_array_known_size_2d(
+// CHECK-SAME:    %[[VAL_0:.*]]: !llvm.ptr<array<10 x array<10 x i32>>>,
+// CHECK-SAME:    %[[VAL_1:.*]]: i64,
+// CHECK-SAME:    %[[VAL_2:.*]]: i64) {
+// CHECK:           %[[VAL_3:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK:           %[[VAL_4:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_3]], %[[VAL_2]], %[[VAL_1]]] : (!llvm.ptr<array<10 x array<10 x i32>>>, i64, i64, i64) -> !llvm.ptr<i32>
+// CHECK:           llvm.return
+// CHECK:         }
+
+// 5.2. `fir.derived`
+func @coordinate_ref_derived(%arg0: !fir.ref<!fir.type<dervied_4{field_1:i32, field_2:i32}>>) {
+  %idx = fir.field_index field_2, !fir.type<dervied_4{field_1:i32, field_2:i32}>
+  %q = fir.coordinate_of %arg0, %idx : (!fir.ref<!fir.type<dervied_4{field_1:i32, field_2:i32}>>, !fir.field) -> !fir.ref<i32>
+  return
+}
+// CHECK-LABEL:   llvm.func @coordinate_ref_derived(
+// CHECK-SAME:    %[[VAL_0:.*]]: !llvm.ptr<struct<"dervied_4", (i32, i32)>>) {
+// CHECK:           %[[VAL_1:.*]] = llvm.mlir.constant(1 : i32) : i32
+// CHECK:           %[[VAL_2:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK:           %[[VAL_3:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_2]], %[[VAL_1]]] : (!llvm.ptr<struct<"dervied_4", (i32, i32)>>, i64, i32) -> !llvm.ptr<i32>
+// CHECK:           llvm.return
+// CHECK:         }
+
+func @coordinate_ref_derived_nested(%arg0: !fir.ref<!fir.type<derived_5{field_1:!fir.type<nested_derived{inner1:i32, inner2:f32}>, field_2:i32}>>) {
+  %idx0 = fir.field_index field_1, !fir.type<derived_5{field_1:!fir.type<nested_derived{inner1:i32, inner2:f32}>, field_2:i32}>
+  %idx1 = fir.field_index inner2, !fir.type<nested_derived{inner1:i32, inner2:f32}>
+  %q = fir.coordinate_of %arg0, %idx0, %idx1 : (!fir.ref<!fir.type<derived_5{field_1:!fir.type<nested_derived{inner1:i32, inner2:f32}>, field_2:i32}>>, !fir.field, !fir.field) -> !fir.ref<i32>
+  return
+}
+// CHECK-LABEL:   llvm.func @coordinate_ref_derived_nested(
+// CHECK-SAME:    %[[VAL_0:.*]]: !llvm.ptr<struct<"derived_5", (struct<"nested_derived", (i32, f32)>, i32)>>) {
+// CHECK:           %[[VAL_1:.*]] = llvm.mlir.constant(0 : i32) : i32
+// CHECK:           %[[VAL_2:.*]] = llvm.mlir.constant(1 : i32) : i32
+// CHECK:           %[[VAL_3:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK:           %[[VAL_4:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_3]], %[[VAL_1]], %[[VAL_2]]] : (!llvm.ptr<struct<"derived_5", (struct<"nested_derived", (i32, f32)>, i32)>>, i64, i32, i32) -> !llvm.ptr<i32>
+// CHECK:           llvm.return
+// CHECK:         }
+
+// 5.3 `fir.char`
+func @test_coordinate_of_char(%arr : !fir.ref<!fir.char<10, 2>>) {
+  %1 = arith.constant 10 : i32
+  %2 = fir.coordinate_of %arr, %1 : (!fir.ref<!fir.char<10, 2>>, i32) -> !fir.ref<i80>
+  return
+}
+// CHECK-LABEL:   llvm.func @test_coordinate_of_char(
+// CHECK-SAME:    %[[VAL_0:.*]]: !llvm.ptr<array<2 x i80>>) {
+// CHECK:           %[[VAL_1:.*]] = llvm.mlir.constant(10 : i32) : i32
+// CHECK:           %[[VAL_2:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_1]]] : (!llvm.ptr<array<2 x i80>>, i32) -> !llvm.ptr<i80>
+// CHECK:           llvm.return
+// CHECK:         }
+
+// 5.4 `mlir.tuple`
+func @test_coordinate_of_tuple(%tup : !fir.ref<tuple<!fir.ref<i32>>>) {
+  %1 = arith.constant 0 : i64
+  %2 = fir.coordinate_of %tup, %1 : (!fir.ref<tuple<!fir.ref<i32>>>, i64) -> !fir.ref<i32>
+  return
+}
+// CHECK-LABEL:   llvm.func @test_coordinate_of_tuple(
+// CHECK-SAME:                                     %[[VAL_0:.*]]: !llvm.ptr<struct<(ptr<i32>)>>) {
+// CHECK:           %[[VAL_1:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK:           %[[VAL_2:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK:           %[[VAL_3:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_2]], %[[VAL_1]]] : (!llvm.ptr<struct<(ptr<i32>)>>, i64, i64) -> !llvm.ptr<i32>
+// CHECK:           llvm.return
+// CHECK:         }
+
+// -----
+
+// Test `fir.coordinate_of` conversion - items inside `!fir.ptr`. This should
+// be almost identical to `!fir.ref` (i.e. it's the same code path in the code
+// gen). Instead of duplicating the tests, only one for sanity-checking is added.
+
+// 6.1. `fir.array`
+func @coordinate_array_unknown_size_1d(%arg0: !fir.ptr<!fir.array<? x i32>>, %arg1 : index) {
+   %q = fir.coordinate_of %arg0, %arg1 : (!fir.ptr<!fir.array<? x i32>>, index) -> !fir.ref<i32>
+   return
+}
+// CHECK-LABEL:   llvm.func @coordinate_array_unknown_size_1d(
+// CHECK-SAME:    %[[VAL_0:.*]]: !llvm.ptr<i32>,
+// CHECK-SAME:    %[[VAL_1:.*]]: i64) {
+// CHECK:           %[[VAL_2:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_1]]] : (!llvm.ptr<i32>, i64) -> !llvm.ptr<i32>
+// CHECK:           llvm.return
+// CHECK:         }

flang/test/Fir/invalid.fir

func @test_coordinate_of(%arr : !fir.ref<!fir.char<10>>) {
   %1 = arith.constant 10 : i32
   // expected-error@+1 {{'fir.coordinate_of' op cannot apply coordinate_of to this type}}
   %2 = fir.coordinate_of %arr, %1 : (!fir.ref<!fir.char<10>>, i32) -> !fir.ref<f32>
   return
 }
 
 // -----
 
+func @test_coordinate_of(%arr : !fir.ref<!fir.char<10, 1>>) {
+  %1 = arith.constant 10 : i32
+  // expected-error@+1 {{'fir.coordinate_of' op cannot apply coordinate_of to this type}}
+  %2 = fir.coordinate_of %arr, %1 : (!fir.ref<!fir.char<10, 1>>, i32) -> !fir.ref<f32>
+  return
+}
+
+// -----
+
 %0 = arith.constant 22 : i32
 // expected-error@+1 {{'fir.embox' op operand #0 must be any reference, but got 'i32'}}
 %1 = fir.embox %0 : (i32) -> !fir.box<i32>
 
 // -----
 
 func @fun(%0 : !fir.ref<i32>) {
   %c_100 = arith.constant 100 : index