Skip to content

[flang] Fix character function call bug #67973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 2, 2023
Merged

[flang] Fix character function call bug #67973

merged 1 commit into from
Oct 2, 2023

Conversation

jeanPerier
Copy link
Contributor

Fixes #67658

The bug was that when instantiating a character array result variable, the code inserted a cast from the result buffer to the proper array type if it could see an fir.unboxchar op. But this is wrong for results and on caller side because the fir.emboxchar is visible so, charHelper.genUnboxChar() just takes the operand from that instead of generating an unboxchar.

The fix is simply to move the cast at the place where fir.boxchar<> argument are dealt with. The cast when creating fir.emboxchar is also removed: it adds noise and causes constant length result type to be lowered to fir.char<?>.

The main change from this patch is to deal with the lit test fallout of this cast move and removal.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels Oct 2, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 2, 2023 •
edited
Loading

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Changes

Fixes #67658

The bug was that when instantiating a character array result variable, the code inserted a cast from the result buffer to the proper array type if it could see an fir.unboxchar op. But this is wrong for results and on caller side because the fir.emboxchar is visible so, charHelper.genUnboxChar() just takes the operand from that instead of generating an unboxchar.

The fix is simply to move the cast at the place where fir.boxchar<> argument are dealt with. The cast when creating fir.emboxchar is also removed: it adds noise and causes constant length result type to be lowered to fir.char<?>.

The main change from this patch is to deal with the lit test fallout of this cast move and removal.

Patch is 83.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/67973.diff

30 Files Affected:

  • (modified) flang/lib/Lower/ConvertVariable.cpp (+6-8)
  • (modified) flang/lib/Optimizer/Builder/Character.cpp (+3-1)
  • (modified) flang/test/Lower/HLFIR/bindc-entry-stmt.f90 (+8-10)
  • (modified) flang/test/Lower/HLFIR/calls-assumed-shape.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/char_extremum.f03 (+4-4)
  • (modified) flang/test/Lower/HLFIR/convert-variable.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/function-return.f90 (+2-3)
  • (modified) flang/test/Lower/HLFIR/structure-constructor.f90 (+2-2)
  • (modified) flang/test/Lower/Intrinsics/merge.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/FIR/copyin.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/FIR/private-commonblock.f90 (+3-6)
  • (modified) flang/test/Lower/allocatable-assignment.f90 (+1-1)
  • (modified) flang/test/Lower/allocatable-callee.f90 (+5-9)
  • (modified) flang/test/Lower/allocatables.f90 (+3-6)
  • (modified) flang/test/Lower/array-elemental-calls-char-byval.f90 (+3-6)
  • (modified) flang/test/Lower/array-elemental-calls-char.f90 (+4-8)
  • (modified) flang/test/Lower/array-expression.f90 (+8-8)
  • (modified) flang/test/Lower/call-by-value-attr.f90 (+1-2)
  • (modified) flang/test/Lower/call-by-value.f90 (+2-3)
  • (modified) flang/test/Lower/call-implicit.f90 (+2-3)
  • (modified) flang/test/Lower/call-parenthesized-arg.f90 (+4-6)
  • (modified) flang/test/Lower/derived-allocatable-components.f90 (+6-12)
  • (modified) flang/test/Lower/derived-pointer-components.f90 (+6-12)
  • (modified) flang/test/Lower/derived-types.f90 (+3-4)
  • (modified) flang/test/Lower/dummy-argument-optional-2.f90 (+2-4)
  • (modified) flang/test/Lower/dummy-argument-optional.f90 (+1-2)
  • (modified) flang/test/Lower/entry-statement.f90 (+23-63)
  • (modified) flang/test/Lower/host-associated.f90 (+9-12)
  • (modified) flang/test/Lower/implicit-call-mismatch.f90 (+1-1)
  • (modified) flang/test/Lower/pointer-args-caller.f90 (+1-1)
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index 74e07ec77c0cb55..e3c2ce80996235a 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -1927,8 +1927,13 @@ void Fortran::lower::mapSymbolAttributes(
   if (ba.isChar()) {
     if (arg) {
       assert(!preAlloc && "dummy cannot be pre-allocated");
-      if (arg.getType().isa<fir::BoxCharType>())
+      if (arg.getType().isa<fir::BoxCharType>()) {
         std::tie(addr, len) = charHelp.createUnboxChar(arg);
+        // 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);
+      }
     }
     if (std::optional<int64_t> cstLen = ba.getCharLenConst()) {
       // Static length
@@ -1954,13 +1959,6 @@ 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 (ba.isStaticArray()) {
diff --git a/flang/lib/Optimizer/Builder/Character.cpp b/flang/lib/Optimizer/Builder/Character.cpp
index b164282df2dc5cc..41cdd9a71c735d5 100644
--- a/flang/lib/Optimizer/Builder/Character.cpp
+++ b/flang/lib/Optimizer/Builder/Character.cpp
@@ -208,7 +208,9 @@ fir::factory::CharacterExprHelper::createEmbox(const fir::CharBoxValue &box) {
     builder.create<fir::StoreOp>(loc, buff, temp);
     buff = temp;
   }
-  buff = builder.createConvert(loc, refType, buff);
+  // fir.emboxchar only accepts scalar, cast array buffer to a scalar buffer.
+  if (mlir::isa<fir::SequenceType>(fir::dyn_cast_ptrEleTy(buff.getType())))
+    buff = builder.createConvert(loc, refType, buff);
   // Convert in case the provided length is not of the integer type that must
   // be used in boxchar.
   auto len = builder.createConvert(loc, builder.getCharacterLengthType(),
diff --git a/flang/test/Lower/HLFIR/bindc-entry-stmt.f90 b/flang/test/Lower/HLFIR/bindc-entry-stmt.f90
index 89e494689e99642..3be14ed95e1f038 100644
--- a/flang/test/Lower/HLFIR/bindc-entry-stmt.f90
+++ b/flang/test/Lower/HLFIR/bindc-entry-stmt.f90
@@ -24,15 +24,14 @@ function foo() bind(c)
 ! CHECK-LABEL:   func.func @_QPbar(
 ! CHECK-SAME:                      %[[VAL_0:.*]]: !fir.ref<!fir.char<1>>,
 ! CHECK-SAME:                      %[[VAL_1:.*]]: index) -> !fir.boxchar<1> {
-! CHECK:           %[[VAL_2:.*]] = fir.convert %[[VAL_0]] : (!fir.ref<!fir.char<1>>) -> !fir.ref<!fir.char<1,?>>
 ! CHECK:           %[[VAL_3:.*]] = arith.constant 1 : index
-! CHECK:           %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_3]] {uniq_name = "_QFfooEfoo"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK:           %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_0]] typeparams %[[VAL_3]] {uniq_name = "_QFfooEfoo"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
 ! CHECK:           %[[VAL_5:.*]] = arith.constant 1 : index
-! CHECK:           %[[VAL_6:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_5]] {uniq_name = "_QFfooEbar"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK:           %[[VAL_6:.*]]:2 = hlfir.declare %[[VAL_0]] typeparams %[[VAL_5]] {uniq_name = "_QFfooEbar"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
 ! CHECK:           cf.br ^bb1
 ! CHECK:         ^bb1:
-! CHECK:           hlfir.assign %{{.*}} to %[[VAL_6]]#0 : !fir.ref<!fir.char<1>>, !fir.boxchar<1>
-! CHECK:           %[[VAL_10:.*]] = fir.emboxchar %[[VAL_6]]#1, %[[VAL_5]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.boxchar<1>
+! CHECK:           hlfir.assign %{{.*}} to %[[VAL_6]]#0 : !fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>
+! CHECK:           %[[VAL_10:.*]] = fir.emboxchar %[[VAL_6]]#1, %[[VAL_5]] : (!fir.ref<!fir.char<1>>, index) -> !fir.boxchar<1>
 ! CHECK:           return %[[VAL_10]] : !fir.boxchar<1>
 ! CHECK:         }
 
@@ -44,15 +43,14 @@ function foo2()
 ! CHECK-LABEL:   func.func @_QPfoo2(
 ! CHECK-SAME:                       %[[VAL_0:.*]]: !fir.ref<!fir.char<1>>,
 ! CHECK-SAME:                       %[[VAL_1:.*]]: index) -> !fir.boxchar<1> {
-! CHECK:           %[[VAL_2:.*]] = fir.convert %[[VAL_0]] : (!fir.ref<!fir.char<1>>) -> !fir.ref<!fir.char<1,?>>
 ! CHECK:           %[[VAL_3:.*]] = arith.constant 1 : index
-! CHECK:           %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_3]] {uniq_name = "_QFfoo2Efoo2"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK:           %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_0]] typeparams %[[VAL_3]] {uniq_name = "_QFfoo2Efoo2"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
 ! CHECK:           %[[VAL_5:.*]] = arith.constant 1 : index
-! CHECK:           %[[VAL_6:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_5]] {uniq_name = "_QFfoo2Ebar2"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK:           %[[VAL_6:.*]]:2 = hlfir.declare %[[VAL_0]] typeparams %[[VAL_5]] {uniq_name = "_QFfoo2Ebar2"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
 ! CHECK:           cf.br ^bb1
 ! CHECK:         ^bb1:
-! CHECK:           hlfir.assign %{{.*}} to %[[VAL_6]]#0 : !fir.ref<!fir.char<1>>, !fir.boxchar<1>
-! CHECK:           %[[VAL_10:.*]] = fir.emboxchar %[[VAL_4]]#1, %[[VAL_3]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.boxchar<1>
+! CHECK:           hlfir.assign %{{.*}} to %[[VAL_6]]#0 : !fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>
+! CHECK:           %[[VAL_10:.*]] = fir.emboxchar %[[VAL_4]]#1, %[[VAL_3]] : (!fir.ref<!fir.char<1>>, index) -> !fir.boxchar<1>
 ! CHECK:           return %[[VAL_10]] : !fir.boxchar<1>
 ! CHECK:         }
 
diff --git a/flang/test/Lower/HLFIR/calls-assumed-shape.f90 b/flang/test/Lower/HLFIR/calls-assumed-shape.f90
index 447da252d77bdef..58c41d46167d7d1 100644
--- a/flang/test/Lower/HLFIR/calls-assumed-shape.f90
+++ b/flang/test/Lower/HLFIR/calls-assumed-shape.f90
@@ -90,8 +90,8 @@ subroutine takes_assumed_character(x)
 end subroutine
 ! CHECK-LABEL: func.func @_QPtest_explicit_char_to_box(
 ! 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:.*]] = fir.convert %[[VAL_1]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.array<20x!fir.char<1,10>>>
+! CHECK:  %[[VAL_2:.*]] = arith.constant 10 : index
 ! CHECK:  %[[VAL_4:.*]] = arith.constant 20 : index
 ! CHECK:  %[[VAL_5:.*]] = fir.shape %[[VAL_4]] : (index) -> !fir.shape<1>
 ! CHECK:  %[[VAL_6:.*]]:2 = hlfir.declare %[[VAL_3]](%[[VAL_5:[a-z0-9]*]]) typeparams %[[VAL_2:[a-z0-9]*]] {uniq_name = "_QFtest_explicit_char_to_boxEe"} : (!fir.ref<!fir.array<20x!fir.char<1,10>>>, !fir.shape<1>, index) -> (!fir.ref<!fir.array<20x!fir.char<1,10>>>, !fir.ref<!fir.array<20x!fir.char<1,10>>>)
diff --git a/flang/test/Lower/HLFIR/char_extremum.f03 b/flang/test/Lower/HLFIR/char_extremum.f03
index 32a519ce3b681e5..cc7b80184935eab 100644
--- a/flang/test/Lower/HLFIR/char_extremum.f03
+++ b/flang/test/Lower/HLFIR/char_extremum.f03
@@ -45,14 +45,14 @@ subroutine max2(c1, c2, c3)
 ! CHECK:  %[[VAL_2:[a-zA-Z0-9_]*]]  = fir.shape %[[VAL_C100]] : (index) -> !fir.shape<1>
 ! CHECK:  %[[VAL_3:[a-zA-Z0-9_]*]]:2 = hlfir.declare %[[VAL_1]](%[[VAL_2]]) typeparams %[[VAL_0]]#1 {{.*}} : (!fir.ref<!fir.array<100x!fir.char<1,?>>>, !fir.shape<1>, index) -> (!fir.box<!fir.array<100x!fir.char<1,?>>>, !fir.ref<!fir.array<100x!fir.char<1,?>>>)
 ! CHECK:  %[[VAL_4:[a-zA-Z0-9_]*]]:2 = fir.unboxchar %{{.*}} : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
-! CHECK:  %[[VAL_C10:[a-zA-Z0-9_]*]] = arith.constant 10 : index
 ! CHECK:  %[[VAL_5:[a-zA-Z0-9_]*]] = fir.convert %[[VAL_4]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.array<100x!fir.char<1,10>>>
+! CHECK:  %[[VAL_C10:[a-zA-Z0-9_]*]] = arith.constant 10 : index
 ! CHECK:  %[[VAL_C100_0:[a-zA-Z0-9_]*]] = arith.constant 100 : index
 ! CHECK:  %[[VAL_6:[a-zA-Z0-9_]*]] = fir.shape %[[VAL_C100_0]] : (index) -> !fir.shape<1>
 ! CHECK:  %[[VAL_7:[a-zA-Z0-9_]*]]:2 = hlfir.declare %[[VAL_5]](%[[VAL_6]]) typeparams %[[VAL_C10]] {{.*}} : (!fir.ref<!fir.array<100x!fir.char<1,10>>>, !fir.shape<1>, index) -> (!fir.ref<!fir.array<100x!fir.char<1,10>>>, !fir.ref<!fir.array<100x!fir.char<1,10>>>)
 ! CHECK:  %[[VAL_8:[a-zA-Z0-9_]*]]:2 = fir.unboxchar %{{.*}} : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
-! CHECK:  %[[VAL_C20:[a-zA-Z0-9_]*]] = arith.constant 20 : index
 ! CHECK:  %[[VAL_9:[a-zA-Z0-9_]*]] = fir.convert %[[VAL_8]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.array<100x!fir.char<1,20>>>
+! CHECK:  %[[VAL_C20:[a-zA-Z0-9_]*]] = arith.constant 20 : index
 ! CHECK:  %[[VAL_C100_1:[a-zA-Z0-9_]*]] = arith.constant 100 : index
 ! CHECK:  %[[VAL_10:[a-zA-Z0-9_]*]] = fir.shape %[[VAL_C100_1]] : (index) -> !fir.shape<1>
 ! CHECK:  %[[VAL_11:[a-zA-Z0-9_]*]]:2 = hlfir.declare %[[VAL_9]](%[[VAL_10]]) typeparams %[[VAL_C20]] {{.*}} : (!fir.ref<!fir.array<100x!fir.char<1,20>>>, !fir.shape<1>, index) -> (!fir.ref<!fir.array<100x!fir.char<1,20>>>, !fir.ref<!fir.array<100x!fir.char<1,20>>>)
@@ -78,14 +78,14 @@ subroutine min2(c1, c2, c3)
 ! CHECK:  %[[VAL_2:[a-zA-Z0-9_]*]] = fir.shape %[[VAL_C100]] : (index) -> !fir.shape<1>
 ! CHECK:  %[[VAL_3:[a-zA-Z0-9_]*]]:2 = hlfir.declare %[[VAL_1]](%[[VAL_2]]) typeparams %[[VAL_0]]#1 {{.*}} : (!fir.ref<!fir.array<100x!fir.char<1,?>>>, !fir.shape<1>, index) -> (!fir.box<!fir.array<100x!fir.char<1,?>>>, !fir.ref<!fir.array<100x!fir.char<1,?>>>)
 ! CHECK:  %[[VAL_4:[a-zA-Z0-9_]*]]:2 = fir.unboxchar %arg1 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
-! CHECK:  %[[VAL_C10:[a-zA-Z0-9_]*]] = arith.constant 10 : index
 ! CHECK:  %[[VAL_5:[a-zA-Z0-9_]*]] = fir.convert %[[VAL_4]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.array<100x!fir.char<1,10>>>
+! CHECK:  %[[VAL_C10:[a-zA-Z0-9_]*]] = arith.constant 10 : index
 ! CHECK:  %[[VAL_C100_0:[a-zA-Z0-9_]*]] = arith.constant 100 : index
 ! CHECK:  %[[VAL_6:[a-zA-Z0-9_]*]] = fir.shape %[[VAL_C100_0]] : (index) -> !fir.shape<1>
 ! CHECK:  %[[VAL_7:[a-zA-Z0-9_]*]]:2 = hlfir.declare %[[VAL_5]](%[[VAL_6]]) typeparams %[[VAL_C10]] {{.*}} : (!fir.ref<!fir.array<100x!fir.char<1,10>>>, !fir.shape<1>, index) -> (!fir.ref<!fir.array<100x!fir.char<1,10>>>, !fir.ref<!fir.array<100x!fir.char<1,10>>>)
 ! CHECK:  %[[VAL_8:[a-zA-Z0-9_]*]]:2 = fir.unboxchar %arg2 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
-! CHECK:  %[[VAL_C20:[a-zA-Z0-9_]*]] = arith.constant 20 : index
 ! CHECK:  %[[VAL_C9:[a-zA-Z0-9_]*]] = fir.convert %[[VAL_8]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.array<100x!fir.char<1,20>>>
+! CHECK:  %[[VAL_C20:[a-zA-Z0-9_]*]] = arith.constant 20 : index
 ! CHECK:  %[[VAL_C100_1:[a-zA-Z0-9_]*]] = arith.constant 100 : index
 ! CHECK:  %[[VAL_10:[a-zA-Z0-9_]*]] = fir.shape %[[VAL_C100_1]] : (index) -> !fir.shape<1>
 ! CHECK:  %[[VAL_11:[a-zA-Z0-9_]*]]:2 = hlfir.declare %[[VAL_C9]](%[[VAL_10]]) typeparams %[[VAL_C20]] {{.*}} : (!fir.ref<!fir.array<100x!fir.char<1,20>>>, !fir.shape<1>, index) -> (!fir.ref<!fir.array<100x!fir.char<1,20>>>, !fir.ref<!fir.array<100x!fir.char<1,20>>>)
diff --git a/flang/test/Lower/HLFIR/convert-variable.f90 b/flang/test/Lower/HLFIR/convert-variable.f90
index 746ac085ad115f4..e7487ef870d1dd7 100644
--- a/flang/test/Lower/HLFIR/convert-variable.f90
+++ b/flang/test/Lower/HLFIR/convert-variable.f90
@@ -22,8 +22,8 @@ subroutine scalar_character_cst_len(c)
 ! CHECK-LABEL: func.func @_QPscalar_character_cst_len(
 ! 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:.*]] = fir.convert %[[VAL_1]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1,10>>
+! CHECK:  %[[VAL_2:.*]] = arith.constant 10 : index
 ! 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)
diff --git a/flang/test/Lower/HLFIR/function-return.f90 b/flang/test/Lower/HLFIR/function-return.f90
index 42f66c3d03e14f4..8127133697c9812 100644
--- a/flang/test/Lower/HLFIR/function-return.f90
+++ b/flang/test/Lower/HLFIR/function-return.f90
@@ -17,10 +17,9 @@ integer function simple_return()
 end function
 ! CHECK-LABEL: func.func @_QPchar_return(
 ! CHECK-SAME:  %[[VAL_0:.*]]: !fir.ref<!fir.char<1,10>>
-! CHECK:  %[[VAL_2:.*]] = fir.convert %[[VAL_0]] : (!fir.ref<!fir.char<1,10>>) -> !fir.ref<!fir.char<1,?>>
 ! CHECK:  %[[VAL_3:.*]] = arith.constant 10 : index
-! CHECK:  %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_3]] {uniq_name = "_QFchar_returnEchar_return"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
-! CHECK:  %[[VAL_8:.*]] = fir.emboxchar %[[VAL_4]]#1, %[[VAL_3]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.boxchar<1>
+! CHECK:  %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_0]] typeparams %[[VAL_3]] {uniq_name = "_QFchar_returnEchar_return"} : (!fir.ref<!fir.char<1,10>>, index) -> (!fir.ref<!fir.char<1,10>>, !fir.ref<!fir.char<1,10>>)
+! CHECK:  %[[VAL_8:.*]] = fir.emboxchar %[[VAL_4]]#1, %[[VAL_3]] : (!fir.ref<!fir.char<1,10>>, index) -> !fir.boxchar<1>
 ! CHECK:  return %[[VAL_8]] : !fir.boxchar<1>
 
 integer function array_return()
diff --git a/flang/test/Lower/HLFIR/structure-constructor.f90 b/flang/test/Lower/HLFIR/structure-constructor.f90
index 90d5922d67a6752..797f801ad642408 100644
--- a/flang/test/Lower/HLFIR/structure-constructor.f90
+++ b/flang/test/Lower/HLFIR/structure-constructor.f90
@@ -41,8 +41,8 @@ end subroutine test1
 ! CHECK:           %[[VAL_2:.*]] = fir.alloca !fir.type<_QMtypesTt1{c:!fir.char<1,4>}> {bindc_name = "res", uniq_name = "_QFtest1Eres"}
 ! CHECK:           %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_2]] {uniq_name = "_QFtest1Eres"} : (!fir.ref<!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>) -> (!fir.ref<!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>, !fir.ref<!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>)
 ! CHECK:           %[[VAL_4:.*]]:2 = fir.unboxchar %[[VAL_0]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
-! CHECK:           %[[VAL_5:.*]] = arith.constant 4 : index
 ! CHECK:           %[[VAL_6:.*]] = fir.convert %[[VAL_4]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1,4>>
+! CHECK:           %[[VAL_5:.*]] = arith.constant 4 : index
 ! CHECK:           %[[VAL_7:.*]]:2 = hlfir.declare %[[VAL_6]] typeparams %[[VAL_5]] {uniq_name = "_QFtest1Ex"} : (!fir.ref<!fir.char<1,4>>, index) -> (!fir.ref<!fir.char<1,4>>, !fir.ref<!fir.char<1,4>>)
 ! CHECK:           %[[VAL_8:.*]]:2 = hlfir.declare %[[VAL_1]] {uniq_name = "ctor.temp"} : (!fir.ref<!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>) -> (!fir.ref<!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>, !fir.ref<!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>)
 ! CHECK:           %[[VAL_9:.*]] = fir.embox %[[VAL_8]]#0 : (!fir.ref<!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>) -> !fir.box<!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>
@@ -220,8 +220,8 @@ end subroutine test6
 ! CHECK:           %[[VAL_5:.*]] = fir.alloca !fir.type<_QMtypesTt5{t5m:!fir.box<!fir.heap<!fir.array<?x!fir.type<_QMtypesTt4{c:!fir.box<!fir.heap<!fir.array<?x!fir.char<1,2>>>>}>>>>}>
 ! CHECK:           %[[VAL_6:.*]] = fir.alloca !fir.type<_QMtypesTt6{t5m:!fir.box<!fir.heap<!fir.array<?x!fir.type<_QMtypesTt4{c:!fir.box<!fir.heap<!fir.array<?x!fir.char<1,2>>>>}>>>>,t6m:!fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>}>
 ! CHECK:           %[[VAL_7:.*]]:2 = fir.unboxchar %[[VAL_1]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
-! CHECK:           %[[VAL_8:.*]] = arith.constant 4 : index
 ! CHECK:           %[[VAL_9:.*]] = fir.convert %[[VAL_7]]#0 : (!fir.ref<!fir.char<1,?>>) -> !fir.ref<!fir.char<1,4>>
+! CHECK:           %[[VAL_8:.*]] = arith.constant 4 : index
 ! CHECK:           %[[VAL_10:.*]]:2 = hlfir.declare %[[VAL_9]] typeparams %[[VAL_8]] {uniq_name = "_QFtest6Ec"} : (!fir.ref<!fir.char<1,4>>, index) -> (!fir.ref<!fir.char<1,4>>, !fir.ref<!fir.char<1,4>>)
 ! CHECK:           %[[VAL_11:.*]] = fir.alloca !fir.type<_QMtypesTt6{t5m:!fir.box<!fir.heap<!fir.array<?x!fir.type<_QMtypesTt4{c:!fir.box<!fir.heap<!fir.array<?x!fir.char<1,2>>>>}>>>>,t6m:!fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>}> {bindc_name = "res", uniq_name = "_QFtest6Eres"}
 ! CHECK:           %[[VAL_12:.*]]:2 = hlfir.declare %[[VAL_11]] {uniq_name = "_QFtest6Eres"} : (!fir.ref<!fir.type<_QMtypesTt6{t5m:!fir.box<!fir.heap<!fir.array<?x!fir.type<_QMtypesTt4{c:!fir.box<!fir.heap<!fir.array<?x!fir.char<1,2>>>>}>>>>,t6m:!fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>}>>) -> (!fir.ref<!fir.type<_QMtypesTt6{t5m:!fir.box<!fir.heap<!fir.array<?x!fir.type<_QMtypesTt4{c:!fir.box<!fir.heap<!fir.array<?x!fir.char<1,2>>>>}>>>>,t6m:!fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>}>>, !fir.ref<!fir.type<_QMtypesTt6{t5m:!fir.box<!fir.heap<!fir.array<?x!fir.type<_QMtypesTt4{c:!fir.box<!fir.heap<!fir.array<?x!fir.char<1,2>>>>}>>>>,t6m:!fir.array<1x!fir.type<_QMtypesTt1{c:!fir.char<1,4>}>>}>>)
diff --git a/flang/test/Lower/Intrinsics/merge.f90 b/flang/test/Lower/Intrinsics/merge.f90
index 0ceb670f20fbd90..c8b6b3294b3280f 100644
--- a/flang/test/Lower/Intrinsics/merge.f90
+++ b/flang/test/Lower/Intrinsics/merge.f90
@@ -14,7 +14,7 @@ function merge_test(o1, o2, mask)
 ! CHECK: %[[a2:.*]] = fir.load %[[arg4]] : !fir.ref<!fir.logical<4>>
 ! CHECK: %[[a3:.*]] = fir.convert %[[a2]] : (!fir.logical<4>) -> i1
 ! CHECK: %[[a4:.*]] = arith.select %[[a3]], %[[a0_cast]], %[[a1_cast]] : !fir.ref<!fir.char<1>>
-! CHECK:  %{{.*}} = fir.convert %[[a4]] : (!fir.ref<!fir.char<1>>) -> !fir.ref<i8>
+! CHECK:  %{{.*}} = fir.load %[[a4]] : !fir.ref<!fir.char<1>>
 end
 
 ! CHECK-LABEL: func @_QPmerge_test2(
diff --git a/flang/test/Lower/OpenMP/FIR/copyin.f90 b/flang/test/Lower/OpenMP/FIR/copyin.f90
index ddfa0ea0914628f..2b3f382240958c4 100644
--- a/flang/test/Lower/OpenMP/FIR/copyin.f90
+++ b/flang/test/Lower/OpenMP/FIR/copyin.f90
@@ -85,8 +85,7 @@ subroutine copyin_scalar_array()
 ! CHECK:           }
 ! CHECK:           fir.array_merge_store %[[VAL_16]], %[[VAL_35:.*]] to %[[VAL_14]] : !fir.array<10x!fir.char<1,5>>, !fir.array<10x!fir.char<1,5>>, !fir.ref<!fir.array<10x!fir.char<1,5>>>
 ! CHECK:           omp.barrier
-! CHECK:           %[[VAL_36:.*]] = fir.convert %[[VAL_7]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<!fir.char<1,?>>
-! CHECK:           %[[VAL_37:.*]] = fir.emboxchar %[[VAL_36]], %[[VAL_1]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.boxchar<1>
+! CHECK:           %[[VAL_37:.*]] = fir.emboxchar %[[VAL_7]], %[[VAL_1]] : (!fir.ref<!fir.char<1,5>>, index) -> !fir.boxchar<1>
 ! CHECK:           %[[VAL_38:.*]] = fir.convert %[[VAL_14]] : (!fir.ref<!fir.array<10x!fir.char<1,5>>>) -> !fir.ref<!fir.char<1,?>>
 ! CHECK:           %[[VAL_39:.*]] = fir.emboxchar %[[VAL_38]], %[[VAL_4]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.boxchar<1>
 ! CHECK:           fir.call @_QPsub2(%[[VAL_37]], %[[VAL_39]]) {{.*}}: (!fir.boxchar<1>, !fir.boxchar<1>) -> ()
diff --git a/flang/test/Lower/OpenMP/FIR/private-commonblock.f90 b/flang/test/Lower/OpenMP/FIR/private-commonblock.f90
index 767458e3effbf07..6e98b917cf87fc1 100644
--- a/flang/test/Lower/OpenMP/FIR/private-commonblock.f90
+++ b/flang/test/Lower/OpenMP/FIR/private-commonblock.f90
@@ -34,8 +34,7 @@ subroutine private_common
 !CHECK: %[[val_11:.*]] = fir.coordinate_of %[[val_10]], %[[val_c49]] : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
 !CHECK: %[[val_12:.*]] = fir.convert %[[val_11]] : (!fir.ref<i8>) -> !fir.ref<!fir.array<5x!fir.char<1,5>>>
 !CHECK: %[[val_c5_0:.*]] = arith.constant 5 : index
-!CHECK: %[[val_13:.*]] = fir.convert %[[val_9]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<!fir.char<1,?>>
-!CHECK: %[[val_14:.*]] = fir.emboxchar %[[val_13]], %[[val_c5]] : (!fir.ref<!fir.char<1,?>>, index) ...
[truncated]

psteinfeld
psteinfeld approved these changes Oct 2, 2023
View reviewed changes
Copy link
Contributor

@psteinfeld psteinfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All builds and tests correctly and looks good.

Note that there's a failure in one of the automated tests in the ARM build-bot, but it doesn't look like it has anything to do with these changes.

@jeanPerier
[flang] Fix character function call bug
adcc4a1 
Fixes llvm#67658

The bug was that when instantiating a character array result variable,
the code inserted a cast from the result buffer to the proper array type
if it could see an fir.unboxchar op. But this is wrong for results and
on caller side because the fir.emboxchar is visible so,
charHelper.genUnboxChar() just takes the operand from that instead of
generating an unboxchar.

The fix is simply to move the cast at the place where fir.boxchar<>
argument are dealt with. The cast when creating fir.emboxchar is
also removed: it adds noise and causes constant length result type
to be lowered to fir.char<?>.

The main change from this patch is to deal with the lit test fallout of
this cast move and removal.
vzakhari
vzakhari approved these changes Oct 2, 2023
View reviewed changes
Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! LGTM

@jeanPerier jeanPerier merged commit c0cb8f7 into llvm:main Oct 2, 2023
@jeanPerier jeanPerier deleted the jp-fix-67658 branch October 2, 2023 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psteinfeld psteinfeld psteinfeld approved these changes

@vzakhari vzakhari vzakhari approved these changes

@yus3710-fj yus3710-fj Awaiting requested review from yus3710-fj

Assignees
No one assigned
Labels
flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Flang] Compilation error of UBOUND as the extents of a character array
4 participants
@jeanPerier @llvmbot @psteinfeld @vzakhari