[flang-commits] [flang] [flang][acc] Fix mappableTy.generateAccBounds to correctly handle dynamic-sized arrays (PR #155666)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-openacc

Author: None (khaki3)

<details>
<summary>Changes</summary>

We currently see the misuse of the `startIdx` operand for the `acc::DataBoundsOp` generation in `generateSeqTyAccBounds`. This PR sets `startIdx` to one for all cases and instead adjusts both `lowerbound` and `upperbound`.

---
Full diff: https://github.com/llvm/llvm-project/pull/155666.diff


2 Files Affected:

- (modified) flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp (+3-5) 
- (modified) flang/test/Fir/OpenACC/openacc-mappable.fir (+2-2) 


``````````diff
diff --git a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp
index 5b6d904fb0d59..9b4db4900258b 100644
--- a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp
+++ b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp
@@ -243,8 +243,6 @@ generateSeqTyAccBounds(fir::SequenceType seqType, mlir::Value var,
               mlir::dyn_cast_if_present<fir::ShapeOp>(shape.getDefiningOp())) {
         mlir::Value cummulativeExtent = one;
         for (auto extent : shapeOp.getExtents()) {
-          mlir::Value upperbound =
-              mlir::arith::SubIOp::create(builder, loc, extent, one);
           mlir::Value stride = one;
           if (strideIncludeLowerExtent) {
             stride = cummulativeExtent;
@@ -254,7 +252,7 @@ generateSeqTyAccBounds(fir::SequenceType seqType, mlir::Value var,
           auto accBound = mlir::acc::DataBoundsOp::create(
               builder, loc,
               mlir::acc::DataBoundsType::get(builder.getContext()),
-              /*lowerbound=*/zero, /*upperbound=*/upperbound,
+              /*lowerbound=*/one, /*upperbound=*/extent,
               /*extent=*/extent, /*stride=*/stride, /*strideInBytes=*/false,
               /*startIdx=*/one);
           accBounds.push_back(accBound);
@@ -282,9 +280,9 @@ generateSeqTyAccBounds(fir::SequenceType seqType, mlir::Value var,
             auto accBound = mlir::acc::DataBoundsOp::create(
                 builder, loc,
                 mlir::acc::DataBoundsType::get(builder.getContext()),
-                /*lowerbound=*/zero, /*upperbound=*/upperbound,
+                /*lowerbound=*/lowerbound, /*upperbound=*/upperbound,
                 /*extent=*/extent, /*stride=*/stride, /*strideInBytes=*/false,
-                /*startIdx=*/lowerbound);
+                /*startIdx=*/one);
             accBounds.push_back(accBound);
           }
         }
diff --git a/flang/test/Fir/OpenACC/openacc-mappable.fir b/flang/test/Fir/OpenACC/openacc-mappable.fir
index 71576f4b71075..3535bfaae3c30 100644
--- a/flang/test/Fir/OpenACC/openacc-mappable.fir
+++ b/flang/test/Fir/OpenACC/openacc-mappable.fir
@@ -62,12 +62,12 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<f16 = dense<16> : vector<2xi64>,
   // CHECK: Visiting: %{{.*}} = acc.copyin varPtr(%{{.*}} : !fir.ref<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>> {name = "arr1", structured = false}
   // CHECK: Pointer-like and Mappable: !fir.ref<!fir.array<?xf32>>
   // CHECK: Type category: array
-  // CHECK: Bound[0]: %{{.*}} = acc.bounds lowerbound(%c0{{.*}} : index) upperbound(%{{.*}} : index) extent(%{{.*}} : index) stride(%c1{{.*}} : index) startIdx(%c1{{.*}} : index)
+  // CHECK: Bound[0]: %{{.*}} = acc.bounds lowerbound(%c1{{.*}} : index) upperbound(%{{.*}} : index) extent(%{{.*}} : index) stride(%c1{{.*}} : index) startIdx(%c1{{.*}} : index)
 
   // CHECK: Visiting: %{{.*}} = acc.copyin varPtr(%{{.*}} : !fir.ref<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>> {name = "arr2", structured = false}
   // CHECK: Pointer-like and Mappable: !fir.ref<!fir.array<?xf32>>
   // CHECK: Type category: array
-  // CHECK: Bound[0]: %{{.*}} = acc.bounds lowerbound(%c0{{.*}} : index) upperbound(%{{.*}} : index) extent(%{{.*}} : index) stride(%c1{{.*}} : index) startIdx(%c2{{.*}} : index)
+  // CHECK: Bound[0]: %{{.*}} = acc.bounds lowerbound(%c2{{.*}} : index) upperbound(%{{.*}} : index) extent(%{{.*}} : index) stride(%c1{{.*}} : index) startIdx(%c1{{.*}} : index)
 
   // CHECK: Visiting: %{{.*}} = acc.copyin varPtr(%{{.*}} : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "arr3", structured = false}
   // CHECK: Pointer-like and Mappable: !fir.ref<!fir.array<10xf32>>

``````````

</details>


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