[flang-commits] [flang] [flang] Fixed designator codegen for contiguous boxes. (PR #139003)

Slava Zakharin via flang-commits flang-commits at lists.llvm.org
Mon May 12 13:53:09 PDT 2025


https://github.com/vzakhari updated https://github.com/llvm/llvm-project/pull/139003

>From 5d3fc1fbeb088d04c1dbe863dd0ce6657bc92949 Mon Sep 17 00:00:00 2001
From: Slava Zakharin <szakharin at nvidia.com>
Date: Wed, 7 May 2025 18:07:53 -0700
Subject: [PATCH 1/2] [flang] Fixed designator codegen for contiguous boxes.

Contiguous variables represented with a box do not have
explicit shape, but it looks like the base/shape computation
was assuming that. This caused generation of raw address
fir.array_coor without the shape. This patch is needed
to fix failures hapenning with #138797.
---
 .../flang/Optimizer/Builder/HLFIRTools.h      |  6 ++
 flang/lib/Optimizer/Builder/HLFIRTools.cpp    | 30 ++++++--
 .../HLFIR/Transforms/ConvertToFIR.cpp         | 36 +++++++++-
 flang/test/HLFIR/designate-codegen.fir        | 72 +++++++++++++++++++
 4 files changed, 135 insertions(+), 9 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/HLFIRTools.h b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
index ac80873dc374f..bcba38ed8bd5d 100644
--- a/flang/include/flang/Optimizer/Builder/HLFIRTools.h
+++ b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
@@ -533,6 +533,12 @@ Entity gen1DSection(mlir::Location loc, fir::FirOpBuilder &builder,
                     mlir::ArrayRef<mlir::Value> extents,
                     mlir::ValueRange oneBasedIndices,
                     mlir::ArrayRef<mlir::Value> typeParams);
+
+/// Return explicit lower bounds from a fir.shape result.
+/// Only fir.shape, fir.shift and fir.shape_shift are currently
+/// supported as \p shape.
+llvm::SmallVector<mlir::Value> getExplicitLboundsFromShape(mlir::Value shape);
+
 } // namespace hlfir
 
 #endif // FORTRAN_OPTIMIZER_BUILDER_HLFIRTOOLS_H
diff --git a/flang/lib/Optimizer/Builder/HLFIRTools.cpp b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
index 51ea7305d3d26..752dc0cf86414 100644
--- a/flang/lib/Optimizer/Builder/HLFIRTools.cpp
+++ b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
@@ -70,10 +70,8 @@ getExplicitExtents(fir::FortranVariableOpInterface var,
   return {};
 }
 
-// Return explicit lower bounds. For pointers and allocatables, this will not
-// read the lower bounds and instead return an empty vector.
-static llvm::SmallVector<mlir::Value>
-getExplicitLboundsFromShape(mlir::Value shape) {
+llvm::SmallVector<mlir::Value>
+hlfir::getExplicitLboundsFromShape(mlir::Value shape) {
   llvm::SmallVector<mlir::Value> result;
   auto *shapeOp = shape.getDefiningOp();
   if (auto s = mlir::dyn_cast_or_null<fir::ShapeOp>(shapeOp)) {
@@ -89,10 +87,13 @@ getExplicitLboundsFromShape(mlir::Value shape) {
   }
   return result;
 }
+
+// Return explicit lower bounds. For pointers and allocatables, this will not
+// read the lower bounds and instead return an empty vector.
 static llvm::SmallVector<mlir::Value>
 getExplicitLbounds(fir::FortranVariableOpInterface var) {
   if (mlir::Value shape = var.getShape())
-    return getExplicitLboundsFromShape(shape);
+    return hlfir::getExplicitLboundsFromShape(shape);
   return {};
 }
 
@@ -753,9 +754,24 @@ std::pair<mlir::Value, mlir::Value> hlfir::genVariableFirBaseShapeAndParams(
   }
   if (entity.isScalar())
     return {fir::getBase(exv), mlir::Value{}};
+
+  // Contiguous variables that are represented with a box
+  // may require the shape to be extracted from the box (i.e. evx),
+  // because they itself may not have shape specified.
+  // This happens during late propagationg of contiguous
+  // attribute, e.g.:
+  // %9:2 = hlfir.declare %6
+  //     {fortran_attrs = #fir.var_attrs<contiguous>} :
+  //     (!fir.box<!fir.array<?x?x...>>) ->
+  //     (!fir.box<!fir.array<?x?x...>>, !fir.box<!fir.array<?x?x...>>)
+  // The extended value is an ArrayBoxValue with base being
+  // the raw address of the array.
   if (auto variableInterface = entity.getIfVariableInterface())
-    return {fir::getBase(exv),
-            asEmboxShape(loc, builder, exv, variableInterface.getShape())};
+    if (mlir::isa<fir::BaseBoxType>(fir::getBase(exv).getType()) ||
+        !mlir::isa<fir::BaseBoxType>(entity.getType()) ||
+        variableInterface.getShape())
+      return {fir::getBase(exv),
+              asEmboxShape(loc, builder, exv, variableInterface.getShape())};
   return {fir::getBase(exv), builder.createShape(loc, exv)};
 }
 
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
index 8721a895b5e05..495f11a365185 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
@@ -412,12 +412,44 @@ class DesignateOpConversion
     auto indices = designate.getIndices();
     int i = 0;
     auto attrs = designate.getIsTripletAttr();
+
+    // If the shape specifies a shift and the base is not a box,
+    // then we have to subtract the lower bounds, as long as
+    // fir.array_coor does not support non-default lower bounds
+    // for non-box accesses.
+    llvm::SmallVector<mlir::Value> lbounds;
+    if (shape && !mlir::isa<fir::BaseBoxType>(base.getType()))
+      lbounds = hlfir::getExplicitLboundsFromShape(shape);
+    std::size_t lboundIdx = 0;
     for (auto isTriplet : attrs.asArrayRef()) {
       // Coordinate of the first element are the index and triplets lower
-      // bounds
-      firstElementIndices.push_back(indices[i]);
+      // bounds.
+      mlir::Value index = indices[i];
+      if (!lbounds.empty()) {
+        assert(lboundIdx < lbounds.size() && "missing lbound");
+        mlir::Type indexType = builder.getIndexType();
+        mlir::Value one = builder.createIntegerConstant(loc, indexType, 1);
+        mlir::Value orig = builder.createConvert(loc, indexType, index);
+        mlir::Value lb =
+            builder.createConvert(loc, indexType, lbounds[lboundIdx]);
+        index = builder.create<mlir::arith::SubIOp>(loc, orig, lb);
+        index = builder.create<mlir::arith::AddIOp>(loc, index, one);
+        ++lboundIdx;
+      }
+      firstElementIndices.push_back(index);
       i = i + (isTriplet ? 3 : 1);
     }
+
+    // Remove the shift from the shape, if needed.
+    if (!lbounds.empty()) {
+      mlir::Operation *op = shape.getDefiningOp();
+      if (mlir::isa<fir::ShiftOp>(op))
+        shape = nullptr;
+      else if (auto shiftOp = mlir::dyn_cast<fir::ShapeShiftOp>(op))
+        shape = builder.create<fir::ShapeOp>(loc, shiftOp.getExtents());
+      else
+        TODO(loc, "read fir.shape to get lower bounds");
+    }
     mlir::Type originalDesignateType = designate.getResult().getType();
     const bool isVolatile = fir::isa_volatile_type(originalDesignateType);
     mlir::Type arrayCoorType = fir::ReferenceType::get(baseEleTy, isVolatile);
diff --git a/flang/test/HLFIR/designate-codegen.fir b/flang/test/HLFIR/designate-codegen.fir
index da0a1f82b516e..d3e264941264f 100644
--- a/flang/test/HLFIR/designate-codegen.fir
+++ b/flang/test/HLFIR/designate-codegen.fir
@@ -213,3 +213,75 @@ func.func @test_polymorphic_array_elt(%arg0: !fir.class<!fir.array<?x!fir.type<_
 // CHECK:           %[[VAL_5:.*]] = fir.embox %[[VAL_4]] source_box %[[VAL_2]] : (!fir.ref<!fir.type<_QMtypesTt1>>, !fir.class<!fir.array<?x!fir.type<_QMtypesTt1>>>) -> !fir.class<!fir.type<_QMtypesTt1>>
 // CHECK:           return
 // CHECK:         }
+
+// Test proper generation of fir.array_coor for contiguous box with default lbounds.
+func.func @_QPtest_contiguous_derived_default(%arg0: !fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>> {fir.bindc_name = "d1", fir.contiguous, fir.optional}) {
+  %c1 = arith.constant 1 : index
+  %c16_i32 = arith.constant 16 : i32
+  %0 = fir.dummy_scope : !fir.dscope
+  %1:2 = hlfir.declare %arg0 dummy_scope %0 {fortran_attrs = #fir.var_attrs<contiguous, optional>, uniq_name = "_QFtest_contiguous_derived_defaultEd1"} : (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>, !fir.dscope) -> (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>, !fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>)
+  fir.select_type %1#1 : !fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>> [#fir.type_is<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>, ^bb1, unit, ^bb2]
+^bb1:  // pred: ^bb0
+  %2 = fir.convert %1#1 : (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>) -> !fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>
+  %3:2 = hlfir.declare %2 {fortran_attrs = #fir.var_attrs<contiguous>, uniq_name = "_QFtest_contiguous_derived_defaultEd1"} : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>) -> (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, !fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>)
+  %4 = hlfir.designate %3#0 (%c1, %c1)  : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index, index) -> !fir.ref<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>
+  %5 = hlfir.designate %4{"i"}   : (!fir.ref<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>) -> !fir.ref<i32>
+  hlfir.assign %c16_i32 to %5 : i32, !fir.ref<i32>
+  cf.br ^bb3
+^bb2:  // pred: ^bb0
+  %6:2 = hlfir.declare %1#1 {fortran_attrs = #fir.var_attrs<contiguous>, uniq_name = "_QFtest_contiguous_derived_defaultEd1"} : (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>) -> (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>, !fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>)
+  cf.br ^bb3
+^bb3:  // 2 preds: ^bb1, ^bb2
+  return
+}
+// CHECK-LABEL:   func.func @_QPtest_contiguous_derived_default(
+// CHECK:           %[[VAL_0:.*]] = arith.constant 1 : index
+// CHECK:           %[[VAL_9:.*]] = fir.declare %{{.*}} {fortran_attrs = #fir.var_attrs<contiguous>, uniq_name = "_QFtest_contiguous_derived_defaultEd1"} : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>) -> !fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>
+// CHECK:           %[[VAL_10:.*]] = fir.rebox %[[VAL_9]] : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>) -> !fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>
+// CHECK:           %[[VAL_11:.*]] = fir.box_addr %[[VAL_10]] : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>) -> !fir.ref<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>
+// CHECK:           %[[VAL_12:.*]] = arith.constant 0 : index
+// CHECK:           %[[VAL_13:.*]]:3 = fir.box_dims %[[VAL_10]], %[[VAL_12]] : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index) -> (index, index, index)
+// CHECK:           %[[VAL_14:.*]] = arith.constant 1 : index
+// CHECK:           %[[VAL_15:.*]]:3 = fir.box_dims %[[VAL_10]], %[[VAL_14]] : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index) -> (index, index, index)
+// CHECK:           %[[VAL_16:.*]] = fir.shape %[[VAL_13]]#1, %[[VAL_15]]#1 : (index, index) -> !fir.shape<2>
+// CHECK:           %[[VAL_17:.*]] = fir.array_coor %[[VAL_11]](%[[VAL_16]]) %[[VAL_0]], %[[VAL_0]] : (!fir.ref<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, !fir.shape<2>, index, index) -> !fir.ref<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>
+
+// Test proper generation of fir.array_coor for contiguous box with non-default lbounds.
+func.func @_QPtest_contiguous_derived_lbounds(%arg0: !fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>> {fir.bindc_name = "d1", fir.contiguous}) {
+  %c3 = arith.constant 3 : index
+  %c1 = arith.constant 1 : index
+  %c16_i32 = arith.constant 16 : i32
+  %0 = fir.dummy_scope : !fir.dscope
+  %1 = fir.shift %c1, %c3 : (index, index) -> !fir.shift<2>
+  %2:2 = hlfir.declare %arg0(%1) dummy_scope %0 {fortran_attrs = #fir.var_attrs<contiguous>, uniq_name = "_QFtest_contiguous_derived_lboundsEd1"} : (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>, !fir.shift<2>, !fir.dscope) -> (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>, !fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>)
+  fir.select_type %2#1 : !fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>> [#fir.type_is<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>, ^bb1, unit, ^bb2]
+^bb1:  // pred: ^bb0
+  %3 = fir.convert %2#1 : (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>) -> !fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>
+  %4:2 = hlfir.declare %3(%1) {fortran_attrs = #fir.var_attrs<contiguous>, uniq_name = "_QFtest_contiguous_derived_lboundsEd1"} : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, !fir.shift<2>) -> (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, !fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>)
+  %5 = hlfir.designate %4#0 (%c1, %c3)  : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index, index) -> !fir.ref<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>
+  %6 = hlfir.designate %5{"i"}   : (!fir.ref<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>) -> !fir.ref<i32>
+  hlfir.assign %c16_i32 to %6 : i32, !fir.ref<i32>
+  cf.br ^bb3
+^bb2:  // pred: ^bb0
+  %7:2 = hlfir.declare %2#1(%1) {fortran_attrs = #fir.var_attrs<contiguous>, uniq_name = "_QFtest_contiguous_derived_lboundsEd1"} : (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>, !fir.shift<2>) -> (!fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>, !fir.class<!fir.array<?x?x!fir.type<_QMtypesTt1>>>)
+  cf.br ^bb3
+^bb3:  // 2 preds: ^bb1, ^bb2
+  return
+}
+// CHECK-LABEL:   func.func @_QPtest_contiguous_derived_lbounds(
+// CHECK:           %[[VAL_0:.*]] = arith.constant 3 : index
+// CHECK:           %[[VAL_1:.*]] = arith.constant 1 : index
+// CHECK:           %[[VAL_8:.*]] = fir.declare %{{.*}}(%[[VAL_4:.*]]) {fortran_attrs = #fir.var_attrs<contiguous>, uniq_name = "_QFtest_contiguous_derived_lboundsEd1"} : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, !fir.shift<2>) -> !fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>
+// CHECK:           %[[VAL_9:.*]] = fir.rebox %[[VAL_8]](%[[VAL_4]]) : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, !fir.shift<2>) -> !fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>
+// CHECK:           %[[VAL_10:.*]] = fir.box_addr %[[VAL_9]] : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>) -> !fir.ref<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>
+// CHECK:           %[[VAL_11:.*]] = arith.constant 0 : index
+// CHECK:           %[[VAL_12:.*]]:3 = fir.box_dims %[[VAL_9]], %[[VAL_11]] : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index) -> (index, index, index)
+// CHECK:           %[[VAL_13:.*]] = arith.constant 1 : index
+// CHECK:           %[[VAL_14:.*]]:3 = fir.box_dims %[[VAL_9]], %[[VAL_13]] : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index) -> (index, index, index)
+// CHECK:           %[[VAL_15:.*]] = arith.constant 1 : index
+// CHECK:           %[[VAL_16:.*]] = arith.subi %[[VAL_1]], %[[VAL_1]] : index
+// CHECK:           %[[VAL_17:.*]] = arith.addi %[[VAL_16]], %[[VAL_15]] : index
+// CHECK:           %[[VAL_18:.*]] = arith.constant 1 : index
+// CHECK:           %[[VAL_19:.*]] = arith.subi %[[VAL_0]], %[[VAL_0]] : index
+// CHECK:           %[[VAL_20:.*]] = arith.addi %[[VAL_19]], %[[VAL_18]] : index
+// CHECK:           %[[VAL_21:.*]] = fir.array_coor %[[VAL_10]] %[[VAL_17]], %[[VAL_20]] : (!fir.ref<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index, index) -> !fir.ref<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>

>From ac7905e6878385069b41eb39f20f1b760d01929b Mon Sep 17 00:00:00 2001
From: Slava Zakharin <szakharin at nvidia.com>
Date: Fri, 9 May 2025 15:32:24 -0700
Subject: [PATCH 2/2] Fixed the fir.shift case, and got rid of the shift
 handling in ConvertToFIR.

---
 .../flang/Optimizer/Builder/HLFIRTools.h      |  6 ----
 flang/lib/Optimizer/Builder/HLFIRTools.cpp    | 19 ++++++++---
 .../HLFIR/Transforms/ConvertToFIR.cpp         | 33 +------------------
 flang/test/HLFIR/designate-codegen.fir        |  9 ++---
 4 files changed, 17 insertions(+), 50 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/HLFIRTools.h b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
index bcba38ed8bd5d..ac80873dc374f 100644
--- a/flang/include/flang/Optimizer/Builder/HLFIRTools.h
+++ b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
@@ -533,12 +533,6 @@ Entity gen1DSection(mlir::Location loc, fir::FirOpBuilder &builder,
                     mlir::ArrayRef<mlir::Value> extents,
                     mlir::ValueRange oneBasedIndices,
                     mlir::ArrayRef<mlir::Value> typeParams);
-
-/// Return explicit lower bounds from a fir.shape result.
-/// Only fir.shape, fir.shift and fir.shape_shift are currently
-/// supported as \p shape.
-llvm::SmallVector<mlir::Value> getExplicitLboundsFromShape(mlir::Value shape);
-
 } // namespace hlfir
 
 #endif // FORTRAN_OPTIMIZER_BUILDER_HLFIRTOOLS_H
diff --git a/flang/lib/Optimizer/Builder/HLFIRTools.cpp b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
index 752dc0cf86414..f2b084cb760b9 100644
--- a/flang/lib/Optimizer/Builder/HLFIRTools.cpp
+++ b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
@@ -70,8 +70,11 @@ getExplicitExtents(fir::FortranVariableOpInterface var,
   return {};
 }
 
-llvm::SmallVector<mlir::Value>
-hlfir::getExplicitLboundsFromShape(mlir::Value shape) {
+// Return explicit lower bounds from a shape result.
+// Only fir.shape, fir.shift and fir.shape_shift are currently
+// supported as shape.
+static llvm::SmallVector<mlir::Value>
+getExplicitLboundsFromShape(mlir::Value shape) {
   llvm::SmallVector<mlir::Value> result;
   auto *shapeOp = shape.getDefiningOp();
   if (auto s = mlir::dyn_cast_or_null<fir::ShapeOp>(shapeOp)) {
@@ -93,7 +96,7 @@ hlfir::getExplicitLboundsFromShape(mlir::Value shape) {
 static llvm::SmallVector<mlir::Value>
 getExplicitLbounds(fir::FortranVariableOpInterface var) {
   if (mlir::Value shape = var.getShape())
-    return hlfir::getExplicitLboundsFromShape(shape);
+    return getExplicitLboundsFromShape(shape);
   return {};
 }
 
@@ -766,12 +769,18 @@ std::pair<mlir::Value, mlir::Value> hlfir::genVariableFirBaseShapeAndParams(
   //     (!fir.box<!fir.array<?x?x...>>, !fir.box<!fir.array<?x?x...>>)
   // The extended value is an ArrayBoxValue with base being
   // the raw address of the array.
-  if (auto variableInterface = entity.getIfVariableInterface())
+  if (auto variableInterface = entity.getIfVariableInterface()) {
+    mlir::Value shape = variableInterface.getShape();
     if (mlir::isa<fir::BaseBoxType>(fir::getBase(exv).getType()) ||
         !mlir::isa<fir::BaseBoxType>(entity.getType()) ||
-        variableInterface.getShape())
+        // Still use the variable's shape if it is present.
+        // If it only specifies a shift, then we have to create
+        // a shape from the exv.
+        (shape && (shape.getDefiningOp<fir::ShapeShiftOp>() ||
+                   shape.getDefiningOp<fir::ShapeOp>())))
       return {fir::getBase(exv),
               asEmboxShape(loc, builder, exv, variableInterface.getShape())};
+  }
   return {fir::getBase(exv), builder.createShape(loc, exv)};
 }
 
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
index 495f11a365185..8f206b5a1ade7 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
@@ -412,44 +412,13 @@ class DesignateOpConversion
     auto indices = designate.getIndices();
     int i = 0;
     auto attrs = designate.getIsTripletAttr();
-
-    // If the shape specifies a shift and the base is not a box,
-    // then we have to subtract the lower bounds, as long as
-    // fir.array_coor does not support non-default lower bounds
-    // for non-box accesses.
-    llvm::SmallVector<mlir::Value> lbounds;
-    if (shape && !mlir::isa<fir::BaseBoxType>(base.getType()))
-      lbounds = hlfir::getExplicitLboundsFromShape(shape);
-    std::size_t lboundIdx = 0;
     for (auto isTriplet : attrs.asArrayRef()) {
       // Coordinate of the first element are the index and triplets lower
       // bounds.
-      mlir::Value index = indices[i];
-      if (!lbounds.empty()) {
-        assert(lboundIdx < lbounds.size() && "missing lbound");
-        mlir::Type indexType = builder.getIndexType();
-        mlir::Value one = builder.createIntegerConstant(loc, indexType, 1);
-        mlir::Value orig = builder.createConvert(loc, indexType, index);
-        mlir::Value lb =
-            builder.createConvert(loc, indexType, lbounds[lboundIdx]);
-        index = builder.create<mlir::arith::SubIOp>(loc, orig, lb);
-        index = builder.create<mlir::arith::AddIOp>(loc, index, one);
-        ++lboundIdx;
-      }
-      firstElementIndices.push_back(index);
+      firstElementIndices.push_back(indices[i]);
       i = i + (isTriplet ? 3 : 1);
     }
 
-    // Remove the shift from the shape, if needed.
-    if (!lbounds.empty()) {
-      mlir::Operation *op = shape.getDefiningOp();
-      if (mlir::isa<fir::ShiftOp>(op))
-        shape = nullptr;
-      else if (auto shiftOp = mlir::dyn_cast<fir::ShapeShiftOp>(op))
-        shape = builder.create<fir::ShapeOp>(loc, shiftOp.getExtents());
-      else
-        TODO(loc, "read fir.shape to get lower bounds");
-    }
     mlir::Type originalDesignateType = designate.getResult().getType();
     const bool isVolatile = fir::isa_volatile_type(originalDesignateType);
     mlir::Type arrayCoorType = fir::ReferenceType::get(baseEleTy, isVolatile);
diff --git a/flang/test/HLFIR/designate-codegen.fir b/flang/test/HLFIR/designate-codegen.fir
index d3e264941264f..5c3ae202fd3b9 100644
--- a/flang/test/HLFIR/designate-codegen.fir
+++ b/flang/test/HLFIR/designate-codegen.fir
@@ -278,10 +278,5 @@ func.func @_QPtest_contiguous_derived_lbounds(%arg0: !fir.class<!fir.array<?x?x!
 // CHECK:           %[[VAL_12:.*]]:3 = fir.box_dims %[[VAL_9]], %[[VAL_11]] : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index) -> (index, index, index)
 // CHECK:           %[[VAL_13:.*]] = arith.constant 1 : index
 // CHECK:           %[[VAL_14:.*]]:3 = fir.box_dims %[[VAL_9]], %[[VAL_13]] : (!fir.box<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index) -> (index, index, index)
-// CHECK:           %[[VAL_15:.*]] = arith.constant 1 : index
-// CHECK:           %[[VAL_16:.*]] = arith.subi %[[VAL_1]], %[[VAL_1]] : index
-// CHECK:           %[[VAL_17:.*]] = arith.addi %[[VAL_16]], %[[VAL_15]] : index
-// CHECK:           %[[VAL_18:.*]] = arith.constant 1 : index
-// CHECK:           %[[VAL_19:.*]] = arith.subi %[[VAL_0]], %[[VAL_0]] : index
-// CHECK:           %[[VAL_20:.*]] = arith.addi %[[VAL_19]], %[[VAL_18]] : index
-// CHECK:           %[[VAL_21:.*]] = fir.array_coor %[[VAL_10]] %[[VAL_17]], %[[VAL_20]] : (!fir.ref<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, index, index) -> !fir.ref<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>
+// CHECK:           %[[VAL_15:.*]] = fir.shape_shift %[[VAL_1]], %[[VAL_12]]#1, %[[VAL_0]], %[[VAL_14]]#1 : (index, index, index, index) -> !fir.shapeshift<2>
+// CHECK:           %[[VAL_16:.*]] = fir.array_coor %[[VAL_10]](%[[VAL_15]]) %[[VAL_1]], %[[VAL_0]] : (!fir.ref<!fir.array<?x?x!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>>, !fir.shapeshift<2>, index, index) -> !fir.ref<!fir.type<_QMtypesTt2{t1:!fir.type<_QMtypesTt1>,i:i32}>>



More information about the flang-commits mailing list