[flang] canonicalize shape in pointer remapping lowering #157472
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: None (jeanPerier) ChangesFix for #153221. Canonicalize the new shape of the pointer when lowering pointer assignment with bounds remapping. Note that this only needs to be done for the extents here because lower bounds are canonicalized in LBOUND inquiries and descriptor creation (embox/rebox codegen) based on the extent value. Full diff: https://github.com/llvm/llvm-project/pull/157472.diff 4 Files Affected:
diff --git a/flang/lib/Optimizer/Builder/MutableBox.cpp b/flang/lib/Optimizer/Builder/MutableBox.cpp
index 50c945df5b465..d4cdfecd0b088 100644
--- a/flang/lib/Optimizer/Builder/MutableBox.cpp
+++ b/flang/lib/Optimizer/Builder/MutableBox.cpp
@@ -603,21 +603,23 @@ void fir::factory::associateMutableBoxWithRemap(
mlir::ValueRange lbounds, mlir::ValueRange ubounds) {
// Compute new extents
llvm::SmallVector<mlir::Value> extents;
- auto idxTy = builder.getIndexType();
+ mlir::Type idxTy = builder.getIndexType();
+ mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
if (!lbounds.empty()) {
auto one = builder.createIntegerConstant(loc, idxTy, 1);
for (auto [lb, ub] : llvm::zip(lbounds, ubounds)) {
- auto lbi = builder.createConvert(loc, idxTy, lb);
- auto ubi = builder.createConvert(loc, idxTy, ub);
- auto diff = mlir::arith::SubIOp::create(builder, loc, idxTy, ubi, lbi);
+
+ mlir::Value lbi = builder.createConvert(loc, idxTy, lb);
+ mlir::Value ubi = builder.createConvert(loc, idxTy, ub);
extents.emplace_back(
- mlir::arith::AddIOp::create(builder, loc, idxTy, diff, one));
+ fir::factory::computeExtent(builder, loc, lbi, ubi, zero, one));
}
} else {
// lbounds are default. Upper bounds and extents are the same.
- for (auto ub : ubounds) {
- auto cast = builder.createConvert(loc, idxTy, ub);
- extents.emplace_back(cast);
+ for (mlir::Value ub : ubounds) {
+ mlir::Value cast = builder.createConvert(loc, idxTy, ub);
+ extents.emplace_back(
+ fir::factory::genMaxWithZero(builder, loc, cast, zero));
}
}
const auto newRank = extents.size();
diff --git a/flang/test/Lower/HLFIR/allocatable-and-pointer-status-change.f90 b/flang/test/Lower/HLFIR/allocatable-and-pointer-status-change.f90
index e2fd268bf994d..08492e913c992 100644
--- a/flang/test/Lower/HLFIR/allocatable-and-pointer-status-change.f90
+++ b/flang/test/Lower/HLFIR/allocatable-and-pointer-status-change.f90
@@ -58,12 +58,16 @@ subroutine pointer_remapping(p, ziel)
! CHECK: %[[VAL_14:.*]] = fir.convert %[[VAL_9]] : (i64) -> index
! CHECK: %[[VAL_15:.*]] = arith.subi %[[VAL_14]], %[[VAL_13]] : index
! CHECK: %[[VAL_16:.*]] = arith.addi %[[VAL_15]], %[[VAL_12]] : index
+! CHECK: %[[cmp0:.*]] = arith.cmpi sgt, %[[VAL_16]], %c0{{.*}} : index
+! CHECK: %[[ext0:.*]] = arith.select %[[cmp0]], %[[VAL_16]], %c0{{.*}} : index
! CHECK: %[[VAL_17:.*]] = fir.convert %[[VAL_10]] : (i64) -> index
! CHECK: %[[VAL_18:.*]] = fir.convert %[[VAL_11]] : (i64) -> index
! CHECK: %[[VAL_19:.*]] = arith.subi %[[VAL_18]], %[[VAL_17]] : index
! CHECK: %[[VAL_20:.*]] = arith.addi %[[VAL_19]], %[[VAL_12]] : index
+! CHECK: %[[cmp1:.*]] = arith.cmpi sgt, %[[VAL_20]], %c0{{.*}} : index
+! CHECK: %[[ext1:.*]] = arith.select %[[cmp1]], %[[VAL_20]], %c0{{.*}} : index
! CHECK: %[[VAL_21:.*]] = fir.convert %[[VAL_7]]#0 : (!fir.ref<!fir.array<10x20x30xf32>>) -> !fir.ref<!fir.array<?x?xf32>>
-! CHECK: %[[VAL_22:.*]] = fir.shape_shift %[[VAL_8]], %[[VAL_16]], %[[VAL_10]], %[[VAL_20]] : (i64, index, i64, index) -> !fir.shapeshift<2>
+! CHECK: %[[VAL_22:.*]] = fir.shape_shift %[[VAL_8]], %[[ext0]], %[[VAL_10]], %[[ext1]] : (i64, index, i64, index) -> !fir.shapeshift<2>
! CHECK: %[[VAL_23:.*]] = fir.embox %[[VAL_21]](%[[VAL_22]]) : (!fir.ref<!fir.array<?x?xf32>>, !fir.shapeshift<2>) -> !fir.box<!fir.ptr<!fir.array<?x?xf32>>>
! CHECK: fir.store %[[VAL_23]] to %[[VAL_2]]#0 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?x?xf32>>>>
end subroutine
diff --git a/flang/test/Lower/HLFIR/issue80884.f90 b/flang/test/Lower/HLFIR/issue80884.f90
index 5c05a99a53eaf..a5a51782ae735 100644
--- a/flang/test/Lower/HLFIR/issue80884.f90
+++ b/flang/test/Lower/HLFIR/issue80884.f90
@@ -26,7 +26,9 @@ subroutine issue80884(p, targ)
! CHECK: %[[VAL_13:.*]] = fir.convert %[[VAL_5]] : (i64) -> index
! CHECK: %[[VAL_14:.*]] = arith.subi %[[VAL_13]], %[[VAL_12]] : index
! CHECK: %[[VAL_15:.*]] = arith.addi %[[VAL_14]], %[[VAL_11]] : index
+! CHECK: %[[cmp0:.*]] = arith.cmpi sgt, %[[VAL_15]], %c0{{.*}} : index
+! CHECK: %[[ext0:.*]] = arith.select %[[cmp0]], %[[VAL_15]], %c0{{.*}} : index
! CHECK: %[[VAL_16:.*]] = fir.convert %[[VAL_10]] : (!fir.ref<!fir.array<10x10xf32>>) -> !fir.ref<!fir.array<?xf32>>
-! CHECK: %[[VAL_17:.*]] = fir.shape_shift %[[VAL_4]], %[[VAL_15]] : (i64, index) -> !fir.shapeshift<1>
+! CHECK: %[[VAL_17:.*]] = fir.shape_shift %[[VAL_4]], %[[ext0]] : (i64, index) -> !fir.shapeshift<1>
! CHECK: %[[VAL_18:.*]] = fir.embox %[[VAL_16]](%[[VAL_17]]) : (!fir.ref<!fir.array<?xf32>>, !fir.shapeshift<1>) -> !fir.box<!fir.ptr<!fir.array<?xf32>>>
! CHECK: fir.store %[[VAL_18]] to %[[VAL_2]]#0 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
diff --git a/flang/test/Lower/pointer-assignments.f90 b/flang/test/Lower/pointer-assignments.f90
index ac9c99c97a570..98fd61df3602f 100644
--- a/flang/test/Lower/pointer-assignments.f90
+++ b/flang/test/Lower/pointer-assignments.f90
@@ -113,11 +113,15 @@ subroutine test_array_remap(p, x)
! CHECK-DAG: %[[c2_idx:.*]] = fir.convert %c2{{.*}} : (i64) -> index
! CHECK-DAG: %[[c11_idx:.*]] = fir.convert %c11{{.*}} : (i64) -> index
! CHECK-DAG: %[[diff0:.*]] = arith.subi %[[c11_idx]], %[[c2_idx]] : index
- ! CHECK-DAG: %[[ext0:.*]] = arith.addi %[[diff0:.*]], %c1{{.*}} : index
+ ! CHECK-DAG: %[[raw_ext0:.*]] = arith.addi %[[diff0:.*]], %c1{{.*}} : index
+ ! CHECK-DAG: %[[cmp0:.*]] = arith.cmpi sgt, %[[raw_ext0]], %c0{{.*}} : index
+ ! CHECK-DAG: %[[ext0:.*]] = arith.select %[[cmp0]], %[[raw_ext0]], %c0{{.*}} : index
! CHECK-DAG: %[[c3_idx:.*]] = fir.convert %c3{{.*}} : (i64) -> index
! CHECK-DAG: %[[c12_idx:.*]] = fir.convert %c12{{.*}} : (i64) -> index
! CHECK-DAG: %[[diff1:.*]] = arith.subi %[[c12_idx]], %[[c3_idx]] : index
- ! CHECK-DAG: %[[ext1:.*]] = arith.addi %[[diff1]], %c1{{.*}} : index
+ ! CHECK-DAG: %[[raw_ext1:.*]] = arith.addi %[[diff1]], %c1{{.*}} : index
+ ! CHECK-DAG: %[[cmp1:.*]] = arith.cmpi sgt, %[[raw_ext1]], %c0{{.*}} : index
+ ! CHECK-DAG: %[[ext1:.*]] = arith.select %[[cmp1]], %[[raw_ext1]], %c0{{.*}} : index
! CHECK-DAG: %[[addrCast:.*]] = fir.convert %[[x]] : (!fir.ref<!fir.array<100xf32>>) -> !fir.ref<!fir.array<?x?xf32>>
! CHECK: %[[shape:.*]] = fir.shape_shift %c2{{.*}}, %[[ext0]], %c3{{.*}}, %[[ext1]]
! CHECK: %[[box:.*]] = fir.embox %[[addrCast]](%[[shape]]) : (!fir.ref<!fir.array<?x?xf32>>, !fir.shapeshift<2>) -> !fir.box<!fir.ptr<!fir.array<?x?xf32>>>
@@ -132,9 +136,9 @@ subroutine test_array_char_remap(p, x)
character(*), target :: x(100)
character(:), pointer :: p(:, :)
! CHECK: subi
- ! CHECK: %[[ext0:.*]] = arith.addi
+ ! CHECK: %[[ext0:.*]] = arith.select
! CHECK: subi
- ! CHECK: %[[ext1:.*]] = arith.addi
+ ! CHECK: %[[ext1:.*]] = arith.select
! CHECK: %[[shape:.*]] = fir.shape_shift %c2{{.*}}, %[[ext0]], %c3{{.*}}, %[[ext1]]
! CHECK: %[[box:.*]] = fir.embox %{{.*}}(%[[shape]]) typeparams %[[unbox]]#1 : (!fir.ref<!fir.array<?x?x!fir.char<1,?>>>, !fir.shapeshift<2>, index) -> !fir.box<!fir.ptr<!fir.array<?x?x!fir.char<1,?>>>>
! CHECK: fir.store %[[box]] to %[[p]]
@@ -218,9 +222,9 @@ subroutine test_array_non_contig_remap(p, x)
real, target :: x(:)
real, pointer :: p(:, :)
! CHECK: subi
- ! CHECK: %[[ext0:.*]] = arith.addi
+ ! CHECK: %[[ext0:.*]] = arith.select
! CHECK: subi
- ! CHECK: %[[ext1:.*]] = arith.addi
+ ! CHECK: %[[ext1:.*]] = arith.select
! CHECK: %[[shape:.*]] = fir.shape_shift %{{.*}}, %[[ext0]], %{{.*}}, %[[ext1]]
! CHECK: %[[rebox:.*]] = fir.rebox %[[x]](%[[shape]]) : (!fir.box<!fir.array<?xf32>>, !fir.shapeshift<2>) -> !fir.box<!fir.ptr<!fir.array<?x?xf32>>>
! CHECK: fir.store %[[rebox]] to %[[p]] : !fir.ref<!fir.box<!fir.ptr<!fir.array<?x?xf32>>>>
@@ -250,13 +254,17 @@ subroutine test_array_non_contig_remap(p, x)
! CHECK: %[[VAL_18:.*]] = fir.convert %[[VAL_4]] : (i64) -> index
! CHECK: %[[VAL_19:.*]] = arith.subi %[[VAL_18]], %[[VAL_17]] : index
! CHECK: %[[VAL_20:.*]] = arith.addi %[[VAL_19]], %[[VAL_16]] : index
+! CHECK: %[[cmp0:.*]] = arith.cmpi sgt, %[[VAL_20]], %c0{{.*}} : index
+! CHECK: %[[ext0:.*]] = arith.select %[[cmp0]], %[[VAL_20]], %c0{{.*}} : index
! CHECK: %[[VAL_21:.*]] = fir.convert %[[VAL_5]] : (i64) -> index
! CHECK: %[[VAL_22:.*]] = fir.convert %[[VAL_6]] : (i64) -> index
! CHECK: %[[VAL_23:.*]] = arith.subi %[[VAL_22]], %[[VAL_21]] : index
! CHECK: %[[VAL_24:.*]] = arith.addi %[[VAL_23]], %[[VAL_16]] : index
+! CHECK: %[[cmp1:.*]] = arith.cmpi sgt, %[[VAL_24]], %c0{{.*}} : index
+! CHECK: %[[ext1:.*]] = arith.select %[[cmp1]], %[[VAL_24]], %c0{{.*}} : index
! CHECK: %[[VAL_25:.*]] = fir.convert %[[VAL_3]] : (i64) -> index
! CHECK: %[[VAL_26:.*]] = fir.convert %[[VAL_5]] : (i64) -> index
-! CHECK: %[[VAL_27:.*]] = fir.shape_shift %[[VAL_25]], %[[VAL_20]], %[[VAL_26]], %[[VAL_24]] : (index, index, index, index) -> !fir.shapeshift<2>
+! CHECK: %[[VAL_27:.*]] = fir.shape_shift %[[VAL_25]], %[[ext0]], %[[VAL_26]], %[[ext1]] : (index, index, index, index) -> !fir.shapeshift<2>
! CHECK: %[[VAL_28:.*]] = fir.rebox %[[VAL_15]](%[[VAL_27]]) : (!fir.box<!fir.array<100xf32>>, !fir.shapeshift<2>) -> !fir.box<!fir.ptr<!fir.array<?x?xf32>>>
! CHECK: fir.store %[[VAL_28]] to %[[VAL_0]] : !fir.ref<!fir.box<!fir.ptr<!fir.array<?x?xf32>>>>
! CHECK: return
@@ -333,7 +341,9 @@ subroutine issue857_array_remap(rhs)
! CHECK: %[[c101:.*]] = fir.convert %c101_i64 : (i64) -> index
! CHECK: %[[c200:.*]] = fir.convert %c200_i64 : (i64) -> index
! CHECK: %[[sub:.*]] = arith.subi %[[c200]], %[[c101]] : index
- ! CHECK: %[[extent:.*]] = arith.addi %[[sub]], %c1{{.*}} : index
+ ! CHECK: %[[raw_extent:.*]] = arith.addi %[[sub]], %c1{{.*}} : index
+ ! CHECK: %[[cmp:.*]] = arith.cmpi sgt, %[[raw_extent]], %c0{{.*}} : index
+ ! CHECK: %[[extent:.*]] = arith.select %[[cmp]], %[[raw_extent]], %c0{{.*}} : index
! CHECK: %[[addr:.*]] = fir.box_addr %{{.*}} : (!fir.box<!fir.ptr<!fir.array<?x?x!fir.type<_QFissue857_array_remapTt{i:i32}>>>>) -> !fir.ptr<!fir.array<?x?x!fir.type<_QFissue857_array_remapTt{i:i32}>>>
! CHECK: %[[addr_cast:.*]] = fir.convert %[[addr]] : (!fir.ptr<!fir.array<?x?x!fir.type<_QFissue857_array_remapTt{i:i32}>>>) -> !fir.ptr<!fir.array<?x!fir.type<_QFissue857_array_remapTt{i:i32}>>>
! CHECK: fir.store %[[addr_cast]] to %[[lhs_addr]] : !fir.ref<!fir.ptr<!fir.array<?x!fir.type<_QFissue857_array_remapTt{i:i32}>>>>
|
|
@jeanPerier I updated the description so it will link to the issue automatically and close it when the PR is merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #153221.
Canonicalize the new shape of the pointer when lowering pointer assignment with bounds remapping.
This is done by using the existing helper that generates a compare to zero + select like in the other situation where shapes are lowered.
Note that this only needs to be done for the extents here because lower bounds are canonicalized in LBOUND inquiries and descriptor creation (embox/rebox codegen) based on the extent value.