-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[flang][acc] Fix mappableTy.generateAccBounds to correctly handle dynamic-sized arrays #155666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-flang-fir-hlfir Author: None (khaki3) ChangesWe currently see the misuse of the Full diff: https://github.com/llvm/llvm-project/pull/155666.diff 2 Files Affected:
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>>
|
@llvm/pr-subscribers-openacc Author: None (khaki3) ChangesWe currently see the misuse of the Full diff: https://github.com/llvm/llvm-project/pull/155666.diff 2 Files Affected:
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>>
|
@razvanlupusoru, thanks for your review. I've updated the fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch since I missed that I actually used zero for the lowerbound. Please fix the test and I can approve :) Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Thank you!
We currently see the misuse of the
upperbound
operand for theacc::DataBoundsOp
generation ingenerateSeqTyAccBounds
. This PR keeps settinglowerbound
to zero for all cases and adjustsupperbound
to beextent - 1
.