From 4489ef8e34fefcbbf4fe44d1307b0cc41a12cfa8 Mon Sep 17 00:00:00 2001 From: Valentin Clement Date: Fri, 24 Jun 2022 21:01:25 +0200 Subject: [PATCH] [flang] Fix LBOUND with assumed size array and non constant DIM LBOUND with a non constant DIM argument use the runtime to allow runtime verification of DIM <= RANK. The interface uses a descriptor. This caused undefined behavior because the runtime believed it was seeing an explicit shape arrays with zero extent and returned `1` (the runtime descriptor does not allow making a difference between an explicit shape and an assumed size. Assumed size are not meant to be described by runtime descriptors). Fix the issue by setting the last extent of assumed size to `1` when creating the descriptor to inquire about the LBOUND with the runtime. This patch is part of the upstreaming effort from fir-dev branch. Reviewed By: PeteSteinfeld Differential Revision: https://reviews.llvm.org/D128509 Co-authored-by: Jean Perier --- flang/lib/Lower/IntrinsicCall.cpp | 74 +++++++++++++++++++++++++--------- flang/test/Lower/Intrinsics/lbound.f90 | 2 +- 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/flang/lib/Lower/IntrinsicCall.cpp b/flang/lib/Lower/IntrinsicCall.cpp index ba22c24..918d575 100644 --- a/flang/lib/Lower/IntrinsicCall.cpp +++ b/flang/lib/Lower/IntrinsicCall.cpp @@ -2557,7 +2557,9 @@ void IntrinsicLibrary::genGetEnvironmentVariable( mlir::Value IntrinsicLibrary::genIand(mlir::Type resultType, llvm::ArrayRef args) { assert(args.size() == 2); - return builder.create(loc, args[0], args[1]); + auto arg0 = builder.createConvert(loc, resultType, args[0]); + auto arg1 = builder.createConvert(loc, resultType, args[1]); + return builder.create(loc, arg0, arg1); } // IBCLR @@ -3456,6 +3458,56 @@ static mlir::Value computeLBOUND(fir::FirOpBuilder &builder, mlir::Location loc, return builder.create(loc, dimIsEmpty, one, lb); } +/// Create a fir.box to be passed to the LBOUND runtime. +/// This ensure that local lower bounds of assumed shape are propagated and that +/// a fir.box with equivalent LBOUNDs but an explicit shape is created for +/// assumed size arrays to avoid undefined behaviors in codegen or the runtime. +static mlir::Value createBoxForLBOUND(mlir::Location loc, + fir::FirOpBuilder &builder, + const fir::ExtendedValue &array) { + if (!array.isAssumedSize()) + return array.match( + [&](const fir::BoxValue &boxValue) -> mlir::Value { + // This entity is mapped to a fir.box that may not contain the local + // lower bound information if it is a dummy. Rebox it with the local + // shape information. + mlir::Value localShape = builder.createShape(loc, array); + mlir::Value oldBox = boxValue.getAddr(); + return builder.create(loc, oldBox.getType(), oldBox, + localShape, + /*slice=*/mlir::Value{}); + }, + [&](const auto &) -> mlir::Value { + // This a pointer/allocatable, or an entity not yet tracked with a + // fir.box. For pointer/allocatable, createBox will forward the + // descriptor that contains the correct lower bound information. For + // other entities, a new fir.box will be made with the local lower + // bounds. + return builder.createBox(loc, array); + }); + // Assumed sized are not meant to be emboxed. This could cause the undefined + // extent cannot safely be understood by the runtime/codegen that will + // consider that the dimension is empty and that the related LBOUND value must + // be one. Pretend that the related extent is one to get the correct LBOUND + // value. + llvm::SmallVector shape = + fir::factory::getExtents(loc, builder, array); + assert(!shape.empty() && "assumed size must have at least one dimension"); + shape.back() = builder.createIntegerConstant(loc, builder.getIndexType(), 1); + auto safeToEmbox = array.match( + [&](const fir::CharArrayBoxValue &x) -> fir::ExtendedValue { + return fir::CharArrayBoxValue{x.getAddr(), x.getLen(), shape, + x.getLBounds()}; + }, + [&](const fir::ArrayBoxValue &x) -> fir::ExtendedValue { + return fir::ArrayBoxValue{x.getAddr(), shape, x.getLBounds()}; + }, + [&](const auto &) -> fir::ExtendedValue { + fir::emitFatalError(loc, "not an assumed size array"); + }); + return builder.createBox(loc, safeToEmbox); +} + // LBOUND fir::ExtendedValue IntrinsicLibrary::genLbound(mlir::Type resultType, @@ -3506,25 +3558,7 @@ IntrinsicLibrary::genLbound(mlir::Type resultType, return builder.createConvert(loc, resultType, lb); } - mlir::Value box = array.match( - [&](const fir::BoxValue &boxValue) -> mlir::Value { - // This entity is mapped to a fir.box that may not contain the local - // lower bound information if it is a dummy. Rebox it with the local - // shape information. - mlir::Value localShape = builder.createShape(loc, array); - mlir::Value oldBox = boxValue.getAddr(); - return builder.create( - loc, oldBox.getType(), oldBox, localShape, /*slice=*/mlir::Value{}); - }, - [&](const auto &) -> mlir::Value { - // This a pointer/allocatable, or an entity not yet tracked with a - // fir.box. For pointer/allocatable, createBox will forward the - // descriptor that contains the correct lower bound information. For - // other entities, a new fir.box will be made with the local lower - // bounds. - return builder.createBox(loc, array); - }); - + fir::ExtendedValue box = createBoxForLBOUND(loc, builder, array); return builder.createConvert( loc, resultType, fir::runtime::genLboundDim(builder, loc, fir::getBase(box), dim)); diff --git a/flang/test/Lower/Intrinsics/lbound.f90 b/flang/test/Lower/Intrinsics/lbound.f90 index 2a84d76..3ea0880 100644 --- a/flang/test/Lower/Intrinsics/lbound.f90 +++ b/flang/test/Lower/Intrinsics/lbound.f90 @@ -37,11 +37,11 @@ subroutine lbound_test_2(a, dim, res) res = lbound(a, dim, 8) end subroutine -! CHECK: %[[VAL_0:.*]] = fir.undefined index subroutine lbound_test_3(a, dim, res) real, dimension(2:10, 3:*) :: a integer(8):: dim, res ! CHECK: %[[VAL_1:.*]] = fir.load %arg1 : !fir.ref +! CHECK: %[[VAL_0:.*]] = arith.constant 1 : index ! CHECK: %[[VAL_2:.*]] = fir.shape_shift %{{.*}}, %{{.*}}, %{{.*}}, %[[VAL_0]] : (index, index, index, index) -> !fir.shapeshift<2> ! CHECK: %[[VAL_3:.*]] = fir.embox %arg0(%[[VAL_2]]) : (!fir.ref>, !fir.shapeshift<2>) -> !fir.box> ! CHECK: %[[VAL_4:.*]] = fir.address_of( -- 2.7.4