[flang-commits] [flang] [FIRToMemRef] Fix wrong indexing for converted array_coor over sliced fir.embox (PR #207749)

[Kareem Ergawy via flang-commits]

================
@@ -814,13 +912,52 @@ FIRToMemRef::convertArrayCoorOp(Operation *memOp, fir::ArrayCoorOp arrayCoorOp,
   } else {
     Value oneIdx =
         arith::ConstantIndexOp::create(rewriter, arrayCoorOp->getLoc(), 1);
+    // shapeVec is populated by collectSliceInfoFrom in the order:
+    //   [<arrayCoor's shape>, <embox/rebox's shape>]
+    // When both contribute (firMemrefIsEmbox && arrayCoorOp has a slice), the
+    // first `rank` entries are the *box's* shape (= slice extents) while the
+    // next `rank` are the underlying *parent's* extents. Strides for the
+    // reinterpret_cast must come from the parent's contiguous element strides,
+    // not the slice extents (using the latter yields an outer stride that
+    // walks by the slice's size instead of the parent's leading dim).
+    //
+    // Example -- 4x2 parent, slice a(1:2, :) keeping 2 elements per dim:
+    //   %parent : !fir.ref<!fir.array<4x2xi32>>
+    //   %shape  = fir.shape %c4, %c2
+    //   %eslc   = fir.slice %c1, %c2, %c1, %c1, %c2, %c1    ; a(1:2, :)
+    //   %box    = fir.embox %parent(%shape) [%eslc]
+    //                                             :
+    //                                             !fir.box<!fir.array<2x2xi32>>
+    //   ...  %addr = fir.array_coor %box(...) [%innerSlc] %i, %j
+    //
+    // After collectSliceInfoFrom is called on both the array_coor and the
+    // embox, shapeVec becomes (four entries, Fortran order):
+    //   [ box_dim0_extent(=2), box_dim1_extent(=2),         ; array_coor shape
+    //     parent_dim0_extent(=4), parent_dim1_extent(=2) ]  ; embox shape
+    //
+    // For rank = 2 the loop below emits (memref order, outer -> inner):
+    //   size[outer]   = shapeVec[1]                       = 2  (slice's dim1)
+    //   stride[outer] = shapeVec[parentShapeBase + 0]     = 4  (parent's dim0)
+    //   size[inner]   = shapeVec[0]                       = 2  (slice's dim0)
+    //   stride[inner] = 1
+    //
+    // Without the `parentShapeBase` shift, `stride[outer]` would be
+    // `shapeVec[0] = 2` -- the slice's own extent -- and successive outer
+    // steps would walk by 2 elements instead of the parent's 4-element row,
+    // clobbering the wrong columns.
+    const bool hasParentShape = firMemrefIsEmbox && arrayCoorOp.getSlice() &&
+                                shapeVec.size() >= 2 * rank;
----------------
ergawy wrote:

Yet another bug you uncovered. I should have used `boxRank + arrayCoordRank` not `rank * 2`. And, variable naming was a bit confusing so it is hopefully better now. Added a test.

https://github.com/llvm/llvm-project/pull/207749