[flang] Match the type of the element size in the box in creating fir.box_elesize #100512
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-codegen @llvm/pr-subscribers-flang-fir-hlfir Author: Kelvin Li (kkwli) ChangesCurrently, The type of the element size is Full diff: https://github.com/llvm/llvm-project/pull/100512.diff 4 Files Affected:
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 0e5e30a7024d8..2d52ba1007496 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -6731,7 +6731,9 @@ IntrinsicLibrary::genStorageSize(mlir::Type resultType,
box = builder.createBox(loc, args[0],
/*isPolymorphic=*/args[0].isPolymorphic());
- mlir::Value eleSize = builder.create<fir::BoxEleSizeOp>(loc, kindTy, box);
+ mlir::Type i64Ty = builder.getIntegerType(64);
+ mlir::Value boxEleSize = builder.create<fir::BoxEleSizeOp>(loc, i64Ty, box);
+ mlir::Value eleSize = builder.createConvert(loc, kindTy, boxEleSize);
mlir::Value c8 = builder.createIntegerConstant(loc, kindTy, 8);
return builder.create<mlir::arith::MulIOp>(loc, eleSize, c8);
}
diff --git a/flang/test/Lower/HLFIR/assumed-rank-inquiries.f90 b/flang/test/Lower/HLFIR/assumed-rank-inquiries.f90
index a1d150a21d149..4a7035a9fea0a 100644
--- a/flang/test/Lower/HLFIR/assumed-rank-inquiries.f90
+++ b/flang/test/Lower/HLFIR/assumed-rank-inquiries.f90
@@ -191,9 +191,10 @@ subroutine c_loc_2(x)
! CHECK-SAME: %[[VAL_0:.*]]: !fir.class<!fir.array<*:none>> {fir.bindc_name = "x"}) {
! CHECK: %[[VAL_1:.*]] = fir.dummy_scope : !fir.dscope
! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %[[VAL_1]] {uniq_name = "_QFtest_storage_size_1Ex"} : (!fir.class<!fir.array<*:none>>, !fir.dscope) -> (!fir.class<!fir.array<*:none>>, !fir.class<!fir.array<*:none>>)
-! CHECK: %[[VAL_3:.*]] = fir.box_elesize %[[VAL_2]]#0 : (!fir.class<!fir.array<*:none>>) -> i32
+! CHECK: %[[VAL_3:.*]] = fir.box_elesize %[[VAL_2]]#0 : (!fir.class<!fir.array<*:none>>) -> i64
+! CHECK: %[[VAL_3_CONV:.*]] = fir.convert %[[VAL_3]] : (i64) -> i32
! CHECK: %[[VAL_4:.*]] = arith.constant 8 : i32
-! CHECK: %[[VAL_5:.*]] = arith.muli %[[VAL_3]], %[[VAL_4]] : i32
+! CHECK: %[[VAL_5:.*]] = arith.muli %[[VAL_3_CONV]], %[[VAL_4]] : i32
! CHECK: %[[VAL_6:.*]]:3 = hlfir.associate %[[VAL_5]] {adapt.valuebyref} : (i32) -> (!fir.ref<i32>, !fir.ref<i32>, i1)
! CHECK: fir.call @_QPtakes_integer(%[[VAL_6]]#1) fastmath<contract> : (!fir.ref<i32>) -> ()
! CHECK: hlfir.end_associate %[[VAL_6]]#1, %[[VAL_6]]#2 : !fir.ref<i32>, i1
@@ -213,9 +214,10 @@ subroutine c_loc_2(x)
! CHECK: %[[VAL_13:.*]] = fir.call @_FortranAReportFatalUserError
! CHECK: }
! CHECK: %[[VAL_14:.*]] = fir.load %[[VAL_2]]#0 : !fir.ref<!fir.class<!fir.ptr<!fir.array<*:none>>>>
-! CHECK: %[[VAL_15:.*]] = fir.box_elesize %[[VAL_14]] : (!fir.class<!fir.ptr<!fir.array<*:none>>>) -> i32
+! CHECK: %[[VAL_15:.*]] = fir.box_elesize %[[VAL_14]] : (!fir.class<!fir.ptr<!fir.array<*:none>>>) -> i64
+! CHECK: %[[VAL_15_CONV:.*]] = fir.convert %[[VAL_15]] : (i64) -> i32
! CHECK: %[[VAL_16:.*]] = arith.constant 8 : i32
-! CHECK: %[[VAL_17:.*]] = arith.muli %[[VAL_15]], %[[VAL_16]] : i32
+! CHECK: %[[VAL_17:.*]] = arith.muli %[[VAL_15_CONV]], %[[VAL_16]] : i32
! CHECK: %[[VAL_18:.*]]:3 = hlfir.associate %[[VAL_17]] {adapt.valuebyref} : (i32) -> (!fir.ref<i32>, !fir.ref<i32>, i1)
! CHECK: fir.call @_QPtakes_integer(%[[VAL_18]]#1) fastmath<contract> : (!fir.ref<i32>) -> ()
! CHECK: hlfir.end_associate %[[VAL_18]]#1, %[[VAL_18]]#2 : !fir.ref<i32>, i1
diff --git a/flang/test/Lower/Intrinsics/storage_size-2.f90 b/flang/test/Lower/Intrinsics/storage_size-2.f90
index d6fb68df70ea9..7fd26d28a2e09 100644
--- a/flang/test/Lower/Intrinsics/storage_size-2.f90
+++ b/flang/test/Lower/Intrinsics/storage_size-2.f90
@@ -17,9 +17,10 @@ function return_char(l)
! CHECK: %[[expr:.*]] = hlfir.as_expr %[[res]]#0 move %[[false]] : (!fir.boxchar<1>, i1) -> !hlfir.expr<!fir.char<1,?>>
! CHECK: %[[assoc:.*]]:3 = hlfir.associate %[[expr]] typeparams %[[res_len]] {adapt.valuebyref} : (!hlfir.expr<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>, i1)
! CHECK: %[[val_18:.*]] = fir.embox %[[assoc]]#1 typeparams %[[res_len]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
-! CHECK: %[[val_19:.*]] = fir.box_elesize %[[val_18]] : (!fir.box<!fir.char<1,?>>) -> i32
+! CHECK: %[[val_19:.*]] = fir.box_elesize %[[val_18]] : (!fir.box<!fir.char<1,?>>) -> i64
+! CHECK: %[[val_19_conv:.*]] = fir.convert %[[val_19]] : (i64) -> i32
! CHECK: %[[val_20:.*]] = arith.constant 8 : i32
-! CHECK: %[[val_21:.*]] = arith.muli %[[val_19]], %[[val_20]] : i32
+! CHECK: %[[val_21:.*]] = arith.muli %[[val_19_conv]], %[[val_20]] : i32
! CHECK: fir.call @_FortranAioOutputInteger32(%{{.*}}, %[[val_21]])
end subroutine
diff --git a/flang/test/Lower/Intrinsics/storage_size.f90 b/flang/test/Lower/Intrinsics/storage_size.f90
index b0c9d51f95328..24709e4640727 100644
--- a/flang/test/Lower/Intrinsics/storage_size.f90
+++ b/flang/test/Lower/Intrinsics/storage_size.f90
@@ -32,9 +32,10 @@ integer function unlimited_polymorphic_pointer(p) result(size)
! CHECK: %{{.*}} = fir.call @_FortranAReportFatalUserError(%{{.*}}, %{{.*}}, %{{.*}}) {{.*}} : (!fir.ref<i8>, !fir.ref<i8>, i32) -> none
! CHECK: }
! CHECK: %[[LOAD_P:.*]] = fir.load %[[P]] : !fir.ref<!fir.class<!fir.ptr<none>>>
-! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[LOAD_P]] : (!fir.class<!fir.ptr<none>>) -> i32
+! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[LOAD_P]] : (!fir.class<!fir.ptr<none>>) -> i64
+! CHECK: %[[ELE_SIZE_CONV:.*]] = fir.convert %[[ELE_SIZE]] : (i64) -> i32
! CHECK: %[[C8:.*]] = arith.constant 8 : i32
-! CHECK: %[[BITS:.*]] = arith.muli %[[ELE_SIZE]], %[[C8]] : i32
+! CHECK: %[[BITS:.*]] = arith.muli %[[ELE_SIZE_CONV]], %[[C8]] : i32
! CHECK: fir.store %[[BITS]] to %[[SIZE]] : !fir.ref<i32>
! CHECK: %[[RES:.*]] = fir.load %[[SIZE]] : !fir.ref<i32>
! CHECK: return %[[RES]] : i32
@@ -56,9 +57,10 @@ integer function unlimited_polymorphic_allocatable(p) result(size)
! CHECK: %{{.*}} = fir.call @_FortranAReportFatalUserError(%{{.*}}, %{{.*}}, %{{.*}}) {{.*}} : (!fir.ref<i8>, !fir.ref<i8>, i32) -> none
! CHECK: }
! CHECK: %[[LOAD_P:.*]] = fir.load %[[P]] : !fir.ref<!fir.class<!fir.heap<none>>>
-! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[LOAD_P]] : (!fir.class<!fir.heap<none>>) -> i32
+! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[LOAD_P]] : (!fir.class<!fir.heap<none>>) -> i64
+! CHECK: %[[ELE_SIZE_CONV:.*]] = fir.convert %[[ELE_SIZE]] : (i64) -> i32
! CHECK: %[[C8:.*]] = arith.constant 8 : i32
-! CHECK: %[[BITS:.*]] = arith.muli %[[ELE_SIZE]], %[[C8]] : i32
+! CHECK: %[[BITS:.*]] = arith.muli %[[ELE_SIZE_CONV]], %[[C8]] : i32
! CHECK: fir.store %[[BITS]] to %[[SIZE]] : !fir.ref<i32>
! CHECK: %[[RES:.*]] = fir.load %[[SIZE]] : !fir.ref<i32>
! CHECK: return %[[RES]] : i32
@@ -72,9 +74,10 @@ integer function polymorphic_pointer(p) result(size)
! CHECK-SAME: %[[P:.*]]: !fir.ref<!fir.class<!fir.ptr<!fir.type<_QMstorage_size_testTp1{a:i32}>>>> {fir.bindc_name = "p"}) -> i32 {
! CHECK: %[[SIZE:.*]] = fir.alloca i32 {bindc_name = "size", uniq_name = "_QMstorage_size_testFpolymorphic_pointerEsize"}
! CHECK: %[[LOAD_P:.*]] = fir.load %[[P]] : !fir.ref<!fir.class<!fir.ptr<!fir.type<_QMstorage_size_testTp1{a:i32}>>>>
-! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[LOAD_P]] : (!fir.class<!fir.ptr<!fir.type<_QMstorage_size_testTp1{a:i32}>>>) -> i32
+! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[LOAD_P]] : (!fir.class<!fir.ptr<!fir.type<_QMstorage_size_testTp1{a:i32}>>>) -> i64
+! CHECK: %[[ELE_SIZE_CONV:.*]] = fir.convert %[[ELE_SIZE]] : (i64) -> i32
! CHECK: %[[C8:.*]] = arith.constant 8 : i32
-! CHECK: %[[BITS:.*]] = arith.muli %[[ELE_SIZE]], %[[C8]] : i32
+! CHECK: %[[BITS:.*]] = arith.muli %[[ELE_SIZE_CONV]], %[[C8]] : i32
! CHECK: fir.store %[[BITS]] to %[[SIZE]] : !fir.ref<i32>
! CHECK: %[[RES:.*]] = fir.load %[[SIZE]] : !fir.ref<i32>
! CHECK: return %[[RES]] : i32
@@ -87,9 +90,10 @@ integer function polymorphic(p) result(size)
! CHECK-LABEL: func.func @_QMstorage_size_testPpolymorphic(
! CHECK-SAME: %[[P:.*]]: !fir.class<!fir.type<_QMstorage_size_testTp1{a:i32}>> {fir.bindc_name = "p"}) -> i32 {
! CHECK: %[[SIZE:.*]] = fir.alloca i32 {bindc_name = "size", uniq_name = "_QMstorage_size_testFpolymorphicEsize"}
-! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[P]] : (!fir.class<!fir.type<_QMstorage_size_testTp1{a:i32}>>) -> i32
+! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[P]] : (!fir.class<!fir.type<_QMstorage_size_testTp1{a:i32}>>) -> i64
+! CHECK: %[[ELE_SIZE_CONV:.*]] = fir.convert %[[ELE_SIZE]] : (i64) -> i32
! CHECK: %[[C8:.*]] = arith.constant 8 : i32
-! CHECK: %[[BITS:.*]] = arith.muli %[[ELE_SIZE]], %[[C8]] : i32
+! CHECK: %[[BITS:.*]] = arith.muli %[[ELE_SIZE_CONV]], %[[C8]] : i32
! CHECK: fir.store %[[BITS]] to %[[SIZE]] : !fir.ref<i32>
! CHECK: %[[RES:.*]] = fir.load %[[SIZE]] : !fir.ref<i32>
! CHECK: return %[[RES]] : i32
@@ -127,9 +131,10 @@ integer function polymorphic_value(t) result(size)
! CHECK: %[[IDX:.*]] = arith.subi %[[C1]], %[[DIMI64]] : i64
! CHECK: %[[COORD_OF:.*]] = fir.coordinate_of %[[LOAD_COORD_P]], %[[IDX]] : (!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QMstorage_size_testTp1{a:i32}>>>>, i64) -> !fir.ref<!fir.type<_QMstorage_size_testTp1{a:i32}>>
! CHECK: %[[BOXED:.*]] = fir.embox %[[COORD_OF]] source_box %[[LOAD_COORD_P]] : (!fir.ref<!fir.type<_QMstorage_size_testTp1{a:i32}>>, !fir.class<!fir.ptr<!fir.array<?x!fir.type<_QMstorage_size_testTp1{a:i32}>>>>) -> !fir.class<!fir.type<_QMstorage_size_testTp1{a:i32}>>
-! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[BOXED]] : (!fir.class<!fir.type<_QMstorage_size_testTp1{a:i32}>>) -> i32
+! CHECK: %[[ELE_SIZE:.*]] = fir.box_elesize %[[BOXED]] : (!fir.class<!fir.type<_QMstorage_size_testTp1{a:i32}>>) -> i64
+! CHECK: %[[ELE_SIZE_CONV:.*]] = fir.convert %[[ELE_SIZE]] : (i64) -> i32
! CHECK: %[[C8:.*]] = arith.constant 8 : i32
-! CHECK: %[[SIZE:.*]] = arith.muli %[[ELE_SIZE]], %[[C8]] : i32
+! CHECK: %[[SIZE:.*]] = arith.muli %[[ELE_SIZE_CONV]], %[[C8]] : i32
! CHECK: fir.store %[[SIZE]] to %[[ALLOCA]] : !fir.ref<i32>
! CHECK: %[[RET:.*]] = fir.load %[[ALLOCA]] : !fir.ref<i32>
! CHECK: return %[[RET]] : i32
|
jeanPerier
requested changes
Jul 25, 2024
There was a problem hiding this comment.
Thanks for finding this bug.
I would rather the bug to be fixed where it is in codegen.cpp: the GEP + load should use the descriptor type and a cast should be inserted. That way, there is no need to assume the type inside the descriptor in lowering, and future creation of fir.box_elesize will be safe.
|
@jeanPerier Thanks for the review. I update the patch. Instead of fixing it in |
|
Ping |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently,
%17 = fir.box_elesize %16 : (!fir.class<!fir.ptr<!fir.type<_QFTt{a:i32,b:i32}>>>) -> i32is translated to
The type of the element size is
i64. The load essentially truncates the value and yields incorrect result in the big endian environment. The problem occurs in thestorage_sizeintrinsic on a polymorphic variable.