Skip to content

Commit

Permalink
[flang][hlfir] Fixed length-one assignment.
Browse files Browse the repository at this point in the history
Assignment from a character dummy argument to a length-one character
variable resulted in illegal fir.convert:
  %0 = fir.load %unboxed_dummy : !fir.ref<!fir.char<1,?>>
  %1 = fir.convert %0 : (!fir.char<1,?>) -> !fir.char<1>
  fir.store %1 to %local : !fir.ref<!fir.char<1>>

This change fixes the length-one assignment code to use proper casts.

For character dummy arguments with constant length we will now also
type cast the unboxed reference to the character type with constant length
during the lowering:
  fir.convert %x : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1,8>>

I also adjusted the length-one assignment recognition so that in case
of same-length assignment we recognize length-one from either LHS or RHS
data types.

Reviewed By: jeanPerier

Differential Revision: https://reviews.llvm.org/D149382
  • Loading branch information
vzakhari committed Apr 28, 2023
1 parent 6db45cc commit 8df5913
Show file tree
Hide file tree
Showing 21 changed files with 356 additions and 260 deletions.
13 changes: 7 additions & 6 deletions flang/lib/Lower/ConvertVariable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1840,14 +1840,15 @@ void Fortran::lower::mapSymbolAttributes(
}
}

if (addr && addr.getDefiningOp<fir::UnboxCharOp>()) {
// Ensure proper type is given to array/scalar that transited via
// fir.boxchar arg.
mlir::Type castTy = builder.getRefType(converter.genType(var));
addr = builder.createConvert(loc, castTy, addr);
}

// Compute array extents and lower bounds.
if (ba.isArray()) {
if (addr && addr.getDefiningOp<fir::UnboxCharOp>()) {
// Ensure proper type is given to array that transited via fir.boxchar
// arg.
mlir::Type castTy = builder.getRefType(converter.genType(var));
addr = builder.createConvert(loc, castTy, addr);
}
if (ba.isStaticArray()) {
if (ba.lboundIsAllOnes()) {
for (std::int64_t extent :
Expand Down
45 changes: 36 additions & 9 deletions flang/lib/Optimizer/Builder/Character.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,10 +400,19 @@ void fir::factory::CharacterExprHelper::createLengthOneAssign(
auto addr = lhs.getBuffer();
auto toTy = fir::unwrapRefType(addr.getType());
mlir::Value val = rhs.getBuffer();
if (fir::isa_ref_type(val.getType()))
val = builder.create<fir::LoadOp>(loc, val);
val = builder.createConvert(loc, toTy, val);
builder.create<fir::StoreOp>(loc, val, addr);
if (fir::isa_ref_type(val.getType())) {
auto fromCharLen1RefTy = builder.getRefType(getSingletonCharType(
builder.getContext(),
getCharacterKind(fir::unwrapRefType(val.getType()))));
val = builder.create<fir::LoadOp>(
loc, builder.createConvert(loc, fromCharLen1RefTy, val));
}
auto toCharLen1Ty =
getSingletonCharType(builder.getContext(), getCharacterKind(toTy));
val = builder.createConvert(loc, toCharLen1Ty, val);
builder.create<fir::StoreOp>(
loc, val,
builder.createConvert(loc, builder.getRefType(toCharLen1Ty), addr));
}

/// Returns the minimum of integer mlir::Value \p a and \b.
Expand All @@ -418,11 +427,29 @@ void fir::factory::CharacterExprHelper::createAssign(
const fir::CharBoxValue &lhs, const fir::CharBoxValue &rhs) {
auto rhsCstLen = getCompileTimeLength(rhs);
auto lhsCstLen = getCompileTimeLength(lhs);
bool compileTimeSameLength =
(lhsCstLen && rhsCstLen && *lhsCstLen == *rhsCstLen) ||
(rhs.getLen() == lhs.getLen());

if (compileTimeSameLength && lhsCstLen && *lhsCstLen == 1) {
bool compileTimeSameLength = false;
bool isLengthOneAssign = false;

if (lhsCstLen && rhsCstLen && *lhsCstLen == *rhsCstLen) {
compileTimeSameLength = true;
if (*lhsCstLen == 1)
isLengthOneAssign = true;
} else if (rhs.getLen() == lhs.getLen()) {
compileTimeSameLength = true;

// If the length values are the same for LHS and RHS,
// then we can rely on the constant length deduced from
// any of the two types.
if (lhsCstLen && *lhsCstLen == 1)
isLengthOneAssign = true;
if (rhsCstLen && *rhsCstLen == 1)
isLengthOneAssign = true;

// We could have recognized constant operations here (e.g.
// two different arith.constant ops may produce the same value),
// but for now leave it to CSE to get rid of the duplicates.
}
if (isLengthOneAssign) {
createLengthOneAssign(lhs, rhs);
return;
}
Expand Down
44 changes: 44 additions & 0 deletions flang/test/HLFIR/char_assign.fir
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// RUN: fir-opt --split-input-file %s -convert-hlfir-to-fir | FileCheck %s

// Verify that the special case of length-one character assignment
// is using valid fir.converts.

// -----

// CHECK-LABEL: func.func @_QPtest1
// CHECK: [[C_DECL:%[0-9]*]] = fir.declare{{.*}}{uniq_name = "_QFtest1Ec"} : (!fir.ref<!fir.char<1,?>>, index) -> !fir.ref<!fir.char<1,?>>
// CHECK: [[LOCAL_DECL:%[0-9]*]] = fir.declare{{.*}}{uniq_name = "_QFtest1Elocal"} : (!fir.ref<!fir.char<1>>, index) -> !fir.ref<!fir.char<1>>
// CHECK: [[CVT:%[0-9]*]] = fir.convert [[C_DECL]] : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1>>
// CHECK: [[LD:%[0-9]*]] = fir.load [[CVT]] : !fir.ref<!fir.char<1>>
// CHECK: fir.store [[LD]] to [[LOCAL_DECL]] : !fir.ref<!fir.char<1>>
module {
func.func @_QPtest1(%arg0: !fir.boxchar<1> {fir.bindc_name = "c"}) {
%c1 = arith.constant 1 : index
%0:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
%1:2 = hlfir.declare %0#0 typeparams %c1 {uniq_name = "_QFtest1Ec"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
%2 = fir.alloca !fir.char<1> {bindc_name = "local", uniq_name = "_QFtest1Elocal"}
%3:2 = hlfir.declare %2 typeparams %c1 {uniq_name = "_QFtest1Elocal"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
hlfir.assign %1#0 to %3#0 : !fir.boxchar<1>, !fir.ref<!fir.char<1>>
return
}
}

// -----

// CHECK-LABEL: func.func @_QPtest2
// CHECK: [[C_DECL:%[0-9]*]] = fir.declare{{.*}}{uniq_name = "_QFtest2Ec"} : (!fir.ref<!fir.char<1,?>>, index) -> !fir.ref<!fir.char<1,?>>
// CHECK: [[LOCAL_DECL:%[0-9]*]] = fir.declare{{.*}}{uniq_name = "_QFtest2Elocal"} : (!fir.ref<!fir.char<1>>, index) -> !fir.ref<!fir.char<1>>
// CHECK: [[LD:%[0-9]*]] = fir.load [[LOCAL_DECL]] : !fir.ref<!fir.char<1>>
// CHECK: [[CVT:%[0-9]*]] = fir.convert [[C_DECL]] : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1>>
// CHECK: fir.store [[LD]] to [[CVT]] : !fir.ref<!fir.char<1>>
module {
func.func @_QPtest2(%arg0: !fir.boxchar<1> {fir.bindc_name = "c"}) {
%c1 = arith.constant 1 : index
%0:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
%1:2 = hlfir.declare %0#0 typeparams %c1 {uniq_name = "_QFtest2Ec"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
%2 = fir.alloca !fir.char<1> {bindc_name = "local", uniq_name = "_QFtest2Elocal"}
%3:2 = hlfir.declare %2 typeparams %c1 {uniq_name = "_QFtest2Elocal"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
hlfir.assign %3#0 to %1#0 : !fir.ref<!fir.char<1>>, !fir.boxchar<1>
return
}
}
10 changes: 6 additions & 4 deletions flang/test/Lower/HLFIR/calls-character-singleton-result.f90
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ subroutine scalar_char(c, i)
end subroutine
! CHECK-LABEL: func.func @_QPscalar_char(
! CHECK: %[[VAL_2:.*]] = fir.alloca !fir.char<1>
! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare %{{.*}}#0 typeparams %{{.*}} {{.*}}Ec
! CHECK: %[[VAL_4:.*]] = fir.convert %{{.*}}#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1>>
! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare %[[VAL_4]] typeparams %{{.*}} {{.*}}Ec
! CHECK: %[[VAL_6:.*]]:2 = hlfir.declare %{{.*}} {{.*}}Ei
! CHECK: %[[VAL_7:.*]] = fir.load %[[VAL_6]]#1 : !fir.ref<i64>
! CHECK: %[[VAL_8:.*]] = fir.convert %[[VAL_7]] : (i64) -> i8
Expand All @@ -20,7 +21,7 @@ subroutine scalar_char(c, i)
! CHECK: fir.store %[[VAL_10]] to %[[VAL_2]] : !fir.ref<!fir.char<1>>
! CHECK: %[[VAL_11:.*]] = arith.constant false
! CHECK: %[[VAL_12:.*]] = hlfir.as_expr %[[VAL_2]] move %[[VAL_11]] : (!fir.ref<!fir.char<1>>, i1) -> !hlfir.expr<!fir.char<1>>
! CHECK: hlfir.assign %[[VAL_12]] to %[[VAL_5]]#0 : !hlfir.expr<!fir.char<1>>, !fir.boxchar<1>
! CHECK: hlfir.assign %[[VAL_12]] to %[[VAL_5]]#0 : !hlfir.expr<!fir.char<1>>, !fir.ref<!fir.char<1>>

subroutine scalar_bindc(c)
character(1) :: c
Expand All @@ -32,12 +33,13 @@ character(1) function bar() bind(c)
end subroutine
! CHECK-LABEL: func.func @_QPscalar_bindc(
! CHECK: %[[VAL_1:.*]] = fir.alloca !fir.char<1>
! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %{{.*}}#0 typeparams %{{.*}} {{.*}}Ec
! CHECK: %[[VAL_3:.*]] = fir.convert %{{.*}}#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1>>
! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_3]] typeparams %{{.*}} {{.*}}Ec
! CHECK: %[[VAL_5:.*]] = fir.call @bar() fastmath<contract> : () -> !fir.char<1>
! CHECK: fir.store %[[VAL_5]] to %[[VAL_1]] : !fir.ref<!fir.char<1>>
! CHECK: %[[VAL_6:.*]] = arith.constant false
! CHECK: %[[VAL_7:.*]] = hlfir.as_expr %[[VAL_1]] move %[[VAL_6]] : (!fir.ref<!fir.char<1>>, i1) -> !hlfir.expr<!fir.char<1>>
! CHECK: hlfir.assign %[[VAL_7]] to %[[VAL_4]]#0 : !hlfir.expr<!fir.char<1>>, !fir.boxchar<1>
! CHECK: hlfir.assign %[[VAL_7]] to %[[VAL_4]]#0 : !hlfir.expr<!fir.char<1>>, !fir.ref<!fir.char<1>>

subroutine array_char(c, i)
character(1) :: c(100)
Expand Down
3 changes: 2 additions & 1 deletion flang/test/Lower/HLFIR/convert-variable.f90
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ subroutine scalar_character_cst_len(c)
! CHECK-SAME: %[[VAL_0:.*]]: !fir.boxchar<1>
! CHECK: %[[VAL_1:.*]]:2 = fir.unboxchar %[[VAL_0]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
! CHECK: %[[VAL_2:.*]] = arith.constant 10 : index
! CHECK: %[[VAL_3:.*]] = hlfir.declare %[[VAL_1]]#0 typeparams %[[VAL_2]] {uniq_name = "_QFscalar_character_cst_lenEc"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
! CHECK: %[[VAL_3:.*]] = fir.convert %[[VAL_1]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1,10>>
! CHECK: %[[VAL_4:.*]] = hlfir.declare %[[VAL_3]] typeparams %[[VAL_2]] {uniq_name = "_QFscalar_character_cst_lenEc"} : (!fir.ref<!fir.char<1,10>>, index) -> (!fir.ref<!fir.char<1,10>>, !fir.ref<!fir.char<1,10>>)

subroutine array_numeric(x)
integer :: x(10, 20)
Expand Down
15 changes: 9 additions & 6 deletions flang/test/Lower/HLFIR/implicit-call-mismatch.f90
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ subroutine pass_kind2_char_to_char(c)
call takes_char(c)
end subroutine
! CHECK-LABEL: func.func @_QPpass_kind2_char_to_char(
! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare {{.*}}Ec
! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare {{.*}}Ec
! CHECK: %[[VAL_3:.*]] = fir.emboxchar %[[VAL_2]]#1, %{{.*}} : (!fir.ref<!fir.char<2,4>>, index) -> !fir.boxchar<2>
! CHECK: %[[VAL_4:.*]] = fir.address_of(@_QPtakes_char) : (!fir.boxchar<1>) -> ()
! CHECK: %[[VAL_5:.*]] = fir.convert %[[VAL_4]] : ((!fir.boxchar<1>) -> ()) -> ((!fir.boxchar<2>) -> ())
! CHECK: fir.call %[[VAL_5]](%[[VAL_3]]#0) {{.*}}: (!fir.boxchar<2>) -> ()
! CHECK: fir.call %[[VAL_5]](%[[VAL_3]]) {{.*}}: (!fir.boxchar<2>) -> ()

subroutine takes_real(r)
real(8) :: r
Expand All @@ -60,10 +61,11 @@ subroutine pass_char_to_real(c)
call takes_real(c)
end subroutine
! CHECK-LABEL: func.func @_QPpass_char_to_real(
! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare {{.*}}Ec
! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare {{.*}}Ec
! CHECK: %[[VAL_3:.*]] = fir.emboxchar %[[VAL_2]]#1, %{{.*}} : (!fir.ref<!fir.char<1,8>>, index) -> !fir.boxchar<1>
! CHECK: %[[VAL_4:.*]] = fir.address_of(@_QPtakes_real) : (!fir.ref<f64>) -> ()
! CHECK: %[[VAL_5:.*]] = fir.convert %[[VAL_4]] : ((!fir.ref<f64>) -> ()) -> ((!fir.boxchar<1>) -> ())
! CHECK: fir.call %[[VAL_5]](%[[VAL_3]]#0) {{.*}}: (!fir.boxchar<1>) -> ()
! CHECK: fir.call %[[VAL_5]](%[[VAL_3]]) {{.*}}: (!fir.boxchar<1>) -> ()

subroutine pass_proc_to_real()
real(8), external :: proc
Expand Down Expand Up @@ -105,10 +107,11 @@ subroutine pass_char_to_char_proc(c)
call takes_char_proc(c)
end subroutine
! CHECK-LABEL: func.func @_QPpass_char_to_char_proc(
! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare {{.*}}Ec
! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare {{.*}}Ec
! CHECK: %[[VAL_3:.*]] = fir.emboxchar %[[VAL_2]]#1, %{{.*}} : (!fir.ref<!fir.char<1,8>>, index) -> !fir.boxchar<1>
! CHECK: %[[VAL_4:.*]] = fir.address_of(@_QPtakes_char_proc) : (tuple<!fir.boxproc<() -> ()>, i64>) -> ()
! CHECK: %[[VAL_5:.*]] = fir.convert %[[VAL_4]] : ((tuple<!fir.boxproc<() -> ()>, i64>) -> ()) -> ((!fir.boxchar<1>) -> ())
! CHECK: fir.call %[[VAL_5]](%[[VAL_3]]#0) {{.*}}: (!fir.boxchar<1>) -> ()
! CHECK: fir.call %[[VAL_5]](%[[VAL_3]]) {{.*}}: (!fir.boxchar<1>) -> ()

subroutine takes_proc(proc)
real(8), external :: proc
Expand Down
2 changes: 1 addition & 1 deletion flang/test/Lower/HLFIR/substrings.f90
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ subroutine cst_len(array, scalar)
! CHECK: %[[VAL_34:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_15]]:%[[VAL_5]]:%[[VAL_15]]) substr %[[VAL_16]], %[[VAL_3]] shape %[[VAL_6]] typeparams %[[VAL_33]] : (!fir.ref<!fir.array<100x!fir.char<1,10>>>, index, index, index, index, index, !fir.shape<1>, index) -> !fir.box<!fir.array<100x!fir.char<1,9>>>

print *, scalar(2:5)
! CHECK: %[[VAL_40:.*]] = hlfir.designate %[[VAL_9]]#0 substr %[[VAL_16]], %[[VAL_17]] typeparams %[[VAL_18]] : (!fir.boxchar<1>, index, index, index) -> !fir.ref<!fir.char<1,4>>
! CHECK: %[[VAL_40:.*]] = hlfir.designate %[[VAL_9]]#0 substr %[[VAL_16]], %[[VAL_17]] typeparams %[[VAL_18]] : (!fir.ref<!fir.char<1,10>>, index, index, index) -> !fir.ref<!fir.char<1,4>>
end subroutine

! CHECK-LABEL: func.func @_QPdyn_len(
Expand Down
31 changes: 16 additions & 15 deletions flang/test/Lower/Intrinsics/achar.f90
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
! RUN: bbc -emit-fir %s -o - | fir-opt --canonicalize | FileCheck %s
! RUN: %flang_fc1 -emit-fir %s -o - | fir-opt --canonicalize | FileCheck %s

! CHECK-LABEL: test1
! CHECK-SAME: (%[[XREF:.*]]: !fir.ref<i32> {{.*}}, %[[CBOX:.*]]: !fir.boxchar<1> {{.*}})
! CHECK-DAG: %[[C1:.*]] = arith.constant 1 : index
! CHECK-DAG: %[[FALSE:.*]] = arith.constant false
! CHECK: %[[TEMP:.*]] = fir.alloca !fir.char<1> {adapt.valuebyref}
! CHECK: %[[C:.*]]:2 = fir.unboxchar %[[CBOX]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
! CHECK: %[[X:.*]] = fir.load %[[XREF]] : !fir.ref<i32>
! CHECK: %[[X_I8:.*]] = fir.convert %[[X]] : (i32) -> i8
! CHECK: %[[UNDEF:.*]] = fir.undefined !fir.char<1>
! CHECK: %[[XCHAR:.*]] = fir.insert_value %[[UNDEF]], %[[X_I8]], [0 : index] : (!fir.char<1>, i8) -> !fir.char<1>
! CHECK: fir.store %[[XCHAR]] to %[[TEMP]] : !fir.ref<!fir.char<1>>
! CHECK: %[[C1_I64:.*]] = fir.convert %[[C1]] : (index) -> i64
! CHECK: %[[C_CVT:.*]] = fir.convert %[[C]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<i8>
! CHECK: %[[TEMP_WITH_XCHAR:.*]] = fir.convert %[[TEMP]] : (!fir.ref<!fir.char<1>>) -> !fir.ref<i8>
! CHECK: fir.call @llvm.memmove.p0.p0.i64(%[[C_CVT]], %[[TEMP_WITH_XCHAR]], %[[C1_I64]], %[[FALSE]]) {{.*}}: (!fir.ref<i8>, !fir.ref<i8>, i64, i1) -> ()

! CHECK-LABEL: func.func @_QPtest1(
! CHECK-SAME: %[[VAL_0:.*]]: !fir.ref<i32> {fir.bindc_name = "x"},
! CHECK-SAME: %[[VAL_1:.*]]: !fir.boxchar<1> {fir.bindc_name = "c"}) {
! CHECK: %[[VAL_2:.*]] = fir.alloca !fir.char<1> {adapt.valuebyref}
! CHECK: %[[VAL_3:.*]]:2 = fir.unboxchar %[[VAL_1]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
! CHECK: %[[VAL_4:.*]] = fir.convert %[[VAL_3]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1>>
! CHECK: %[[VAL_5:.*]] = fir.load %[[VAL_0]] : !fir.ref<i32>
! CHECK: %[[VAL_6:.*]] = fir.convert %[[VAL_5]] : (i32) -> i8
! CHECK: %[[VAL_7:.*]] = fir.undefined !fir.char<1>
! CHECK: %[[VAL_8:.*]] = fir.insert_value %[[VAL_7]], %[[VAL_6]], [0 : index] : (!fir.char<1>, i8) -> !fir.char<1>
! CHECK: fir.store %[[VAL_8]] to %[[VAL_2]] : !fir.ref<!fir.char<1>>
! CHECK: %[[VAL_9:.*]] = fir.load %[[VAL_2]] : !fir.ref<!fir.char<1>>
! CHECK: fir.store %[[VAL_9]] to %[[VAL_4]] : !fir.ref<!fir.char<1>>
! CHECK: return
! CHECK: }
subroutine test1(x, c)
integer :: x
character :: c
Expand Down
Loading

0 comments on commit 8df5913

Please sign in to comment.