[flang-commits] [flang] cb3f1e2 - [flang] Fix context less NULL() lowering
Jean Perier via flang-commits
flang-commits at lists.llvm.org
Fri Mar 31 00:15:20 PDT 2023
Author: Jean Perier
Date: 2023-03-31T09:13:59+02:00
New Revision: cb3f1e2d12c61c650f9871198499f248a19f59d2
URL: https://github.com/llvm/llvm-project/commit/cb3f1e2d12c61c650f9871198499f248a19f59d2
DIFF: https://github.com/llvm/llvm-project/commit/cb3f1e2d12c61c650f9871198499f248a19f59d2.diff
LOG: [flang] Fix context less NULL() lowering
The current context less lowering of NULL is producing invalid code
(can lead to reading outside of allocated memory): it is casting
a simple pointer to a descriptor address.
Later, reads are made to this descriptor. It used to be "OK" when
fir.load of fir.box were no-ops, but this was incorrect, and the
fir.load codegen is known doing a copy, and read the whole descriptor
data, not only the base address.
The previous patch that allowed fir.box<None> allocation, this
code fix this by allocating an actual fir.box<None>.
Note: this is still an overkill way to lower foo(null()). HLFIR
lowering always contextualize NULL() lowering leading to much simpler
code:
```
%absent = fir.absent fir.box<T>
fir.call @foo(%absent)
```
Differential Revision: https://reviews.llvm.org/D147239
Added:
Modified:
flang/lib/Lower/ConvertExpr.cpp
flang/test/Lower/dummy-argument-optional.f90
flang/test/Lower/polymorphic.f90
Removed:
################################################################################
diff --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp
index b9d217cc4489..becaf13dd3ec 100644
--- a/flang/lib/Lower/ConvertExpr.cpp
+++ b/flang/lib/Lower/ConvertExpr.cpp
@@ -711,20 +711,22 @@ class ScalarExprLowering {
}
/// A `NULL()` in a position where a mutable box is expected has the same
- /// semantics as an absent optional box value.
+ /// semantics as an absent optional box value. Note: this code should
+ /// be depreciated because the rank information is not known here. A
+ /// scalar fir.box is created: it should not be cast to an array box type
+ /// later, but there is no way to enforce that here.
ExtValue genMutableBoxValueImpl(const Fortran::evaluate::NullPointer &) {
mlir::Location loc = getLoc();
- auto nullConst = builder.createNullConstant(loc);
- auto noneTy = mlir::NoneType::get(builder.getContext());
- auto polyRefTy = fir::LLVMPointerType::get(noneTy);
- // MutableBoxValue will dereference the box, so create a bogus temporary for
- // the `nullptr`. The LLVM optimizer will garbage collect the temp.
- auto temp =
- builder.createTemporary(loc, polyRefTy, /*shape=*/mlir::ValueRange{});
- auto nullPtr = builder.createConvert(loc, polyRefTy, nullConst);
- builder.create<fir::StoreOp>(loc, nullPtr, temp);
+ mlir::Type noneTy = mlir::NoneType::get(builder.getContext());
+ mlir::Type polyRefTy = fir::PointerType::get(noneTy);
+ mlir::Type boxType = fir::BoxType::get(polyRefTy);
+ mlir::Value nullConst = builder.createNullConstant(loc, polyRefTy);
+ mlir::Value tempBox =
+ builder.createTemporary(loc, boxType, /*shape=*/mlir::ValueRange{});
+ mlir::Value nullBox = builder.create<fir::EmboxOp>(loc, boxType, nullConst);
+ builder.create<fir::StoreOp>(loc, nullBox, tempBox);
auto nullBoxTy = builder.getRefType(fir::BoxType::get(noneTy));
- return fir::MutableBoxValue(builder.createConvert(loc, nullBoxTy, temp),
+ return fir::MutableBoxValue(tempBox,
/*lenParameters=*/mlir::ValueRange{},
/*mutableProperties=*/{});
}
diff --git a/flang/test/Lower/dummy-argument-optional.f90 b/flang/test/Lower/dummy-argument-optional.f90
index e7dafbaea09c..749df6277add 100644
--- a/flang/test/Lower/dummy-argument-optional.f90
+++ b/flang/test/Lower/dummy-argument-optional.f90
@@ -195,9 +195,10 @@ subroutine alloc_component_eval_only_once(x)
! CHECK-LABEL: func @_QMoptPnull_as_optional() {
subroutine null_as_optional
- ! CHECK: %[[temp:.*]] = fir.alloca !fir.llvm_ptr<none>
- ! CHECK: %[[null:.*]] = fir.zero_bits !fir.ref<none>
- ! CHECK: fir.store %{{.*}} to %[[temp]] : !fir.ref<!fir.llvm_ptr<none>>
+ ! CHECK: %[[null_ptr:.*]] = fir.alloca !fir.box<!fir.ptr<none>>
+ ! CHECK: %[[null:.*]] = fir.zero_bits !fir.ptr<none>
+ ! CHECK: %[[null_box:.*]] = fir.embox %[[null]] : (!fir.ptr<none>) -> !fir.box<!fir.ptr<none>>
+ ! CHECK: fir.store %[[null_box]] to %[[null_ptr]] : !fir.ref<!fir.box<!fir.ptr<none>>>
! CHECK: fir.call @_QMoptPassumed_shape(%{{.*}}) {{.*}}: (!fir.box<!fir.array<?xf32>>) -> ()
call assumed_shape(null())
end subroutine null_as_optional
diff --git a/flang/test/Lower/polymorphic.f90 b/flang/test/Lower/polymorphic.f90
index ccc3d8699861..ff9dacc7b7d0 100644
--- a/flang/test/Lower/polymorphic.f90
+++ b/flang/test/Lower/polymorphic.f90
@@ -843,15 +843,19 @@ subroutine test_call_with_null()
end subroutine
! CHECK-LABEL: func.func @_QMpolymorphic_testPtest_call_with_null() {
-! CHECK: %[[NULL_LLVM_PTR:.*]] = fir.alloca !fir.llvm_ptr<none>
-! CHECK: %[[NULL_BOX_NONE:.*]] = fir.convert %[[NULL_LLVM_PTR]] : (!fir.ref<!fir.llvm_ptr<none>>) -> !fir.ref<!fir.box<none>>
-! CHECK: %[[BOX_NONE:.*]] = fir.load %[[NULL_BOX_NONE]] : !fir.ref<!fir.box<none>>
-! CHECK: %[[BOX_ADDR:.*]] = fir.box_addr %[[BOX_NONE]] : (!fir.box<none>) -> !fir.ref<none>
-! CHECK: %[[BOX_ADDR_I64:.*]] = fir.convert %[[BOX_ADDR]] : (!fir.ref<none>) -> i64
+! CHECK: %[[NULL_PTR:.*]] = fir.alloca !fir.box<!fir.ptr<none>>
+! CHECK: %[[NULL:.*]] = fir.zero_bits !fir.ptr<none>
+! CHECK: %[[NULL_BOX:.*]] = fir.embox %[[NULL]] : (!fir.ptr<none>) -> !fir.box<!fir.ptr<none>>
+! CHECK: fir.store %[[NULL_BOX]] to %[[NULL_PTR]] : !fir.ref<!fir.box<!fir.ptr<none>>>
+! CHECK: %[[BOX_NONE:.*]] = fir.load %[[NULL_PTR]] : !fir.ref<!fir.box<!fir.ptr<none>>>
+! CHECK: %[[BOX_ADDR:.*]] = fir.box_addr %[[BOX_NONE]] : (!fir.box<!fir.ptr<none>>) -> !fir.ptr<none>
+! CHECK: %[[BOX_ADDR_I64:.*]] = fir.convert %[[BOX_ADDR]] : (!fir.ptr<none>) -> i64
! CHECK: %[[C0:.*]] = arith.constant 0 : i64
! CHECK: %[[IS_ALLOCATED_OR_ASSOCIATED:.*]] = arith.cmpi ne, %[[BOX_ADDR_I64]], %[[C0]] : i64
! CHECK: %[[ABSENT:.*]] = fir.absent !fir.class<none>
-! CHECK: %[[BOX_NONE:.*]] = fir.load %[[NULL_BOX_NONE]] : !fir.ref<!fir.box<none>>
+! CHECK: %[[PTR_LOAD2:.*]] = fir.load %[[NULL_PTR]] : !fir.ref<!fir.box<!fir.ptr<none>>>
+! CHECK: %[[BOX_ADDR2:.*]] = fir.box_addr %[[PTR_LOAD2]] : (!fir.box<!fir.ptr<none>>) -> !fir.ptr<none>
+! CHECK: %[[BOX_NONE:.*]] = fir.embox %[[BOX_ADDR2]] : (!fir.ptr<none>) -> !fir.box<none>
! CHECK: %[[CLASS_NONE:.*]] = fir.convert %[[BOX_NONE]] : (!fir.box<none>) -> !fir.class<none>
! CHECK: %[[ARG:.*]] = arith.select %[[IS_ALLOCATED_OR_ASSOCIATED]], %[[CLASS_NONE]], %[[ABSENT]] : !fir.class<none>
! CHECK: fir.call @_QMpolymorphic_testPsub_with_poly_optional(%[[ARG]]) {{.*}} : (!fir.class<none>) -> ()
More information about the flang-commits
mailing list