[flang] Fix XArrayCoorOp conversion for index type slices

[peixin]

Summary

The previous XArrayCoorOp conversion forgot to change getting the
operands from OpAdaptor for upper bound and step of slice. This leads to
the fail of incompatible of types of codegen when slices are index type.

[peixin]

@jeanPerier @schweitz When do you plan to upstream the IO runtime functions to the libraries?

$ cat case.f90 
program m
 integer :: i = 1
 print *, i
end
$ flang-new case.f90 && ./a.out
/usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/7.3.0/../../../../lib64/Scrt1.o: in function `_start':
(.text+0x18): undefined reference to `main'
/usr/bin/ld: (.text+0x1c): undefined reference to `main'
/usr/bin/ld: /tmp/case-f0bed5.o: in function `_QQmain':
/home/..././case.f90:3: undefined reference to `_FortranAioBeginExternalListOutput'
/usr/bin/ld: /home/..././case.f90:3: undefined reference to `_FortranAioOutputInteger32'
/usr/bin/ld: /home/..././case.f90:3: undefined reference to `_FortranAioEndIoStatement'
flang-new: error: linker command failed with exit code 1 (use -v to see invocation)

Currently, I found some runtime functions not in fir-dev such as _FortranARaggedArrayAllocate. Previously, I can get llvm IR from LLVM main and link the libraies from fir-dev to have end-to-end tests for LLVM main. But now, it does not work for some cases now.

Usually, I prefer end-to-end tests. With IO support, I can start that in LLVM main. The bug in this patch is not in fir-dev, although there is another bug for XArrayCoorOp in fir-dev.

[kiranchandramohan]

In D125967#3524741, @peixin wrote:

@jeanPerier @schweitz When do you plan to upstream the IO runtime functions to the libraries?

The runtime IO libraries are upstreamed already. https://github.com/llvm/llvm-project/blob/602f81ec336330f97e22442b98035c6f007cac6d/flang/runtime/io-api.cpp#L194
Developers can use the flag -flang-experimental-exec for linking with the libraries.

$ cat case.f90 
program m
 integer :: i = 1
 print *, i
end
$ flang-new case.f90 && ./a.out
/usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/7.3.0/../../../../lib64/Scrt1.o: in function `_start':
(.text+0x18): undefined reference to `main'
/usr/bin/ld: (.text+0x1c): undefined reference to `main'
/usr/bin/ld: /tmp/case-f0bed5.o: in function `_QQmain':
/home/..././case.f90:3: undefined reference to `_FortranAioBeginExternalListOutput'
/usr/bin/ld: /home/..././case.f90:3: undefined reference to `_FortranAioOutputInteger32'
/usr/bin/ld: /home/..././case.f90:3: undefined reference to `_FortranAioEndIoStatement'
flang-new: error: linker command failed with exit code 1 (use -v to see invocation)

Currently, I found some runtime functions not in fir-dev such as _FortranARaggedArrayAllocate. Previously, I can get llvm IR from LLVM main and link the libraies from fir-dev to have end-to-end tests for LLVM main. But now, it does not work for some cases now.

Hopefully with the flag mentioned above you can use the libraries from llvm-project/flang.

Usually, I prefer end-to-end tests. With IO support, I can start that in LLVM main. The bug in this patch is not in fir-dev, although there is another bug for XArrayCoorOp in fir-dev.

Apologies, this was probably caused while switching from operands to converted operands in the upstream code.

[peixin]

The runtime IO libraries are upstreamed already. https://github.com/llvm/llvm-project/blob/602f81ec336330f97e22442b98035c6f007cac6d/flang/runtime/io-api.cpp#L194
Developers can use the flag -flang-experimental-exec for linking with the libraries.

Hopefully with the flag mentioned above you can use the libraries from llvm-project/flang.

Thanks. This works.

Apologies, this was probably caused while switching from operands to converted operands in the upstream code.

Yes. That's what I also noticed. Anyway, using operands directly rather than converted operands in dir-dev is not correct.

[kiranchandramohan]

Apologies, this was probably caused while switching from operands to converted operands in the upstream code.

Yes. That's what I also noticed. Anyway, using operands directly rather than converted operands in dir-dev is not correct.

OK. I think this is just a missed case. It works in fir-dev but due to some core-MLIR change, it does not work upstream.

LGTM. Please wait for @schweitz or @jeanPerier.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

2077

Nit: are these (indexOps, shapeOps, shiftOps, sliceOps) not used anymore? Can these variables be removed?

[peixin]

Thanks @kiranchandramohan for the notice. Remove the unused code.

[peixin]

ping @jeanPerier @schweitz

[schweitz]

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

2060

Can we add some additional tests? An array of 3 dimensions?

It would be good to exercise an array of record type, fir.type, with a subcomponent path and type parameters. Especially if we're not already doing that.

[peixin]

Add two test cases.

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

2060

Yes. These two kinds of tests were missed. Add them now.

[schweitz]

Thanks for adding the tests.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

doRewrite(fir::cg::XArrayCoorOp coor, mlir::Type ty, OpAdaptor adaptor,
     mlir::Value one = genConstantIndex(loc, idxTy, rewriter, 1);
     mlir::Value prevExt = one;
     mlir::Value zero = genConstantIndex(loc, idxTy, rewriter, 0);
     mlir::Value offset = zero;
     const bool isShifted = !coor.shift().empty();
     const bool isSliced = !coor.slice().empty();
     const bool baseIsBoxed = coor.memref().getType().isa<fir::BoxType>();
 
-    auto indexOps = coor.indices().begin();
-    auto shapeOps = coor.shape().begin();
-    auto shiftOps = coor.shift().begin();
-    auto sliceOps = coor.slice().begin();
     // For each dimension of the array, generate the offset calculation.
-    for (unsigned i = 0; i < rank;
-         ++i, ++indexOps, ++shapeOps, ++shiftOps, sliceOps += 3) {
+    for (unsigned i = 0; i < rank; ++i) {
       mlir::Value index =
           integerCast(loc, rewriter, idxTy, operands[coor.indicesOffset() + i]);
       mlir::Value lb = isShifted ? integerCast(loc, rewriter, idxTy,
                                                operands[coor.shiftOffset() + i])
                                  : one;
       mlir::Value step = one;
       bool normalSlice = isSliced;
       // Compute zero based index in dimension i of the element, applying
       // potential triplets and lower bounds.
       if (isSliced) {
-        mlir::Value ub = *(sliceOps + 1);
+        mlir::Value ub = operands[coor.sliceOffset() + i + 1];
         normalSlice = !mlir::isa_and_nonnull<fir::UndefOp>(ub.getDefiningOp());
         if (normalSlice)
-          step = integerCast(loc, rewriter, idxTy, *(sliceOps + 2));
+          step = integerCast(loc, rewriter, idxTy,
+                             operands[coor.sliceOffset() + i + 2]);
       }
       auto idx = rewriter.create<mlir::LLVM::SubOp>(loc, idxTy, index, lb);
       mlir::Value diff =
           rewriter.create<mlir::LLVM::MulOp>(loc, idxTy, idx, step);
       if (normalSlice) {
         mlir::Value sliceLb =
             integerCast(loc, rewriter, idxTy, operands[coor.sliceOffset() + i]);
         auto adj = rewriter.create<mlir::LLVM::SubOp>(loc, idxTy, sliceLb, lb);

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

 // CHECK:         %[[DIFF0:.*]] = llvm.mul %[[IDX]], %[[C1]]  : i64
 // CHECK:         %[[ADJ:.*]] = llvm.sub %[[C10]], %[[C0]]  : i64
 // CHECK:         %[[DIFF1:.*]] = llvm.add %[[DIFF0]], %[[ADJ]]  : i64
 // CHECK:         %[[STRIDE:.*]] = llvm.mul %[[DIFF1]], %[[C1_1]] : i64
 // CHECK:         %[[OFFSET:.*]] = llvm.add %[[STRIDE]], %[[C0_1]]  : i64
 // CHECK:         %[[BITCAST:.*]] = llvm.bitcast %[[ARG0]] : !llvm.ptr<array<100 x i32>> to !llvm.ptr<i32>
 // CHECK:         %{{.*}} = llvm.getelementptr %[[BITCAST]][%[[OFFSET]]] : (!llvm.ptr<i32>, i64) -> !llvm.ptr<i32>
 
+// Conversion with index type shape and slice
+
+func.func @ext_array_coor5(%arg0: !fir.ref<!fir.array<?xi32>>, %idx1 : index, %idx2 : index, %idx3 : index, %idx4 : index, %idx5 : index) {
+  %1 = fircg.ext_array_coor %arg0(%idx1)[%idx2, %idx3, %idx4]<%idx5> : (!fir.ref<!fir.array<?xi32>>, index, index, index, index, index) -> !fir.ref<i32>
+  return
+}
+
+// CHECK-LABEL:   llvm.func @ext_array_coor5(
+// CHECK-SAME:        %[[VAL_0:.*]]: !llvm.ptr<i32>, %[[VAL_1:.*]]: i64, %[[VAL_2:.*]]: i64, %[[VAL_3:.*]]: i64, %[[VAL_4:.*]]: i64, %[[VAL_5:.*]]: i64) {
+// CHECK:           %[[VAL_6:.*]] = llvm.mlir.constant(1 : i64) : i64
+// CHECK:           %[[VAL_7:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK:           %[[VAL_8:.*]] = llvm.sub %[[VAL_5]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_9:.*]] = llvm.mul %[[VAL_8]], %[[VAL_4]]  : i64
+// CHECK:           %[[VAL_10:.*]] = llvm.sub %[[VAL_2]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_11:.*]] = llvm.add %[[VAL_9]], %[[VAL_10]]  : i64
+// CHECK:           %[[VAL_12:.*]] = llvm.mul %[[VAL_11]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_13:.*]] = llvm.add %[[VAL_12]], %[[VAL_7]]  : i64
+// CHECK:           %[[VAL_14:.*]] = llvm.mul %[[VAL_6]], %[[VAL_1]]  : i64
+// CHECK:           %[[VAL_15:.*]] = llvm.bitcast %[[VAL_0]] : !llvm.ptr<i32> to !llvm.ptr<i32>
+// CHECK:           %[[VAL_16:.*]] = llvm.getelementptr %[[VAL_15]][%[[VAL_13]]] : (!llvm.ptr<i32>, i64) -> !llvm.ptr<i32>
+// CHECK:         }
+
+// Conversion for 3-d array
+
+func.func @ext_array_coor6(%arg0: !fir.ref<!fir.array<?x?x?xi32>>, %idx1 : index, %idx2 : index, %idx3 : index, %idx4 : index, %idx5 : index) {
+  %1 = fircg.ext_array_coor %arg0(%idx1, %idx1, %idx1)[%idx2, %idx3, %idx4, %idx2, %idx3, %idx4, %idx2, %idx3, %idx4]<%idx5, %idx5, %idx5> : (!fir.ref<!fir.array<?x?x?xi32>>, index, index, index, index, index, index, index, index, index, index, index, index, index, index, index) -> !fir.ref<i32>
+  return
+}
+
+// CHECK-LABEL:   llvm.func @ext_array_coor6(
+// CHECK-SAME:        %[[VAL_0:.*]]: !llvm.ptr<i32>, %[[VAL_1:.*]]: i64, %[[VAL_2:.*]]: i64, %[[VAL_3:.*]]: i64, %[[VAL_4:.*]]: i64, %[[VAL_5:.*]]: i64) {
+// CHECK:           %[[VAL_6:.*]] = llvm.mlir.constant(1 : i64) : i64
+// CHECK:           %[[VAL_7:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK:           %[[VAL_8:.*]] = llvm.sub %[[VAL_5]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_9:.*]] = llvm.mul %[[VAL_8]], %[[VAL_4]]  : i64
+// CHECK:           %[[VAL_10:.*]] = llvm.sub %[[VAL_2]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_11:.*]] = llvm.add %[[VAL_9]], %[[VAL_10]]  : i64
+// CHECK:           %[[VAL_12:.*]] = llvm.mul %[[VAL_11]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_13:.*]] = llvm.add %[[VAL_12]], %[[VAL_7]]  : i64
+// CHECK:           %[[VAL_14:.*]] = llvm.mul %[[VAL_6]], %[[VAL_1]]  : i64
+// CHECK:           %[[VAL_15:.*]] = llvm.sub %[[VAL_5]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_16:.*]] = llvm.mul %[[VAL_15]], %[[VAL_2]]  : i64
+// CHECK:           %[[VAL_17:.*]] = llvm.sub %[[VAL_3]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_18:.*]] = llvm.add %[[VAL_16]], %[[VAL_17]]  : i64
+// CHECK:           %[[VAL_19:.*]] = llvm.mul %[[VAL_18]], %[[VAL_14]]  : i64
+// CHECK:           %[[VAL_20:.*]] = llvm.add %[[VAL_19]], %[[VAL_13]]  : i64
+// CHECK:           %[[VAL_21:.*]] = llvm.mul %[[VAL_14]], %[[VAL_1]]  : i64
+// CHECK:           %[[VAL_22:.*]] = llvm.sub %[[VAL_5]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_23:.*]] = llvm.mul %[[VAL_22]], %[[VAL_3]]  : i64
+// CHECK:           %[[VAL_24:.*]] = llvm.sub %[[VAL_4]], %[[VAL_6]]  : i64
+// CHECK:           %[[VAL_25:.*]] = llvm.add %[[VAL_23]], %[[VAL_24]]  : i64
+// CHECK:           %[[VAL_26:.*]] = llvm.mul %[[VAL_25]], %[[VAL_21]]  : i64
+// CHECK:           %[[VAL_27:.*]] = llvm.add %[[VAL_26]], %[[VAL_20]]  : i64
+// CHECK:           %[[VAL_28:.*]] = llvm.mul %[[VAL_21]], %[[VAL_1]]  : i64
+// CHECK:           %[[VAL_29:.*]] = llvm.bitcast %[[VAL_0]] : !llvm.ptr<i32> to !llvm.ptr<i32>
+// CHECK:           %[[VAL_30:.*]] = llvm.getelementptr %[[VAL_29]][%[[VAL_27]]] : (!llvm.ptr<i32>, i64) -> !llvm.ptr<i32>
+// CHECK:           llvm.return
+// CHECK:         }
+
+// Conversion for derived type with type param
+
+func.func @ext_array_coor_dt_slice(%arg0: !fir.ref<!fir.array<20x!fir.type<_QFtest_dt_sliceTt{i:i32,j:i32}>>>, %idx1 : index, %idx2 : index, %idx3 : index, %idx4 : index, %idx5 : index) {
+  %1 = fir.field_index i, !fir.type<_QFtest_dt_sliceTt{i:i32,j:i32}>
+  %2 = fircg.ext_array_coor %arg0(%idx1)[%idx2, %idx3, %idx4] path %1 <%idx5>: (!fir.ref<!fir.array<20x!fir.type<_QFtest_dt_sliceTt{i:i32,j:i32}>>>, index, index, index, index, !fir.field, index) -> !fir.ref<!fir.type<_QFtest_dt_sliceTt{i:i32,j:i32}>>
+  return
+}
+
+// CHECK-LABEL:   llvm.func @ext_array_coor_dt_slice(
+// CHECK-SAME:        %[[VAL_0:.*]]: !llvm.ptr<array<20 x struct<"_QFtest_dt_sliceTt", (i32, i32)>>>, %[[VAL_1:.*]]: i64, %[[VAL_2:.*]]: i64, %[[VAL_3:.*]]: i64, %[[VAL_4:.*]]: i64, %[[VAL_5:.*]]: i64) {
+// CHECK:           %[[VAL_6:.*]] = llvm.mlir.constant(0 : i32) : i32
+// CHECK:           %[[VAL_7:.*]] = llvm.mlir.constant(1 : i64) : i64
+// CHECK:           %[[VAL_8:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK:           %[[VAL_9:.*]] = llvm.sub %[[VAL_5]], %[[VAL_7]]  : i64
+// CHECK:           %[[VAL_10:.*]] = llvm.mul %[[VAL_9]], %[[VAL_4]]  : i64
+// CHECK:           %[[VAL_11:.*]] = llvm.sub %[[VAL_2]], %[[VAL_7]]  : i64
+// CHECK:           %[[VAL_12:.*]] = llvm.add %[[VAL_10]], %[[VAL_11]]  : i64
+// CHECK:           %[[VAL_13:.*]] = llvm.mul %[[VAL_12]], %[[VAL_7]]  : i64
+// CHECK:           %[[VAL_14:.*]] = llvm.add %[[VAL_13]], %[[VAL_8]]  : i64
+// CHECK:           %[[VAL_15:.*]] = llvm.mul %[[VAL_7]], %[[VAL_1]]  : i64
+// CHECK:           %[[VAL_16:.*]] = llvm.bitcast %[[VAL_0]] : !llvm.ptr<array<20 x struct<"_QFtest_dt_sliceTt", (i32, i32)>>> to !llvm.ptr<struct<"_QFtest_dt_sliceTt", (i32, i32)>>
+// CHECK:           %[[VAL_17:.*]] = llvm.getelementptr %[[VAL_16]][%[[VAL_14]], 0] : (!llvm.ptr<struct<"_QFtest_dt_sliceTt", (i32, i32)>>, i64) -> !llvm.ptr<struct<"_QFtest_dt_sliceTt", (i32, i32)>>
+// CHECK:           llvm.return
+// CHECK:         }
+
 // -----
 
 // Check `fircg.ext_rebox` conversion to LLVM IR dialect
 
 // Test applying slice on fir.box. Note that the slice is 1D where as the array is 2D.
 //   subroutine foo(x)
 //     real :: x(3:, 4:)
 //     call bar(x(5, 6:80:3))