From: Jean Perier Date: Fri, 31 Mar 2023 07:13:52 +0000 (+0200) Subject: [flang] Fix context less NULL() lowering X-Git-Tag: upstream/17.0.6~13108 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cb3f1e2d12c61c650f9871198499f248a19f59d2;p=platform%2Fupstream%2Fllvm.git [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 allocation, this code fix this by allocating an actual fir.box. 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 fir.call @foo(%absent) ``` Differential Revision: https://reviews.llvm.org/D147239 --- diff --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp index b9d217c..becaf13 100644 --- a/flang/lib/Lower/ConvertExpr.cpp +++ b/flang/lib/Lower/ConvertExpr.cpp @@ -711,20 +711,22 @@ public: } /// 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(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(loc, boxType, nullConst); + builder.create(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 e7dafba..749df62 100644 --- a/flang/test/Lower/dummy-argument-optional.f90 +++ b/flang/test/Lower/dummy-argument-optional.f90 @@ -195,9 +195,10 @@ end subroutine ! CHECK-LABEL: func @_QMoptPnull_as_optional() { subroutine null_as_optional - ! CHECK: %[[temp:.*]] = fir.alloca !fir.llvm_ptr - ! CHECK: %[[null:.*]] = fir.zero_bits !fir.ref - ! CHECK: fir.store %{{.*}} to %[[temp]] : !fir.ref> + ! CHECK: %[[null_ptr:.*]] = fir.alloca !fir.box> + ! CHECK: %[[null:.*]] = fir.zero_bits !fir.ptr + ! CHECK: %[[null_box:.*]] = fir.embox %[[null]] : (!fir.ptr) -> !fir.box> + ! CHECK: fir.store %[[null_box]] to %[[null_ptr]] : !fir.ref>> ! CHECK: fir.call @_QMoptPassumed_shape(%{{.*}}) {{.*}}: (!fir.box>) -> () call assumed_shape(null()) end subroutine null_as_optional diff --git a/flang/test/Lower/polymorphic.f90 b/flang/test/Lower/polymorphic.f90 index ccc3d8699..ff9dacc 100644 --- a/flang/test/Lower/polymorphic.f90 +++ b/flang/test/Lower/polymorphic.f90 @@ -843,15 +843,19 @@ module polymorphic_test end subroutine ! CHECK-LABEL: func.func @_QMpolymorphic_testPtest_call_with_null() { -! CHECK: %[[NULL_LLVM_PTR:.*]] = fir.alloca !fir.llvm_ptr -! CHECK: %[[NULL_BOX_NONE:.*]] = fir.convert %[[NULL_LLVM_PTR]] : (!fir.ref>) -> !fir.ref> -! CHECK: %[[BOX_NONE:.*]] = fir.load %[[NULL_BOX_NONE]] : !fir.ref> -! CHECK: %[[BOX_ADDR:.*]] = fir.box_addr %[[BOX_NONE]] : (!fir.box) -> !fir.ref -! CHECK: %[[BOX_ADDR_I64:.*]] = fir.convert %[[BOX_ADDR]] : (!fir.ref) -> i64 +! CHECK: %[[NULL_PTR:.*]] = fir.alloca !fir.box> +! CHECK: %[[NULL:.*]] = fir.zero_bits !fir.ptr +! CHECK: %[[NULL_BOX:.*]] = fir.embox %[[NULL]] : (!fir.ptr) -> !fir.box> +! CHECK: fir.store %[[NULL_BOX]] to %[[NULL_PTR]] : !fir.ref>> +! CHECK: %[[BOX_NONE:.*]] = fir.load %[[NULL_PTR]] : !fir.ref>> +! CHECK: %[[BOX_ADDR:.*]] = fir.box_addr %[[BOX_NONE]] : (!fir.box>) -> !fir.ptr +! CHECK: %[[BOX_ADDR_I64:.*]] = fir.convert %[[BOX_ADDR]] : (!fir.ptr) -> 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 -! CHECK: %[[BOX_NONE:.*]] = fir.load %[[NULL_BOX_NONE]] : !fir.ref> +! CHECK: %[[PTR_LOAD2:.*]] = fir.load %[[NULL_PTR]] : !fir.ref>> +! CHECK: %[[BOX_ADDR2:.*]] = fir.box_addr %[[PTR_LOAD2]] : (!fir.box>) -> !fir.ptr +! CHECK: %[[BOX_NONE:.*]] = fir.embox %[[BOX_ADDR2]] : (!fir.ptr) -> !fir.box ! CHECK: %[[CLASS_NONE:.*]] = fir.convert %[[BOX_NONE]] : (!fir.box) -> !fir.class ! CHECK: %[[ARG:.*]] = arith.select %[[IS_ALLOCATED_OR_ASSOCIATED]], %[[CLASS_NONE]], %[[ABSENT]] : !fir.class ! CHECK: fir.call @_QMpolymorphic_testPsub_with_poly_optional(%[[ARG]]) {{.*}} : (!fir.class) -> ()