Refactor the hlfir declare utility for clarity

Author: ashermancinelli

flang/lib/Optimizer/Dialect/FIROps.cpp

@@ -1542,8 +1542,7 @@ bool fir::ConvertOp::canBeConverted(mlir::Type inType, mlir::Type outType) {
 // 1. passing an entity to an external function and there's nothing we can do
 //    about volatility after that happens, or
 // 2. for code generation, at which point we represent volatility with
-// attributes
-//    on the LLVM instructions and intrinsics.
+//    attributes on the LLVM instructions and intrinsics.
 //
 // For all other cases, volatility ought to match exactly.
 static mlir::LogicalResult verifyVolatility(mlir::Type inType,

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

@@ -207,34 +207,37 @@ static bool hasExplicitLowerBounds(mlir::Value shape) {
          mlir::isa<fir::ShapeShiftType, fir::ShiftType>(shape.getType());
 }
 
-static std::pair<mlir::Type, mlir::Value> updateDeclaredInputTypeWithVolatility(
-    mlir::Type inputType, mlir::Value memref, mlir::OpBuilder &builder,
-    fir::FortranVariableFlagsAttr fortran_attrs) {
-  if (fortran_attrs &&
-      bitEnumContainsAny(fortran_attrs.getFlags(),
-                         fir::FortranVariableFlagsEnum::fortran_volatile)) {
-    // A volatile pointer's pointee is volatile.
-    const bool isPointer = bitEnumContainsAny(
-        fortran_attrs.getFlags(), fir::FortranVariableFlagsEnum::pointer);
-    // An allocatable's inner type's volatility matches that of the reference.
-    const bool isAllocatable = bitEnumContainsAny(
-        fortran_attrs.getFlags(), fir::FortranVariableFlagsEnum::allocatable);
-    auto updateType = [&](auto t) {
-      using FIRT = decltype(t);
-      auto elementType = t.getEleTy();
-      const bool elementTypeIsBox = mlir::isa<fir::BoxType>(elementType);
-      const bool elementTypeIsVolatile = isPointer || isAllocatable ||
-                                         elementTypeIsBox ||
-                                         fir::isa_volatile_type(elementType);
-      auto newEleTy =
-          fir::updateTypeWithVolatility(elementType, elementTypeIsVolatile);
-      inputType = FIRT::get(newEleTy, true);
-    };
-    llvm::TypeSwitch<mlir::Type>(inputType)
-        .Case<fir::ReferenceType, fir::BoxType, fir::ClassType>(updateType);
-    memref =
-        builder.create<fir::VolatileCastOp>(memref.getLoc(), inputType, memref);
+static std::pair<mlir::Type, mlir::Value>
+updateDeclaredInputTypeWithVolatility(mlir::Type inputType, mlir::Value memref,
+                                      mlir::OpBuilder &builder,
+                                      fir::FortranVariableFlagsEnum flags) {
+  if (!bitEnumContainsAny(flags,
+                          fir::FortranVariableFlagsEnum::fortran_volatile)) {
+    return std::make_pair(inputType, memref);
   }
+
+  // A volatile pointer's pointee is volatile.
+  const bool isPointer =
+      bitEnumContainsAny(flags, fir::FortranVariableFlagsEnum::pointer);
+  // An allocatable's inner type's volatility matches that of the reference.
+  const bool isAllocatable =
+      bitEnumContainsAny(flags, fir::FortranVariableFlagsEnum::allocatable);
+
+  auto updateType = [&](auto t) {
+    using FIRT = decltype(t);
+    auto elementType = t.getEleTy();
+    const bool elementTypeIsBox = mlir::isa<fir::BaseBoxType>(elementType);
+    const bool elementTypeIsVolatile = isPointer || isAllocatable ||
+                                       elementTypeIsBox ||
+                                       fir::isa_volatile_type(elementType);
+    auto newEleTy =
+        fir::updateTypeWithVolatility(elementType, elementTypeIsVolatile);
+    inputType = FIRT::get(newEleTy, true);
+  };
+  llvm::TypeSwitch<mlir::Type>(inputType)
+      .Case<fir::ReferenceType, fir::BoxType, fir::ClassType>(updateType);
+  memref =
+      builder.create<fir::VolatileCastOp>(memref.getLoc(), inputType, memref);
   return std::make_pair(inputType, memref);
 }
 
@@ -248,8 +251,11 @@ void hlfir::DeclareOp::build(mlir::OpBuilder &builder,
   auto nameAttr = builder.getStringAttr(uniq_name);
   mlir::Type inputType = memref.getType();
   bool hasExplicitLbs = hasExplicitLowerBounds(shape);
-  std::tie(inputType, memref) = updateDeclaredInputTypeWithVolatility(
-      inputType, memref, builder, fortran_attrs);
+  if (fortran_attrs) {
+    const auto flags = fortran_attrs.getFlags();
+    std::tie(inputType, memref) = updateDeclaredInputTypeWithVolatility(
+        inputType, memref, builder, flags);
+  }
   mlir::Type hlfirVariableType =
       getHLFIRVariableType(inputType, hasExplicitLbs);
   build(builder, result, {hlfirVariableType, inputType}, memref, shape,

flang/test/Fir/invalid.fir

@@ -1260,7 +1260,7 @@ func.func @dc_invalid_reduction(%arg0: index, %arg1: index) {
 
 // Should fail when volatility changes from a fir.convert
 func.func @bad_convert_volatile(%arg0: !fir.ref<i32>) -> !fir.ref<i32, volatile> {
-  // expected-error@+1 {{op this conversion does not preserve volatilit}}
+  // expected-error@+1 {{op this conversion does not preserve volatility}}
   %0 = fir.convert %arg0 : (!fir.ref<i32>) -> !fir.ref<i32, volatile>
   return %0 : !fir.ref<i32, volatile>
 }