[flang-commits] [flang] [flang][hlfir] address char_convert issues as mentioned in #64315 (PR #67570)

Anthony Cabrera via flang-commits flang-commits at lists.llvm.org
Mon Oct 2 12:47:42 PDT 2023


https://github.com/cabreraam updated https://github.com/llvm/llvm-project/pull/67570

>From 8e8e859a19ce1233a3e78a89914fa3d9af6c0b70 Mon Sep 17 00:00:00 2001
From: cabreraam <cabreraam33 at gmail.com>
Date: Tue, 26 Sep 2023 18:02:00 -0400
Subject: [PATCH 1/3] [flang][hlfir] address char_convert issues as mentioned
 in #64315

---
 flang/lib/Lower/ConvertExprToHLFIR.cpp | 28 ++++++++++--
 flang/test/Lower/HLFIR/charconvert.f90 | 60 ++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 3 deletions(-)
 create mode 100644 flang/test/Lower/HLFIR/charconvert.f90

diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index bc98fdd917d41d0..7fed9dc5c08868e 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -1403,6 +1403,22 @@ struct UnaryOp<Fortran::evaluate::Parentheses<T>> {
   }
 };
 
+static fir::CharBoxValue genUnboxChar(mlir::Location loc,
+                                      fir::FirOpBuilder &builder,
+                                      mlir::Value boxChar) {
+  if (auto emboxChar = boxChar.getDefiningOp<fir::EmboxCharOp>())
+    return {emboxChar.getMemref(), emboxChar.getLen()};
+  mlir::Type refType = fir::ReferenceType::get(
+      boxChar.getType().cast<fir::BoxCharType>().getEleTy());
+  auto unboxed = builder.create<fir::UnboxCharOp>(
+      loc, refType, builder.getIndexType(), boxChar);
+  mlir::Value addr = unboxed.getResult(0);
+  mlir::Value len = unboxed.getResult(1);
+  if (auto varIface = boxChar.getDefiningOp<fir::FortranVariableOpInterface>())
+    if (mlir::Value explicitlen = varIface.getExplicitCharLen())
+      len = explicitlen;
+  return {addr, len};
+}
 template <Fortran::common::TypeCategory TC1, int KIND,
           Fortran::common::TypeCategory TC2>
 struct UnaryOp<
@@ -1414,7 +1430,6 @@ struct UnaryOp<
                                          hlfir::Entity lhs) {
     if constexpr (TC1 == Fortran::common::TypeCategory::Character &&
                   TC2 == TC1) {
-      // TODO(loc, "character conversion in HLFIR");
       auto kindMap = builder.getKindMap();
       mlir::Type fromTy = lhs.getFortranElementType();
       mlir::Value origBufferSize = genCharLength(loc, builder, lhs);
@@ -1435,8 +1450,15 @@ struct UnaryOp<
       // allocate space on the stack for toBuffer
       auto dest = builder.create<fir::AllocaOp>(loc, toTy,
                                                 mlir::ValueRange{bufferSize});
-      builder.create<fir::CharConvertOp>(loc, lhs.getFirBase(), origBufferSize,
-                                         dest);
+      auto src = lhs.getFirBase();
+      if (lhs.getType().isa<hlfir::ExprType>()) {
+        if (auto asExpr = lhs.getDefiningOp<hlfir::AsExprOp>()) {
+          src = asExpr.getVar();
+        }
+      } else if (lhs.getType().isa<fir::BoxCharType>())
+        if (!lhs.getDefiningOp<hlfir::DeclareOp>())
+          src = genUnboxChar(loc, builder, lhs).getAddr();
+      builder.create<fir::CharConvertOp>(loc, src, origBufferSize, dest);
 
       return hlfir::EntityWithAttributes{builder.create<hlfir::DeclareOp>(
           loc, dest, "ctor.temp", /*shape=*/nullptr,
diff --git a/flang/test/Lower/HLFIR/charconvert.f90 b/flang/test/Lower/HLFIR/charconvert.f90
new file mode 100644
index 000000000000000..9779e79e4034d90
--- /dev/null
+++ b/flang/test/Lower/HLFIR/charconvert.f90
@@ -0,0 +1,60 @@
+! Test lowering of character concatenation to HLFIR
+! RUN: bbc -emit-hlfir -o - %s 2>&1 | FileCheck %s
+
+subroutine charconvert1(c,n)
+  character(*,4),intent(in) :: c(:)
+  integer,intent(in) :: n
+  interface
+     subroutine callee(c)
+       character(*),intent(in) :: c(:)
+     end subroutine callee
+  end interface
+  call show([character(n)::c])
+end subroutine charconvert1
+
+! CHECK-LABEL: func.func @_QPcharconvert1
+! CHECK:   %[[VAL_2:.*]]:2 = hlfir.declare %{{.*}} {fortran_attrs = #fir.var_attrs<intent_in>, uniq_name = "_QFcharconvert1Ec"} : (!fir.box<!fir.array<?x!fir.char<4,?>>>) -> (!fir.box<!fir.array<?x!fir.char<4,?>>>, !fir.box<!fir.array<?x!fir.char<4,?>>>)
+! CHECK:   ^bb0(%[[ARG2:.*]]: index):
+! CHECK:     %[[VAL_37:.*]] = fir.box_elesize %[[VAL_2]]#1 : (!fir.box<!fir.array<?x!fir.char<4,?>>>) -> index
+! CHECK:     %[[C4_4:.*]] = arith.constant 4 : index
+! CHECK:     %[[VAL_38:.*]] = arith.divsi %[[VAL_37]], %[[C4_4]] : index
+! CHECK:     %[[VAL_39:.*]] = hlfir.designate %[[VAL_2]]#0 (%[[ARG2]])  typeparams %[[VAL_38]] : (!fir.box<!fir.array<?x!fir.char<4,?>>>, index, index) -> !fir.boxchar<4>
+! CHECK:     %[[C4_5:.*]] = arith.constant 4 : index
+! CHECK:     %[[VAL_40:.*]] = arith.muli %[[VAL_38]], %[[C4_5]] : index
+! CHECK:     %[[VAL_41:.*]] = fir.alloca !fir.char<1,?>(%[[VAL_40]] : index)
+! CHECK:     %[[VAL_42:.*]]:2 = fir.unboxchar %[[VAL_39]] : (!fir.boxchar<4>) -> (!fir.ref<!fir.char<4,?>>, index)
+! CHECK:     fir.char_convert %[[VAL_42]]#0 for %[[VAL_38:.*]] to %[[VAL_41]] : !fir.ref<!fir.char<4,?>>, index, !fir.ref<!fir.char<1,?>>
+! CHECK:     %[[VAL_43:.*]]:2 = hlfir.declare %[[VAL_41]] typeparams %[[VAL_38]] {uniq_name = "ctor.temp"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK:     hlfir.yield_element %[[VAL_43]]#0 : !fir.boxchar<1>
+! CHECK:   }
+
+subroutine test2(x)
+  integer,intent(in) :: x
+  character(kind=4) :: cx
+  cx = achar(x)
+end subroutine test2
+! CHECK-LABEL: func.func @_QPtest2
+! CHECK-SAME: %[[ARG0:.*]]: !fir.ref<i32>
+! CHECK:   %[[VAL_0:.*]] = fir.alloca !fir.char<1>
+! CHECK:   %[[C1:.*]] = arith.constant 1 : index
+! CHECK:   %[[VAL_1:.*]] = fir.alloca !fir.char<4> {bindc_name = "cx", uniq_name = "_QFtest2Ecx"}
+! CHECK:   %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] typeparams %[[C1]] {uniq_name = "_QFtest2Ecx"} : (!fir.ref<!fir.char<4>>, index) -> (!fir.ref<!fir.char<4>>, !fir.ref<!fir.char<4>>)
+! CHECK:   %[[VAL_3:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<intent_in>, uniq_name = "_QFtest2Ex"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:   %[[VAL_4:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
+! CHECK:   %[[VAL_5:.*]] = fir.convert %[[VAL_4]] : (i32) -> i64
+! CHECK:   %[[VAL_6:.*]] = fir.convert %[[VAL_5]] : (i64) -> 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_0]] : !fir.ref<!fir.char<1>>
+! CHECK:   %[[FALSE:.*]] = arith.constant false
+! CHECK:   %[[VAL_9:.*]] = hlfir.as_expr %[[VAL_0]] move %[[FALSE]] : (!fir.ref<!fir.char<1>>, i1) -> !hlfir.expr<!fir.char<1>>
+! CHECK:   %[[C1_0:.*]] = arith.constant 1 : index
+! CHECK:   %[[VAL_10:.*]] = fir.alloca !fir.char<4,?>(%[[C1_0]] : index)
+! CHECK:   fir.char_convert %[[VAL_0:.*]] for %[[C1_0]] to %[[VAL_10]] : !fir.ref<!fir.char<1>>, index, !fir.ref<!fir.char<4,?>>
+! CHECK:   %[[VAL_11:.*]]:2 = hlfir.declare %[[VAL_10]] typeparams %[[C1_0]] {uniq_name = "ctor.temp"} : (!fir.ref<!fir.char<4,?>>, index) -> (!fir.boxchar<4>, !fir.ref<!fir.char<4,?>>)
+! CHECK:   %[[C1_I64:.*]] = arith.constant 1 : i64
+! CHECK:   %[[VAL_12:.*]] = hlfir.set_length %[[VAL_11]]#0 len %[[C1_I64]] : (!fir.boxchar<4>, i64) -> !hlfir.expr<!fir.char<4>>
+! CHECK:   hlfir.assign %[[VAL_12:.*]] to %[[VAL_2]]#0 : !hlfir.expr<!fir.char<4>>, !fir.ref<!fir.char<4>>
+! CHECK:   hlfir.destroy %[[VAL_9:.*]] : !hlfir.expr<!fir.char<1>>
+! CHECK:   return
+! CHECK: }

>From 94c1f85dbcc9332363c964eb6d583f0454956d2f Mon Sep 17 00:00:00 2001
From: cabreraam <cabreraam33 at gmail.com>
Date: Mon, 2 Oct 2023 15:34:33 -0400
Subject: [PATCH 2/3] address Jean's comment

---
 flang/lib/Lower/ConvertExprToHLFIR.cpp | 13 ++++++++-----
 flang/test/Lower/HLFIR/charconvert.f90 | 12 ++++++------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index 7fed9dc5c08868e..4161fc0bcc84279 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -1403,7 +1403,7 @@ struct UnaryOp<Fortran::evaluate::Parentheses<T>> {
   }
 };
 
-static fir::CharBoxValue genUnboxChar(mlir::Location loc,
+/*static fir::CharBoxValue genUnboxChar(mlir::Location loc,
                                       fir::FirOpBuilder &builder,
                                       mlir::Value boxChar) {
   if (auto emboxChar = boxChar.getDefiningOp<fir::EmboxCharOp>())
@@ -1418,7 +1418,7 @@ static fir::CharBoxValue genUnboxChar(mlir::Location loc,
     if (mlir::Value explicitlen = varIface.getExplicitCharLen())
       len = explicitlen;
   return {addr, len};
-}
+}*/
 template <Fortran::common::TypeCategory TC1, int KIND,
           Fortran::common::TypeCategory TC2>
 struct UnaryOp<
@@ -1450,15 +1450,18 @@ struct UnaryOp<
       // allocate space on the stack for toBuffer
       auto dest = builder.create<fir::AllocaOp>(loc, toTy,
                                                 mlir::ValueRange{bufferSize});
-      auto src = lhs.getFirBase();
+      /*auto src = lhs.getFirBase();
       if (lhs.getType().isa<hlfir::ExprType>()) {
         if (auto asExpr = lhs.getDefiningOp<hlfir::AsExprOp>()) {
           src = asExpr.getVar();
         }
       } else if (lhs.getType().isa<fir::BoxCharType>())
         if (!lhs.getDefiningOp<hlfir::DeclareOp>())
-          src = genUnboxChar(loc, builder, lhs).getAddr();
-      builder.create<fir::CharConvertOp>(loc, src, origBufferSize, dest);
+          src = genUnboxChar(loc, builder, lhs).getAddr();*/
+      auto src = hlfir::convertToAddress(loc, builder, lhs,
+                                         lhs.getFortranElementType());
+      builder.create<fir::CharConvertOp>(loc, src.first.getCharBox()->getAddr(),
+                                         origBufferSize, dest);
 
       return hlfir::EntityWithAttributes{builder.create<hlfir::DeclareOp>(
           loc, dest, "ctor.temp", /*shape=*/nullptr,
diff --git a/flang/test/Lower/HLFIR/charconvert.f90 b/flang/test/Lower/HLFIR/charconvert.f90
index 9779e79e4034d90..b76eea665234ea1 100644
--- a/flang/test/Lower/HLFIR/charconvert.f90
+++ b/flang/test/Lower/HLFIR/charconvert.f90
@@ -28,18 +28,18 @@ end subroutine charconvert1
 ! CHECK:     hlfir.yield_element %[[VAL_43]]#0 : !fir.boxchar<1>
 ! CHECK:   }
 
-subroutine test2(x)
+subroutine charconvert2(x)
   integer,intent(in) :: x
   character(kind=4) :: cx
   cx = achar(x)
-end subroutine test2
-! CHECK-LABEL: func.func @_QPtest2
+end subroutine charconvert2
+! CHECK-LABEL: func.func @_QPcharconvert2
 ! CHECK-SAME: %[[ARG0:.*]]: !fir.ref<i32>
 ! CHECK:   %[[VAL_0:.*]] = fir.alloca !fir.char<1>
 ! CHECK:   %[[C1:.*]] = arith.constant 1 : index
-! CHECK:   %[[VAL_1:.*]] = fir.alloca !fir.char<4> {bindc_name = "cx", uniq_name = "_QFtest2Ecx"}
-! CHECK:   %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] typeparams %[[C1]] {uniq_name = "_QFtest2Ecx"} : (!fir.ref<!fir.char<4>>, index) -> (!fir.ref<!fir.char<4>>, !fir.ref<!fir.char<4>>)
-! CHECK:   %[[VAL_3:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<intent_in>, uniq_name = "_QFtest2Ex"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:   %[[VAL_1:.*]] = fir.alloca !fir.char<4> {bindc_name = "cx", uniq_name = "_QFcharconvert2Ecx"}
+! CHECK:   %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] typeparams %[[C1]] {uniq_name = "_QFcharconvert2Ecx"} : (!fir.ref<!fir.char<4>>, index) -> (!fir.ref<!fir.char<4>>, !fir.ref<!fir.char<4>>)
+! CHECK:   %[[VAL_3:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<intent_in>, uniq_name = "_QFcharconvert2Ex"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:   %[[VAL_4:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
 ! CHECK:   %[[VAL_5:.*]] = fir.convert %[[VAL_4]] : (i32) -> i64
 ! CHECK:   %[[VAL_6:.*]] = fir.convert %[[VAL_5]] : (i64) -> i8

>From 186ae265a9a4351836a1be9e386bdbf40ad50720 Mon Sep 17 00:00:00 2001
From: cabreraam <cabreraam33 at gmail.com>
Date: Mon, 2 Oct 2023 15:47:19 -0400
Subject: [PATCH 3/3] additional cleanup

---
 flang/lib/Lower/ConvertExprToHLFIR.cpp | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index 4161fc0bcc84279..e00a56e024b3f9a 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -1403,22 +1403,6 @@ struct UnaryOp<Fortran::evaluate::Parentheses<T>> {
   }
 };
 
-/*static fir::CharBoxValue genUnboxChar(mlir::Location loc,
-                                      fir::FirOpBuilder &builder,
-                                      mlir::Value boxChar) {
-  if (auto emboxChar = boxChar.getDefiningOp<fir::EmboxCharOp>())
-    return {emboxChar.getMemref(), emboxChar.getLen()};
-  mlir::Type refType = fir::ReferenceType::get(
-      boxChar.getType().cast<fir::BoxCharType>().getEleTy());
-  auto unboxed = builder.create<fir::UnboxCharOp>(
-      loc, refType, builder.getIndexType(), boxChar);
-  mlir::Value addr = unboxed.getResult(0);
-  mlir::Value len = unboxed.getResult(1);
-  if (auto varIface = boxChar.getDefiningOp<fir::FortranVariableOpInterface>())
-    if (mlir::Value explicitlen = varIface.getExplicitCharLen())
-      len = explicitlen;
-  return {addr, len};
-}*/
 template <Fortran::common::TypeCategory TC1, int KIND,
           Fortran::common::TypeCategory TC2>
 struct UnaryOp<
@@ -1450,14 +1434,6 @@ struct UnaryOp<
       // allocate space on the stack for toBuffer
       auto dest = builder.create<fir::AllocaOp>(loc, toTy,
                                                 mlir::ValueRange{bufferSize});
-      /*auto src = lhs.getFirBase();
-      if (lhs.getType().isa<hlfir::ExprType>()) {
-        if (auto asExpr = lhs.getDefiningOp<hlfir::AsExprOp>()) {
-          src = asExpr.getVar();
-        }
-      } else if (lhs.getType().isa<fir::BoxCharType>())
-        if (!lhs.getDefiningOp<hlfir::DeclareOp>())
-          src = genUnboxChar(loc, builder, lhs).getAddr();*/
       auto src = hlfir::convertToAddress(loc, builder, lhs,
                                          lhs.getFortranElementType());
       builder.create<fir::CharConvertOp>(loc, src.first.getCharBox()->getAddr(),



More information about the flang-commits mailing list