[flang] Correctly prepare allocatable runtime call arguments (llvm#138727)

When lowering allocatables, the generated calls to runtime functions
were not using the runtime::createArguments utility which handles the
required conversions. createArguments is where I added the implicit
volatile casts to handle converting volatile variables to the
appropriate type based on their volatility in the callee. Because the
calls to allocatable runtime functions were not using this function,
their arguments were not casted to have the appropriate volatility.

Add a test to demonstrate that volatile and allocatable
class/box/reference types are appropriately casted before calling into
the runtime library.

Instead of using a recursive variadic template to perform the
conversions in createArguments, map over the arguments directly so that
createArguments can be called with an ArrayRef of arguments. Some cases
in Allocatable.cpp already had a vector of values at the point where
createArguments needed to be called - the new overload allows calling
with a vector of args or the variadic version with each argument spelled
out at the callsite.

This change resulted in the allocatable runtime calls having their
arguments converted left-to-right, which changed some of the test
results. I used CHECK-DAG to ignore the order.

Add some missing handling of volatile class entities, which I previously
missed because I had not yet enabled volatile class entities in Lower.

Author: ashermancinelli

flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h

@@ -26,6 +26,7 @@
 #include "flang/Support/Fortran.h"
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/MLIRContext.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include <cstdint>
 #include <functional>
@@ -824,33 +825,23 @@ static mlir::func::FuncOp getIORuntimeFunc(mlir::Location loc,
   return getRuntimeFunc<E>(loc, builder, /*isIO=*/true);
 }
 
-namespace helper {
-template <int N, typename A>
-void createArguments(llvm::SmallVectorImpl<mlir::Value> &result,
-                     fir::FirOpBuilder &builder, mlir::Location loc,
-                     mlir::FunctionType fTy, A arg) {
-  result.emplace_back(
-      builder.createConvertWithVolatileCast(loc, fTy.getInput(N), arg));
-}
-
-template <int N, typename A, typename... As>
-void createArguments(llvm::SmallVectorImpl<mlir::Value> &result,
-                     fir::FirOpBuilder &builder, mlir::Location loc,
-                     mlir::FunctionType fTy, A arg, As... args) {
-  result.emplace_back(
-      builder.createConvertWithVolatileCast(loc, fTy.getInput(N), arg));
-  createArguments<N + 1>(result, builder, loc, fTy, args...);
+inline llvm::SmallVector<mlir::Value>
+createArguments(fir::FirOpBuilder &builder, mlir::Location loc,
+                mlir::FunctionType fTy, llvm::ArrayRef<mlir::Value> args) {
+  return llvm::map_to_vector(llvm::zip_equal(fTy.getInputs(), args),
+                             [&](const auto &pair) -> mlir::Value {
+                               auto [type, argument] = pair;
+                               return builder.createConvertWithVolatileCast(
+                                   loc, type, argument);
+                             });
 }
-} // namespace helper
 
 /// Create a SmallVector of arguments for a runtime call.
 template <typename... As>
 llvm::SmallVector<mlir::Value>
 createArguments(fir::FirOpBuilder &builder, mlir::Location loc,
                 mlir::FunctionType fTy, As... args) {
-  llvm::SmallVector<mlir::Value> result;
-  helper::createArguments<0>(result, builder, loc, fTy, args...);
-  return result;
+  return createArguments(builder, loc, fTy, {args...});
 }
 
 } // namespace fir::runtime

flang/lib/Lower/Allocatable.cpp

@@ -138,12 +138,10 @@ static void genRuntimeSetBounds(fir::FirOpBuilder &builder, mlir::Location loc,
                                                                     builder)
           : fir::runtime::getRuntimeFunc<mkRTKey(AllocatableSetBounds)>(
                 loc, builder);
-  llvm::SmallVector<mlir::Value> args{box.getAddr(), dimIndex, lowerBound,
-                                      upperBound};
-  llvm::SmallVector<mlir::Value> operands;
-  for (auto [fst, snd] : llvm::zip(args, callee.getFunctionType().getInputs()))
-    operands.emplace_back(builder.createConvert(loc, snd, fst));
-  builder.create<fir::CallOp>(loc, callee, operands);
+  const auto args = fir::runtime::createArguments(
+      builder, loc, callee.getFunctionType(), box.getAddr(), dimIndex,
+      lowerBound, upperBound);
+  builder.create<fir::CallOp>(loc, callee, args);
 }
 
 /// Generate runtime call to set the lengths of a character allocatable or
@@ -162,9 +160,7 @@ static void genRuntimeInitCharacter(fir::FirOpBuilder &builder,
   if (inputTypes.size() != 5)
     fir::emitFatalError(
         loc, "AllocatableInitCharacter runtime interface not as expected");
-  llvm::SmallVector<mlir::Value> args;
-  args.push_back(builder.createConvert(loc, inputTypes[0], box.getAddr()));
-  args.push_back(builder.createConvert(loc, inputTypes[1], len));
+  llvm::SmallVector<mlir::Value> args = {box.getAddr(), len};
   if (kind == 0)
     kind = mlir::cast<fir::CharacterType>(box.getEleTy()).getFKind();
   args.push_back(builder.createIntegerConstant(loc, inputTypes[2], kind));
@@ -173,7 +169,9 @@ static void genRuntimeInitCharacter(fir::FirOpBuilder &builder,
   // TODO: coarrays
   int corank = 0;
   args.push_back(builder.createIntegerConstant(loc, inputTypes[4], corank));
-  builder.create<fir::CallOp>(loc, callee, args);
+  const auto convertedArgs = fir::runtime::createArguments(
+      builder, loc, callee.getFunctionType(), args);
+  builder.create<fir::CallOp>(loc, callee, convertedArgs);
 }
 
 /// Generate a sequence of runtime calls to allocate memory.
@@ -194,10 +192,9 @@ static mlir::Value genRuntimeAllocate(fir::FirOpBuilder &builder,
   args.push_back(errorManager.errMsgAddr);
   args.push_back(errorManager.sourceFile);
   args.push_back(errorManager.sourceLine);
-  llvm::SmallVector<mlir::Value> operands;
-  for (auto [fst, snd] : llvm::zip(args, callee.getFunctionType().getInputs()))
-    operands.emplace_back(builder.createConvert(loc, snd, fst));
-  return builder.create<fir::CallOp>(loc, callee, operands).getResult(0);
+  const auto convertedArgs = fir::runtime::createArguments(
+      builder, loc, callee.getFunctionType(), args);
+  return builder.create<fir::CallOp>(loc, callee, convertedArgs).getResult(0);
 }
 
 /// Generate a sequence of runtime calls to allocate memory and assign with the
@@ -213,14 +210,11 @@ static mlir::Value genRuntimeAllocateSource(fir::FirOpBuilder &builder,
                 loc, builder)
           : fir::runtime::getRuntimeFunc<mkRTKey(AllocatableAllocateSource)>(
                 loc, builder);
-  llvm::SmallVector<mlir::Value> args{
-      box.getAddr(),           fir::getBase(source),
-      errorManager.hasStat,    errorManager.errMsgAddr,
-      errorManager.sourceFile, errorManager.sourceLine};
-  llvm::SmallVector<mlir::Value> operands;
-  for (auto [fst, snd] : llvm::zip(args, callee.getFunctionType().getInputs()))
-    operands.emplace_back(builder.createConvert(loc, snd, fst));
-  return builder.create<fir::CallOp>(loc, callee, operands).getResult(0);
+  const auto args = fir::runtime::createArguments(
+      builder, loc, callee.getFunctionType(), box.getAddr(),
+      fir::getBase(source), errorManager.hasStat, errorManager.errMsgAddr,
+      errorManager.sourceFile, errorManager.sourceLine);
+  return builder.create<fir::CallOp>(loc, callee, args).getResult(0);
 }
 
 /// Generate runtime call to apply mold to the descriptor.
@@ -234,14 +228,12 @@ static void genRuntimeAllocateApplyMold(fir::FirOpBuilder &builder,
                                                                     builder)
           : fir::runtime::getRuntimeFunc<mkRTKey(AllocatableApplyMold)>(
                 loc, builder);
-  llvm::SmallVector<mlir::Value> args{
+  const auto args = fir::runtime::createArguments(
+      builder, loc, callee.getFunctionType(),
       fir::factory::getMutableIRBox(builder, loc, box), fir::getBase(mold),
       builder.createIntegerConstant(
-          loc, callee.getFunctionType().getInputs()[2], rank)};
-  llvm::SmallVector<mlir::Value> operands;
-  for (auto [fst, snd] : llvm::zip(args, callee.getFunctionType().getInputs()))
-    operands.emplace_back(builder.createConvert(loc, snd, fst));
-  builder.create<fir::CallOp>(loc, callee, operands);
+          loc, callee.getFunctionType().getInputs()[2], rank));
+  builder.create<fir::CallOp>(loc, callee, args);
 }
 
 /// Generate a runtime call to deallocate memory.
@@ -669,15 +661,13 @@ class AllocateStmtHelper {
 
     llvm::ArrayRef<mlir::Type> inputTypes =
         callee.getFunctionType().getInputs();
-    llvm::SmallVector<mlir::Value> args;
-    args.push_back(builder.createConvert(loc, inputTypes[0], box.getAddr()));
-    args.push_back(builder.createConvert(loc, inputTypes[1], typeDescAddr));
     mlir::Value rankValue =
         builder.createIntegerConstant(loc, inputTypes[2], rank);
     mlir::Value corankValue =
         builder.createIntegerConstant(loc, inputTypes[3], corank);
-    args.push_back(rankValue);
-    args.push_back(corankValue);
+    const auto args = fir::runtime::createArguments(
+        builder, loc, callee.getFunctionType(), box.getAddr(), typeDescAddr,
+        rankValue, corankValue);
     builder.create<fir::CallOp>(loc, callee, args);
   }
 
@@ -696,8 +686,6 @@ class AllocateStmtHelper {
 
     llvm::ArrayRef<mlir::Type> inputTypes =
         callee.getFunctionType().getInputs();
-    llvm::SmallVector<mlir::Value> args;
-    args.push_back(builder.createConvert(loc, inputTypes[0], box.getAddr()));
     mlir::Value categoryValue = builder.createIntegerConstant(
         loc, inputTypes[1], static_cast<int32_t>(category));
     mlir::Value kindValue =
@@ -706,10 +694,9 @@ class AllocateStmtHelper {
         builder.createIntegerConstant(loc, inputTypes[3], rank);
     mlir::Value corankValue =
         builder.createIntegerConstant(loc, inputTypes[4], corank);
-    args.push_back(categoryValue);
-    args.push_back(kindValue);
-    args.push_back(rankValue);
-    args.push_back(corankValue);
+    const auto args = fir::runtime::createArguments(
+        builder, loc, callee.getFunctionType(), box.getAddr(), categoryValue,
+        kindValue, rankValue, corankValue);
     builder.create<fir::CallOp>(loc, callee, args);
   }
 

flang/lib/Lower/ConvertExprToHLFIR.cpp

@@ -227,6 +227,12 @@ class HlfirDesignatorBuilder {
         isVolatile = true;
     }
 
+    // Check if the base type is volatile
+    if (partInfo.base.has_value()) {
+      mlir::Type baseType = partInfo.base.value().getType();
+      isVolatile = isVolatile || fir::isa_volatile_type(baseType);
+    }
+
     // Dynamic type of polymorphic base must be kept if the designator is
     // polymorphic.
     if (isPolymorphic(designatorNode))
@@ -238,12 +244,6 @@ class HlfirDesignatorBuilder {
     if (charType && charType.hasDynamicLen())
       return fir::BoxCharType::get(charType.getContext(), charType.getFKind());
 
-    // Check if the base type is volatile
-    if (partInfo.base.has_value()) {
-      mlir::Type baseType = partInfo.base.value().getType();
-      isVolatile = isVolatile || fir::isa_volatile_type(baseType);
-    }
-
     // Arrays with non default lower bounds or dynamic length or dynamic extent
     // need a fir.box to hold the dynamic or lower bound information.
     if (fir::hasDynamicSize(resultValueType) ||

flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp

@@ -210,15 +210,14 @@ static bool hasExplicitLowerBounds(mlir::Value shape) {
 static std::pair<mlir::Type, mlir::Value> updateDeclareInputTypeWithVolatility(
     mlir::Type inputType, mlir::Value memref, mlir::OpBuilder &builder,
     fir::FortranVariableFlagsAttr fortran_attrs) {
-  if (mlir::isa<fir::BoxType, fir::ReferenceType>(inputType) && fortran_attrs &&
+  if (fortran_attrs &&
       bitEnumContainsAny(fortran_attrs.getFlags(),
                          fir::FortranVariableFlagsEnum::fortran_volatile)) {
     const bool isPointer = bitEnumContainsAny(
         fortran_attrs.getFlags(), fir::FortranVariableFlagsEnum::pointer);
     auto updateType = [&](auto t) {
       using FIRT = decltype(t);
-      // If an entity is a pointer, the entity it points to is volatile, as far
-      // as consumers of the pointer are concerned.
+      // A volatile pointer's pointee is volatile.
       auto elementType = t.getEleTy();
       const bool elementTypeIsVolatile =
           isPointer || fir::isa_volatile_type(elementType);
@@ -227,8 +226,7 @@ static std::pair<mlir::Type, mlir::Value> updateDeclareInputTypeWithVolatility(
       inputType = FIRT::get(newEleTy, true);
     };
     llvm::TypeSwitch<mlir::Type>(inputType)
-        .Case<fir::ReferenceType, fir::BoxType>(updateType)
-        .Default([](mlir::Type t) { return t; });
+        .Case<fir::ReferenceType, fir::BoxType, fir::ClassType>(updateType);
     memref =
         builder.create<fir::VolatileCastOp>(memref.getLoc(), inputType, memref);
   }