[flang][hlfir] place scalar in memory in convertToBox/convertToAddress

[jeanPerier]

Summary

Implement the TODO. Be careful to use and propagate the expression
type to create the temporary since the mlir value may have been computed
with a different value type (e.g., i1 for logical) that should not be
used for in memory values that must have Fortran types.

Co-authored-by: Tom Eccles <[e-mail]>

[clementval]

LGTM

[tblah]

LGTM, thanks for this

flang/include/flang/Lower/ConvertExprToHLFIR.h

 fir::BoxValue convertExprToBox(mlir::Location loc,
                                Fortran::lower::AbstractConverter &,
                                const Fortran::lower::SomeExpr &,
                                Fortran::lower::SymMap &,
                                Fortran::lower::StatementContext &);
 fir::BoxValue convertToBox(mlir::Location loc,
                            Fortran::lower::AbstractConverter &,
                            hlfir::Entity entity,
-                           Fortran::lower::StatementContext &);
+                           Fortran::lower::StatementContext &,
+                           mlir::Type fortranType);
 
 /// Lower an evaluate::Expr to fir::ExtendedValue address.
 /// The address may be a raw fir.ref<T>, or a fir.box<T>/fir.class<T>, (pointer
 /// and allocatable are dereferenced).
 /// - If expression is not a variable, or is a designator with vector
 ///   subscripts, a temporary is created to hold the expression value and
 ///   is returned as:
 ///   - a fir.class<T> if the expression is polymorphic.

 fir::ExtendedValue convertExprToAddress(mlir::Location loc,
                                         Fortran::lower::AbstractConverter &,
                                         const Fortran::lower::SomeExpr &,
                                         Fortran::lower::SymMap &,
                                         Fortran::lower::StatementContext &);
 fir::ExtendedValue convertToAddress(mlir::Location loc,
                                     Fortran::lower::AbstractConverter &,
                                     hlfir::Entity entity,
-                                    Fortran::lower::StatementContext &);
+                                    Fortran::lower::StatementContext &,
+                                    mlir::Type fortranType);
 
 /// Lower an evaluate::Expr to a fir::ExtendedValue value.
 fir::ExtendedValue convertExprToValue(mlir::Location loc,
                                       Fortran::lower::AbstractConverter &,
                                       const Fortran::lower::SomeExpr &,
                                       Fortran::lower::SymMap &,
                                       Fortran::lower::StatementContext &);
 fir::ExtendedValue convertToValue(mlir::Location loc,

flang/lib/Lower/ConvertCall.cpp

if (arg.value()->handleDynamicOptional())
       TODO(loc, "intrinsic dynamically optional arguments");
     hlfir::Entity actual = arg.value()->getActual(loc, builder);
     if (!argLowering) {
       // No argument lowering instruction, lower by value.
       operands.emplace_back(
           Fortran::lower::convertToValue(loc, converter, actual, stmtCtx));
       continue;
     }
+    // Helper to get the type of the Fortran expression in case it is a
+    // computed value that must be placed in memory (logicals are computed as
+    // i1, but must be placed in memory as fir.logical).
+    auto getActualFortranElementType = [&]() {
+      const Fortran::lower::SomeExpr *expr =
+          callContext.procRef.UnwrapArgExpr(arg.index());
+      assert(expr && "must be an expr");
+      mlir::Type type = converter.genType(*expr);
+      return hlfir::getFortranElementType(type);
+    };
     // Ad-hoc argument lowering handling.
     fir::ArgLoweringRule argRules =
         fir::lowerIntrinsicArgumentAs(*argLowering, arg.index());
     switch (argRules.lowerAs) {
     case fir::LowerIntrinsicArgAs::Value:
       operands.emplace_back(
           Fortran::lower::convertToValue(loc, converter, actual, stmtCtx));
       continue;
     case fir::LowerIntrinsicArgAs::Addr:
-      operands.emplace_back(
-          Fortran::lower::convertToAddress(loc, converter, actual, stmtCtx));
+      operands.emplace_back(Fortran::lower::convertToAddress(
+          loc, converter, actual, stmtCtx, getActualFortranElementType()));
       continue;
     case fir::LowerIntrinsicArgAs::Box:
-      operands.emplace_back(
-          Fortran::lower::convertToBox(loc, converter, actual, stmtCtx));
+      operands.emplace_back(Fortran::lower::convertToBox(
+          loc, converter, actual, stmtCtx, getActualFortranElementType()));
       continue;
     case fir::LowerIntrinsicArgAs::Inquired:
       // Place hlfir.expr in memory, and unbox fir.boxchar. Other entities
       // are translated to fir::ExtendedValue without transformation (notably,
       // pointers/allocatable are not dereferenced).
       // TODO: once lowering to FIR retires, UBOUND and LBOUND can be simplified
       // since the fir.box lowered here are now guaranteed to contain the local
       // lower bounds thanks to the hlfir.declare (the extra rebox can be

flang/lib/Lower/ConvertExprToHLFIR.cpp

 
 hlfir::EntityWithAttributes Fortran::lower::convertExprToHLFIR(
     mlir::Location loc, Fortran::lower::AbstractConverter &converter,
     const Fortran::lower::SomeExpr &expr, Fortran::lower::SymMap &symMap,
     Fortran::lower::StatementContext &stmtCtx) {
   return HlfirBuilder(loc, converter, symMap, stmtCtx).gen(expr);
 }
 
+static fir::ExtendedValue placeTrivialInMemory(mlir::Location loc,
+                                               fir::FirOpBuilder &builder,
+                                               mlir::Value val,
+                                               mlir::Type fortranType) {
+  auto temp = builder.createTemporary(loc, fortranType);
+  builder.createStoreWithConvert(loc, val, temp);
+  return temp;
+}
+
 fir::BoxValue Fortran::lower::convertToBox(
     mlir::Location loc, Fortran::lower::AbstractConverter &converter,
-    hlfir::Entity entity, Fortran::lower::StatementContext &stmtCtx) {
+    hlfir::Entity entity, Fortran::lower::StatementContext &stmtCtx,
+    mlir::Type fortranType) {
   auto exv = Fortran::lower::translateToExtendedValue(
       loc, converter.getFirOpBuilder(), entity, stmtCtx);
   if (fir::isa_trivial(fir::getBase(exv).getType()))
-    TODO(loc, "place trivial in memory");
+    exv = placeTrivialInMemory(loc, converter.getFirOpBuilder(),
+                               fir::getBase(exv), fortranType);
   return fir::factory::createBoxValue(converter.getFirOpBuilder(), loc, exv);
 }
 fir::BoxValue Fortran::lower::convertExprToBox(
     mlir::Location loc, Fortran::lower::AbstractConverter &converter,
     const Fortran::lower::SomeExpr &expr, Fortran::lower::SymMap &symMap,
     Fortran::lower::StatementContext &stmtCtx) {
   hlfir::EntityWithAttributes loweredExpr =
       HlfirBuilder(loc, converter, symMap, stmtCtx).gen(expr);
-  return convertToBox(loc, converter, loweredExpr, stmtCtx);
+  return convertToBox(loc, converter, loweredExpr, stmtCtx,
+                      converter.genType(expr));
 }
 
 fir::ExtendedValue Fortran::lower::convertToAddress(
     mlir::Location loc, Fortran::lower::AbstractConverter &converter,
-    hlfir::Entity entity, Fortran::lower::StatementContext &stmtCtx) {
+    hlfir::Entity entity, Fortran::lower::StatementContext &stmtCtx,
+    mlir::Type fortranType) {
   fir::FirOpBuilder &builder = converter.getFirOpBuilder();
   entity = hlfir::derefPointersAndAllocatables(loc, builder, entity);
   fir::ExtendedValue exv =
       Fortran::lower::translateToExtendedValue(loc, builder, entity, stmtCtx);
   if (fir::isa_trivial(fir::getBase(exv).getType()))
-    TODO(loc, "place trivial in memory");
+    return placeTrivialInMemory(loc, builder, fir::getBase(exv), fortranType);
   return exv;
 }
+
 fir::ExtendedValue Fortran::lower::convertExprToAddress(
     mlir::Location loc, Fortran::lower::AbstractConverter &converter,
     const Fortran::lower::SomeExpr &expr, Fortran::lower::SymMap &symMap,
     Fortran::lower::StatementContext &stmtCtx) {
   hlfir::EntityWithAttributes loweredExpr =
       HlfirBuilder(loc, converter, symMap, stmtCtx).gen(expr);
-  return convertToAddress(loc, converter, loweredExpr, stmtCtx);
+  return convertToAddress(loc, converter, loweredExpr, stmtCtx,
+                          converter.genType(expr));
 }
 
 fir::ExtendedValue Fortran::lower::convertToValue(
     mlir::Location loc, Fortran::lower::AbstractConverter &converter,
     hlfir::Entity entity, Fortran::lower::StatementContext &stmtCtx) {
   auto &builder = converter.getFirOpBuilder();
   fir::ExtendedValue exv =
       Fortran::lower::translateToExtendedValue(loc, builder, entity, stmtCtx);

flang/test/Lower/HLFIR/expr-box.f90

 ! Test lowering of of expressions as fir.box
 ! RUN: bbc -hlfir -o - %s 2>&1 | FileCheck %s
 
 ! CHECK-LABEL: func.func @_QPfoo(
 ! CHECK-SAME:  %[[VAL_0:.*]]: !fir.ref<!fir.array<10xi32>>
 subroutine foo(x)
   integer :: x(21:30)
-  print *, x 
+  print *, x
 ! CHECK-DAG:  %[[VAL_3:.*]] = arith.constant 21 : index
 ! CHECK-DAG:  %[[VAL_4:.*]] = arith.constant 10 : index
 ! CHECK:  %[[VAL_5:.*]] = fir.shape_shift %[[VAL_3]], %[[VAL_4]] : (index, index) -> !fir.shapeshift<1>
 ! CHECK:  %[[VAL_6:.*]]:2 = hlfir.declare %[[VAL_0]](%[[VAL_5]]) {uniq_name = "_QFfooEx"} : (!fir.ref<!fir.array<10xi32>>, !fir.shapeshift<1>) -> (!fir.box<!fir.array<10xi32>>, !fir.ref<!fir.array<10xi32>>)
 ! CHECK:  fir.embox %[[VAL_6]]#1(%[[VAL_5]]) : (!fir.ref<!fir.array<10xi32>>, !fir.shapeshift<1>) -> !fir.box<!fir.array<10xi32>>
 end subroutine
+
+subroutine test_place_in_memory_and_embox()
+ logical(8) :: a, b
+ write(06), a.and.b
+end subroutine
+! CHECK-LABEL: func.func @_QPtest_place_in_memory_and_embox(
+! CHECK:  %[[TEMP:.*]] = fir.alloca !fir.logical<8>
+! CHECK:  %[[AND:.*]] = arith.andi {{.*}}
+! CHECK:  %[[CAST:.*]] = fir.convert %[[AND]] : (i1) -> !fir.logical<8>
+! CHECK:  fir.store %[[CAST]] to %[[TEMP]] : !fir.ref<!fir.logical<8>>
+! CHECK:  %[[BOX:.*]] = fir.embox %[[TEMP]] : (!fir.ref<!fir.logical<8>>) -> !fir.box<!fir.logical<8>>
+! CHECK:  %[[BOX_CAST:.*]] = fir.convert %[[BOX]] : (!fir.box<!fir.logical<8>>) -> !fir.box<none>
+! CHECK:  fir.call @_FortranAioOutputDescriptor(%{{.*}}, %[[BOX_CAST]])
+
+! check we can box a trivial value
+subroutine sumMask(s, a)
+  integer :: s
+  integer :: a(:)
+  s = sum(a, mask=.true.)
+endsubroutine
+! CHECK-LABEL: func.func @_QPsummask(
+! CHECK:      %[[TRUE:.*]] = arith.constant true
+! CHECK:      %[[ALLOC:.*]] = fir.alloca !fir.logical<4>
+! CHECK:      %[[TRUE_L4:.*]] = fir.convert %[[TRUE]] : (i1) -> !fir.logical<4>
+! CHECK-NEXT: fir.store %[[TRUE_L4]] to %[[ALLOC]]
+! CHECK-NEXT: %[[BOX:.*]] = fir.embox %[[ALLOC]] : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>