[flang][openacc] Fix the value of lower bound, upper bound and extent

[clementval]

Summary

Update the lowering to match the intent of the newly added operation
as described here D148860.

[razvanlupusoru]

Thank you for addressing this issue. For default lb of 1, the acc.bounds operation appears to work as intended.
However, would this work for the case of "integer :: array(0:9)" or similarly non-1 constant lowerbound?

[clementval]

In D148932#4287938, @razvanlupusoru wrote:

Thank you for addressing this issue. For default lb of 1, the acc.bounds operation appears to work as intended.
However, would this work for the case of "integer :: array(0:9)" or similarly non-1 constant lowerbound?

Let me add couple of tests for these cases.

[clementval]

Add test for array section from array with non default lower bound. Update the
code to support these cases correctly.

[clementval]

flang/lib/Lower/OpenACC.cpp

134

The special case here will be merge with the non constant bound support so the code will hopefully look better.

[razvanlupusoru]

I went pretty meticulously through the tests and everything looks correct. Thank you for all the detail on writing these tests.

[clementval]

clang-format

flang/lib/Lower/OpenACC.cpp

 }
 
 static llvm::SmallVector<mlir::Value>
 genBoundsOps(fir::FirOpBuilder &builder, mlir::Location loc,
              Fortran::lower::AbstractConverter &converter,
              const std::list<Fortran::parser::SectionSubscript> &subscripts,
              std::stringstream &asFortran, const Fortran::parser::Name &name) {
   int dimension = 0;
-  mlir::Type i64Ty = builder.getI64Type();
+  mlir::Type idxTy = builder.getIndexType();
   mlir::Type boundTy = builder.getType<mlir::acc::DataBoundsType>();
   llvm::SmallVector<mlir::Value> bounds;
   for (const auto &subscript : subscripts) {
     if (const auto *triplet{
             std::get_if<Fortran::parser::SubscriptTriplet>(&subscript.u)}) {
       if (dimension != 0)
         asFortran << ',';
       mlir::Value lbound, ubound, extent;
       std::optional<std::int64_t> lval, uval;
+      mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
+      fir::ExtendedValue dataExv =
+          converter.getSymbolExtendedValue(*name.symbol);
+      mlir::Value baseLb =
+          fir::factory::readLowerBound(builder, loc, dataExv, dimension, one);
+      bool defaultLb = baseLb == one;
+
       const auto &lower{std::get<0>(triplet->t)};
       if (lower) {
         lval = Fortran::semantics::GetIntValue(lower);
         if (lval) {
-          lbound = builder.createIntegerConstant(loc, i64Ty, *lval);
+          if (defaultLb) {
+            lbound = builder.createIntegerConstant(loc, idxTy, *lval - 1);
+          } else {
+            mlir::Value lb = builder.createIntegerConstant(loc, idxTy, *lval);
+            lbound = builder.create<mlir::arith::SubIOp>(loc, lb, baseLb);
+          }
           asFortran << *lval;
         } else {
           TODO(loc, "non constant lower bound in array section");
         }
       }
       asFortran << ':';
       const auto &upper{std::get<1>(triplet->t)};
       if (upper) {
         uval = Fortran::semantics::GetIntValue(upper);
         if (uval) {
-          ubound = builder.createIntegerConstant(loc, i64Ty, *uval);
+          if (defaultLb) {
+            ubound = builder.createIntegerConstant(loc, idxTy, *uval - 1);
+          } else {
+            mlir::Value ub = builder.createIntegerConstant(loc, idxTy, *uval);
+            ubound = builder.create<mlir::arith::SubIOp>(loc, ub, baseLb);
+          }
           asFortran << *uval;
         } else {
           TODO(loc, "non constant upper bound in array section");
         }
       }
       if (lower && upper) {
         if (lval && uval && *uval < *lval) {
           mlir::emitError(loc, "zero sized array section");
           break;
         } else if (std::get<2>(triplet->t)) {
           const auto &strideExpr{std::get<2>(triplet->t)};
           if (strideExpr) {
             mlir::emitError(loc, "stride cannot be specified on "
                                  "an OpenACC array section");
             break;
           }
         }
       }
       if (!ubound) {
-        fir::ExtendedValue x = converter.getSymbolExtendedValue(*name.symbol);
-        extent = fir::factory::readExtent(builder, loc, x, dimension);
+        mlir::Value ext =
+            fir::factory::readExtent(builder, loc, dataExv, dimension);
+        extent = builder.create<mlir::arith::SubIOp>(loc, ext, baseLb);
       }
       mlir::Value empty;
       mlir::Value bound = builder.create<mlir::acc::DataBoundsOp>(
-          loc, boundTy, lbound, ubound, extent, empty, false, empty);
+          loc, boundTy, lbound, ubound, extent, empty, false, baseLb);
       bounds.push_back(bound);
       ++dimension;
     }
   }
   return bounds;
 }
 
 template <typename Op>

flang/test/Lower/OpenACC/acc-enter-data.f90

!CHECK: acc.enter_data wait(%[[WAIT2]], %[[WAIT3]] : i32, i32) dataOperands(%[[CREATE_A]] : !fir.ref<!fir.array<10x10xf32>>)
   !$acc enter data create(a) wait(devnum: 1: queues: 1, 2)
 !CHECK: %[[CREATE_A:.*]] = acc.create varPtr(%[[A]] : !fir.ref<!fir.array<10x10xf32>>)   -> !fir.ref<!fir.array<10x10xf32>> {name = "a", structured = false}
 !CHECK: %[[WAIT4:.*]] = arith.constant 1 : i32
 !CHECK: %[[WAIT5:.*]] = arith.constant 2 : i32
 !CHECK: %[[WAIT6:.*]] = arith.constant 1 : i32
 !CHECK: acc.enter_data wait_devnum(%[[WAIT6]] : i32) wait(%[[WAIT4]], %[[WAIT5]] : i32, i32) dataOperands(%[[CREATE_A]] : !fir.ref<!fir.array<10x10xf32>>)
 
   !$acc enter data copyin(a(1:10,1:5))
-!CHECK: %[[LB1:.*]] = arith.constant 1 : i64
-!CHECK: %[[UB1:.*]] = arith.constant 10 : i64
-!CHECK: %[[BOUND1:.*]] = acc.bounds lowerbound(%[[LB1]] : i64) upperbound(%[[UB1]] : i64)
-!CHECK: %[[LB2:.*]] = arith.constant 1 : i64
-!CHECK: %[[UB2:.*]] = arith.constant 5 : i64
-!CHECK: %[[BOUND2:.*]] = acc.bounds lowerbound(%[[LB2]] : i64) upperbound(%[[UB2]] : i64)
+!CHECK: %[[LB1:.*]] = arith.constant 0 : index
+!CHECK: %[[UB1:.*]] = arith.constant 9 : index
+!CHECK: %[[BOUND1:.*]] = acc.bounds lowerbound(%[[LB1]] : index) upperbound(%[[UB1]] : index) startIdx(%c1{{.*}} : index)
+!CHECK: %[[LB2:.*]] = arith.constant 0 : index
+!CHECK: %[[UB2:.*]] = arith.constant 4 : index
+!CHECK: %[[BOUND2:.*]] = acc.bounds lowerbound(%[[LB2]] : index) upperbound(%[[UB2]] : index) startIdx(%c1{{.*}} : index) 
 !CHECK: %[[COPYIN_A:.*]] = acc.copyin varPtr(%[[A]] : !fir.ref<!fir.array<10x10xf32>>) bounds(%[[BOUND1]], %[[BOUND2]]) -> !fir.ref<!fir.array<10x10xf32>> {name = "a(1:10,1:5)", structured = false}
 !CHECK: acc.enter_data dataOperands(%[[COPYIN_A]] : !fir.ref<!fir.array<10x10xf32>>)
 
   !$acc enter data copyin(a(1:,1:5))
-!CHECK: %[[LB1:.*]] = arith.constant 1 : i64
-!CHECK: %[[BOUND1:.*]] = acc.bounds   lowerbound(%[[LB1]] : i64) extent(%[[EXTENT_C10]] : index)
-!CHECK: %[[LB2:.*]] = arith.constant 1 : i64
-!CHECK: %[[UB2:.*]] = arith.constant 5 : i64
-!CHECK: %[[BOUND2:.*]] = acc.bounds   lowerbound(%[[LB2]] : i64) upperbound(%[[UB2]] : i64)
-!CHECK: %[[COPYIN_A:.*]] = acc.copyin varPtr(%[[A]] : !fir.ref<!fir.array<10x10xf32>>)   bounds(%[[BOUND1]], %[[BOUND2]]) -> !fir.ref<!fir.array<10x10xf32>> {name = "a(1:,1:5)", structured = false}
+!CHECK: %[[LB1:.*]] = arith.constant 0 : index
+!CHECK: %[[EXTENT:.*]] = arith.subi %[[EXTENT_C10:.*]], %c1{{.*}} : index 
+!CHECK: %[[BOUND1:.*]] = acc.bounds   lowerbound(%[[LB1]] : index) extent(%[[EXTENT]] : index)  startIdx(%c1{{.*}} : index)
+!CHECK: %[[LB2:.*]] = arith.constant 0 : index
+!CHECK: %[[UB2:.*]] = arith.constant 4 : index
+!CHECK: %[[BOUND2:.*]] = acc.bounds lowerbound(%[[LB2]] : index) upperbound(%[[UB2]] : index)  startIdx(%c1{{.*}} : index)
+!CHECK: %[[COPYIN_A:.*]] = acc.copyin varPtr(%[[A]] : !fir.ref<!fir.array<10x10xf32>>) bounds(%[[BOUND1]], %[[BOUND2]]) -> !fir.ref<!fir.array<10x10xf32>> {name = "a(1:,1:5)", structured = false}
 !CHECK: acc.enter_data   dataOperands(%[[COPYIN_A]] : !fir.ref<!fir.array<10x10xf32>>)
 
   !$acc enter data copyin(a(:10,1:5))
-!CHECK: %[[UB1:.*]] = arith.constant 10 : i64
-!CHECK: %[[BOUND1:.*]] = acc.bounds   upperbound(%[[UB1]] : i64)
-!CHECK: %[[LB2:.*]] = arith.constant 1 : i64
-!CHECK: %[[UB2:.*]] = arith.constant 5 : i64
-!CHECK: %[[BOUND2:.*]] = acc.bounds   lowerbound(%[[LB2]] : i64) upperbound(%[[UB2]] : i64)
-!CHECK: %[[COPYIN_A:.*]] = acc.copyin varPtr(%[[A]] : !fir.ref<!fir.array<10x10xf32>>)   bounds(%[[BOUND1]], %[[BOUND2]]) -> !fir.ref<!fir.array<10x10xf32>> {name = "a(:10,1:5)", structured = false}
-!CHECK: acc.enter_data   dataOperands(%[[COPYIN_A]] : !fir.ref<!fir.array<10x10xf32>>)
+!CHECK: %[[UB1:.*]] = arith.constant 9 : index
+!CHECK: %[[BOUND1:.*]] = acc.bounds   upperbound(%[[UB1]] : index)
+!CHECK: %[[LB2:.*]] = arith.constant 0 : index
+!CHECK: %[[UB2:.*]] = arith.constant 4 : index
+!CHECK: %[[BOUND2:.*]] = acc.bounds lowerbound(%[[LB2]] : index) upperbound(%[[UB2]] : index)  startIdx(%c1{{.*}} : index)
+!CHECK: %[[COPYIN_A:.*]] = acc.copyin varPtr(%[[A]] : !fir.ref<!fir.array<10x10xf32>>) bounds(%[[BOUND1]], %[[BOUND2]]) -> !fir.ref<!fir.array<10x10xf32>> {name = "a(:10,1:5)", structured = false}
+!CHECK: acc.enter_data dataOperands(%[[COPYIN_A]] : !fir.ref<!fir.array<10x10xf32>>)
 
+  !$acc enter data copyin(a(:,:))
+!CHECK: %[[C1:.*]] = arith.constant 1 : index
+!CHECK: %[[EXT:.*]] = arith.subi %c10{{.*}}, %[[C1]] : index
+!CHECK: %[[BOUND1:.*]] = acc.bounds extent(%[[EXT]] : index) startIdx(%[[C1]] : index)
+!CHECK: %[[C1:.*]] = arith.constant 1 : index
+!CHECK: %[[EXT:.*]] = arith.subi %c10{{.*}}, %[[C1]] : index
+!CHECK: %[[BOUND2:.*]] = acc.bounds extent(%[[EXT]] : index) startIdx(%[[C1]] : index)
+!CHECK: %[[COPYIN_A:.*]] = acc.copyin varPtr(%[[A]] : !fir.ref<!fir.array<10x10xf32>>) bounds(%[[BOUND1]], %[[BOUND2]]) -> !fir.ref<!fir.array<10x10xf32>> {name = "a(:,:)", structured = false}
+!CHECK: acc.enter_data dataOperands(%[[COPYIN_A]] : !fir.ref<!fir.array<10x10xf32>>)
+
 end subroutine acc_enter_data
 
 
 subroutine acc_enter_data_dummy(a)
   real :: a(1:10)
 
 !CHECK-LABEL: func.func @_QPacc_enter_data_dummy
 !CHECK-SAME:    %[[A:.*]]: !fir.ref<!fir.array<10xf32>> {fir.bindc_name = "a"}
 
   !$acc enter data create(a(5:10))
-!CHECK: %[[LB1:.*]] = arith.constant 5 : i64
-!CHECK: %[[UB1:.*]] = arith.constant 10 : i64
-!CHECK: %[[BOUND1:.*]] = acc.bounds lowerbound(%[[LB1]] : i64) upperbound(%[[UB1]] : i64)
-!CHECK: %[[CREATE1:.*]] = acc.create varPtr(%[[A]] : !fir.ref<!fir.array<10xf32>>)   bounds(%[[BOUND1]]) -> !fir.ref<!fir.array<10xf32>> {name = "a(5:10)", structured = false}
-!CHECK: acc.enter_data   dataOperands(%[[CREATE1]] : !fir.ref<!fir.array<10xf32>>)
+!CHECK: %[[LB1:.*]] = arith.constant 4 : index
+!CHECK: %[[UB1:.*]] = arith.constant 9 : index
+!CHECK: %[[BOUND1:.*]] = acc.bounds lowerbound(%[[LB1]] : index) upperbound(%[[UB1]] : index)  startIdx(%c1{{.*}} : index)
+!CHECK: %[[CREATE1:.*]] = acc.create varPtr(%[[A]] : !fir.ref<!fir.array<10xf32>>) bounds(%[[BOUND1]]) -> !fir.ref<!fir.array<10xf32>> {name = "a(5:10)", structured = false}
+!CHECK: acc.enter_data dataOperands(%[[CREATE1]] : !fir.ref<!fir.array<10xf32>>)
 
 end subroutine
+
+! Test lowering of array section for non default lower bound.
+subroutine acc_enter_data_non_default_lb()
+  integer :: a(0:9)
+
+!CHECK-LABEL: func.func @_QPacc_enter_data_non_default_lb() {
+!CHECK: %[[BASELB:.*]] = arith.constant 0 : index
+!CHECK: %[[A:.*]] = fir.alloca !fir.array<10xi32> {bindc_name = "a", uniq_name = "_QFacc_enter_data_non_default_lbEa"}
+
+  !$acc enter data create(a(5:9))
+!CHECK: %[[SECTIONLB:.*]] = arith.constant 5 : index
+!CHECK: %[[LB:.*]] = arith.subi %[[SECTIONLB]], %[[BASELB]] : index
+!CHECK: %[[SECTIONUB:.*]] = arith.constant 9 : index
+!CHECK: %[[UB:.*]] = arith.subi %[[SECTIONUB]], %[[BASELB]] : index
+!CHECK: %[[BOUND:.*]] = acc.bounds lowerbound(%[[LB]] : index) upperbound(%[[UB]] : index) startIdx(%[[BASELB]] : index)
+!CHECK: %[[CREATE:.*]] = acc.create varPtr(%[[A]] : !fir.ref<!fir.array<10xi32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xi32>> {name = "a(5:9)", structured = false}
+!CHECK: acc.enter_data dataOperands(%[[CREATE]] : !fir.ref<!fir.array<10xi32>>)
+
+  !$acc enter data create(a(:))
+!CHECK: %[[EXT:.*]] = arith.subi %c10{{.*}}, %[[BASELB]] : index
+!CHECK: %[[BOUND:.*]] = acc.bounds extent(%[[EXT]] : index) startIdx(%[[BASELB]] : index)
+!CHECK: %[[CREATE:.*]] = acc.create varPtr(%[[A]] : !fir.ref<!fir.array<10xi32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xi32>> {name = "a(:)", structured = false}
+!CHECK: acc.enter_data dataOperands(%[[CREATE]] : !fir.ref<!fir.array<10xi32>>)
+
+  !$acc enter data create(a(:6))
+!CHECK: %[[SECTIONUB:.*]] = arith.constant 6 : index
+!CHECK: %[[UB:.*]] = arith.subi %[[SECTIONUB]], %[[BASELB]] : index
+!CHECK: %[[BOUND:.*]] = acc.bounds upperbound(%[[UB]] : index) startIdx(%[[BASELB]] : index)
+!CHECK: %[[CREATE:.*]] = acc.create varPtr(%[[A]] : !fir.ref<!fir.array<10xi32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xi32>> {name = "a(:6)", structured = false}
+!CHECK: acc.enter_data dataOperands(%[[CREATE]] : !fir.ref<!fir.array<10xi32>>)
+
+  !$acc enter data create(a(4:))
+!CHECK: %[[SECTIONLB:.*]] = arith.constant 4 : index
+!CHECK: %[[LB:.*]] = arith.subi %[[SECTIONLB]], %[[BASELB]] : index
+!CHECK: %[[EXT:.*]] = arith.subi %c10{{.*}}, %[[BASELB]] : index
+!CHECK: %[[BOUND:.*]] = acc.bounds lowerbound(%[[LB]] : index) extent(%[[EXT]] : index) startIdx(%[[BASELB]] : index)
+!CHECK: %[[CREATE:.*]] = acc.create varPtr(%[[A]] : !fir.ref<!fir.array<10xi32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xi32>> {name = "a(4:)", structured = false}
+!CHECK: acc.enter_data dataOperands(%[[CREATE]] : !fir.ref<!fir.array<10xi32>>)
+
+end subroutine