Reland "[flang][nfc] Support volatility in Fir ops" (#135039)

#134858 had an extraneous include which caused the shared library builds
to break.

flang/include/flang/Optimizer/Builder/FIRBuilder.h

@@ -397,6 +397,11 @@ public:
   mlir::Value createConvert(mlir::Location loc, mlir::Type toTy,
                             mlir::Value val);
 
+  /// Create a fir.convert op with a volatile cast if the source value's type
+  /// does not match the target type's volatility.
+  mlir::Value createConvertWithVolatileCast(mlir::Location loc, mlir::Type toTy,
+                                            mlir::Value val);
+
   /// Create a fir.store of \p val into \p addr. A lazy conversion
   /// of \p val to the element type of \p addr is created if needed.
   void createStoreWithConvert(mlir::Location loc, mlir::Value val,

flang/include/flang/Optimizer/Dialect/FIROps.h

@@ -50,6 +50,12 @@ struct DebuggingResource
   mlir::StringRef getName() final { return "DebuggingResource"; }
 };
 
+/// Model operations which read from/write to volatile memory
+struct VolatileMemoryResource
+    : public mlir::SideEffects::Resource::Base<VolatileMemoryResource> {
+  mlir::StringRef getName() final { return "VolatileMemoryResource"; }
+};
+
 class CoordinateIndicesAdaptor;
 using IntOrValue = llvm::PointerUnion<mlir::IntegerAttr, mlir::Value>;
 

flang/include/flang/Optimizer/Dialect/FIROps.td

@@ -286,7 +286,8 @@ def fir_FreeMemOp : fir_Op<"freemem", [MemoryEffects<[MemFree]>]> {
   let assemblyFormat = "$heapref attr-dict `:` qualified(type($heapref))";
 }
 
-def fir_LoadOp : fir_OneResultOp<"load", [FirAliasTagOpInterface]> {
+def fir_LoadOp : fir_OneResultOp<"load", [FirAliasTagOpInterface,
+                                          DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
   let summary = "load a value from a memory reference";
   let description = [{
     Load a value from a memory reference into an ssa-value (virtual register).
@@ -302,7 +303,7 @@ def fir_LoadOp : fir_OneResultOp<"load", [FirAliasTagOpInterface]> {
     or null.
   }];
 
-  let arguments = (ins Arg<AnyReferenceLike, "", [MemRead]>:$memref,
+  let arguments = (ins AnyReferenceLike:$memref,
                   OptionalAttr<LLVM_TBAATagArrayAttr>:$tbaa);
 
   let builders = [OpBuilder<(ins "mlir::Value":$refVal)>,
@@ -315,7 +316,8 @@ def fir_LoadOp : fir_OneResultOp<"load", [FirAliasTagOpInterface]> {
   }];
 }
 
-def fir_StoreOp : fir_Op<"store", [FirAliasTagOpInterface]> {
+def fir_StoreOp : fir_Op<"store", [FirAliasTagOpInterface,
+                                   DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
   let summary = "store an SSA-value to a memory location";
 
   let description = [{
@@ -335,7 +337,7 @@ def fir_StoreOp : fir_Op<"store", [FirAliasTagOpInterface]> {
   }];
 
   let arguments = (ins AnyType:$value,
-                   Arg<AnyReferenceLike, "", [MemWrite]>:$memref,
+                   AnyReferenceLike:$memref,
                    OptionalAttr<LLVM_TBAATagArrayAttr>:$tbaa);
 
   let builders = [OpBuilder<(ins "mlir::Value":$value, "mlir::Value":$memref)>];
@@ -348,7 +350,7 @@ def fir_StoreOp : fir_Op<"store", [FirAliasTagOpInterface]> {
   }];
 }
 
-def fir_CopyOp : fir_Op<"copy", []> {
+def fir_CopyOp : fir_Op<"copy", [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
   let summary = "copy constant size memory";
 
   let description = [{
@@ -369,8 +371,8 @@ def fir_CopyOp : fir_Op<"copy", []> {
     TODO: add FirAliasTagOpInterface to carry TBAA.
   }];
 
-  let arguments = (ins Arg<AnyRefOfConstantSizeAggregateType, "", [MemRead]>:$source,
-                       Arg<AnyRefOfConstantSizeAggregateType, "", [MemWrite]>:$destination,
+  let arguments = (ins AnyRefOfConstantSizeAggregateType:$source,
+                       AnyRefOfConstantSizeAggregateType:$destination,
                        OptionalAttr<UnitAttr>:$no_overlap);
 
   let builders = [OpBuilder<(ins "mlir::Value":$source,
@@ -1373,7 +1375,8 @@ def fir_BoxTypeDescOp : fir_SimpleOneResultOp<"box_tdesc", [NoMemoryEffect]> {
 //   !- Merge the new and old values into the memory for "A"
 //   array_merge_store <updated A> to <A's address>
 
-def fir_ArrayLoadOp : fir_Op<"array_load", [AttrSizedOperandSegments]> {
+def fir_ArrayLoadOp : fir_Op<"array_load", [AttrSizedOperandSegments,
+                                            DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
 
   let summary = "Load an array as a value.";
 
@@ -1412,7 +1415,7 @@ def fir_ArrayLoadOp : fir_Op<"array_load", [AttrSizedOperandSegments]> {
   }];
 
   let arguments = (ins
-    Arg<AnyRefOrBox, "", [MemRead]>:$memref,
+    AnyRefOrBox:$memref,
     Optional<AnyShapeOrShiftType>:$shape,
     Optional<fir_SliceType>:$slice,
     Variadic<AnyIntegerType>:$typeparams
@@ -1624,7 +1627,7 @@ def fir_ArrayAccessOp : fir_Op<"array_access", [AttrSizedOperandSegments,
 
     It is only possible to use `array_access` on an `array_load` result value or
     a value that can be trace back transitively to an `array_load` as the
-    dominating source. Other array operation such as `array_amend` can be in
+    dominating source. Other array operations such as `array_amend` can be in
     between.
 
     TODO: The above restriction is not enforced. The design of the operation
@@ -1685,7 +1688,7 @@ def fir_ArrayAmendOp : fir_Op<"array_amend", [NoMemoryEffect]> {
 }
 
 def fir_ArrayMergeStoreOp : fir_Op<"array_merge_store",
-    [AttrSizedOperandSegments]> {
+    [AttrSizedOperandSegments, DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
 
   let summary = "Store merged array value to memory.";
 
@@ -1714,7 +1717,7 @@ def fir_ArrayMergeStoreOp : fir_Op<"array_merge_store",
   let arguments = (ins
     fir_SequenceType:$original,
     fir_SequenceType:$sequence,
-    Arg<AnyRefOrBox, "", [MemWrite]>:$memref,
+    AnyRefOrBox:$memref,
     Optional<fir_SliceType>:$slice,
     Variadic<AnyIntegerType>:$typeparams
   );
@@ -2752,6 +2755,22 @@ def fir_AddrOfOp : fir_OneResultOp<"address_of", [NoMemoryEffect]> {
   let assemblyFormat = "`(` $symbol `)` attr-dict `:` type($resTy)";
 }
 
+def fir_VolatileCastOp : fir_SimpleOneResultOp<"volatile_cast", [NoMemoryEffect]> {
+  let summary = "cast between volatile and non-volatile types";
+  let description = [{
+    Cast between volatile and non-volatile types. The types must be otherwise
+    identical. A value's volatility cannot be changed by a fir.convert operation.
+    Reinterpreting a value as volatile must be done explicitly using this operation.
+  }];
+  let arguments = (ins AnyRefOrBox:$value);
+  let results = (outs AnyRefOrBox:$res);
+  let assemblyFormat = [{
+    $value attr-dict `:` functional-type($value, results)
+  }];
+  let hasVerifier = 1;
+  let hasFolder = 1;
+}
+
 def fir_ConvertOp : fir_SimpleOneResultOp<"convert", [NoMemoryEffect]> {
   let summary = "encapsulates all Fortran entity type conversions";
 

flang/include/flang/Optimizer/Dialect/FIROpsSupport.h

@@ -15,13 +15,26 @@
 
 namespace fir {
 
-/// Return true iff the Operation is a non-volatile LoadOp or ArrayLoadOp.
-inline bool nonVolatileLoad(mlir::Operation *op) {
-  if (auto load = mlir::dyn_cast<fir::LoadOp>(op))
-    return !load->getAttr("volatile");
-  if (auto arrLoad = mlir::dyn_cast<fir::ArrayLoadOp>(op))
-    return !arrLoad->getAttr("volatile");
-  return false;
+/// The LLVM dialect represents volatile memory accesses as read and write
+/// effects to an unknown memory location, but this may be overly conservative.
+/// LLVM Language Reference only specifies that volatile memory accesses
+/// must not be reordered relative to other volatile memory accesses, so it
+/// is more precise to use a separate memory resource for volatile memory
+/// accesses.
+inline void addVolatileMemoryEffects(
+    mlir::TypeRange type,
+    llvm::SmallVectorImpl<
+        mlir::SideEffects::EffectInstance<mlir::MemoryEffects::Effect>>
+        &effects) {
+  for (mlir::Type t : type) {
+    if (fir::isa_volatile_type(t)) {
+      effects.emplace_back(mlir::MemoryEffects::Read::get(),
+                           fir::VolatileMemoryResource::get());
+      effects.emplace_back(mlir::MemoryEffects::Write::get(),
+                           fir::VolatileMemoryResource::get());
+      break;
+    }
+  }
 }
 
 /// Return true iff the Operation is a call.

flang/lib/Lower/ConvertExprToHLFIR.cpp

@@ -415,8 +415,13 @@ private:
         .Case<fir::SequenceType>([&](fir::SequenceType seqTy) -> mlir::Type {
           return fir::SequenceType::get(seqTy.getShape(), newEleTy);
         })
-        .Case<fir::PointerType, fir::HeapType, fir::ReferenceType, fir::BoxType,
-              fir::ClassType>([&](auto t) -> mlir::Type {
+        .Case<fir::ReferenceType, fir::BoxType, fir::ClassType>(
+            [&](auto t) -> mlir::Type {
+              using FIRT = decltype(t);
+              return FIRT::get(changeElementType(t.getEleTy(), newEleTy),
+                               t.isVolatile());
+            })
+        .Case<fir::PointerType, fir::HeapType>([&](auto t) -> mlir::Type {
           using FIRT = decltype(t);
           return FIRT::get(changeElementType(t.getEleTy(), newEleTy));
         })

flang/lib/Optimizer/Builder/FIRBuilder.cpp

@@ -577,6 +577,17 @@ mlir::Value fir::FirOpBuilder::convertWithSemantics(
   return createConvert(loc, toTy, val);
 }
 
+mlir::Value fir::FirOpBuilder::createConvertWithVolatileCast(mlir::Location loc,
+                                                             mlir::Type toTy,
+                                                             mlir::Value val) {
+  if (fir::isa_volatile_type(val.getType()) != fir::isa_volatile_type(toTy)) {
+    mlir::Type volatileAdjustedType = fir::updateTypeWithVolatility(
+        val.getType(), fir::isa_volatile_type(toTy));
+    val = create<fir::VolatileCastOp>(loc, volatileAdjustedType, val);
+  }
+  return createConvert(loc, toTy, val);
+}
+
 mlir::Value fir::factory::createConvert(mlir::OpBuilder &builder,
                                         mlir::Location loc, mlir::Type toTy,
                                         mlir::Value val) {
@@ -739,19 +750,20 @@ mlir::Value fir::FirOpBuilder::createBox(mlir::Location loc,
         << itemAddr.getType();
     llvm_unreachable("not a memory reference type");
   }
+  const bool isVolatile = fir::isa_volatile_type(itemAddr.getType());
   mlir::Type boxTy;
   mlir::Value tdesc;
   // Avoid to wrap a box/class with box/class.
   if (mlir::isa<fir::BaseBoxType>(elementType)) {
     boxTy = elementType;
   } else {
-    boxTy = fir::BoxType::get(elementType);
+    boxTy = fir::BoxType::get(elementType, isVolatile);
     if (isPolymorphic) {
       elementType = fir::updateTypeForUnlimitedPolymorphic(elementType);
       if (isAssumedType)
-        boxTy = fir::BoxType::get(elementType);
+        boxTy = fir::BoxType::get(elementType, isVolatile);
       else
-        boxTy = fir::ClassType::get(elementType);
+        boxTy = fir::ClassType::get(elementType, isVolatile);
     }
   }
 

flang/lib/Optimizer/Dialect/FIROps.cpp

@@ -29,6 +29,7 @@
 #include "mlir/IR/Matchers.h"
 #include "mlir/IR/OpDefinition.h"
 #include "mlir/IR/PatternMatch.h"
+#include "mlir/IR/TypeRange.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/TypeSwitch.h"
@@ -853,6 +854,15 @@ std::vector<mlir::Value> fir::ArrayLoadOp::getExtents() {
   return {};
 }
 
+void fir::ArrayLoadOp::getEffects(
+    llvm::SmallVectorImpl<
+        mlir::SideEffects::EffectInstance<mlir::MemoryEffects::Effect>>
+        &effects) {
+  effects.emplace_back(mlir::MemoryEffects::Read::get(), &getMemrefMutable(),
+                       mlir::SideEffects::DefaultResource::get());
+  addVolatileMemoryEffects({getMemref().getType()}, effects);
+}
+
 llvm::LogicalResult fir::ArrayLoadOp::verify() {
   auto eleTy = fir::dyn_cast_ptrOrBoxEleTy(getMemref().getType());
   auto arrTy = mlir::dyn_cast<fir::SequenceType>(eleTy);
@@ -935,6 +945,15 @@ llvm::LogicalResult fir::ArrayMergeStoreOp::verify() {
   return mlir::success();
 }
 
+void fir::ArrayMergeStoreOp::getEffects(
+    llvm::SmallVectorImpl<
+        mlir::SideEffects::EffectInstance<mlir::MemoryEffects::Effect>>
+        &effects) {
+  effects.emplace_back(mlir::MemoryEffects::Write::get(), &getMemrefMutable(),
+                       mlir::SideEffects::DefaultResource::get());
+  addVolatileMemoryEffects({getMemref().getType()}, effects);
+}
+
 //===----------------------------------------------------------------------===//
 // ArrayFetchOp
 //===----------------------------------------------------------------------===//
@@ -1322,6 +1341,36 @@ mlir::ParseResult fir::CmpcOp::parse(mlir::OpAsmParser &parser,
   return parseCmpOp<fir::CmpcOp>(parser, result);
 }
 
+//===----------------------------------------------------------------------===//
+// VolatileCastOp
+//===----------------------------------------------------------------------===//
+
+llvm::LogicalResult fir::VolatileCastOp::verify() {
+  mlir::Type fromType = getValue().getType();
+  mlir::Type toType = getType();
+  // Other than volatility, are the types identical?
+  const bool sameBaseType =
+      llvm::TypeSwitch<mlir::Type, bool>(fromType)
+          .Case<fir::BoxType, fir::ReferenceType, fir::ClassType>(
+              [&](auto type) {
+                using TYPE = decltype(type);
+                return mlir::isa<TYPE>(toType);
+              })
+          .Default([=](mlir::Type) { return fromType == toType; });
+  const bool sameElementType = fir::dyn_cast_ptrOrBoxEleTy(fromType) ==
+                               fir::dyn_cast_ptrOrBoxEleTy(toType);
+  if (!sameBaseType || !sameElementType)
+    return emitOpError("types must be identical except for volatility ")
+           << fromType << " / " << toType;
+  return mlir::success();
+}
+
+mlir::OpFoldResult fir::VolatileCastOp::fold(FoldAdaptor adaptor) {
+  if (getValue().getType() == getType())
+    return getValue();
+  return {};
+}
+
 //===----------------------------------------------------------------------===//
 // ConvertOp
 //===----------------------------------------------------------------------===//
@@ -1461,7 +1510,13 @@ bool fir::ConvertOp::canBeConverted(mlir::Type inType, mlir::Type outType) {
 }
 
 llvm::LogicalResult fir::ConvertOp::verify() {
-  if (canBeConverted(getValue().getType(), getType()))
+  mlir::Type inType = getValue().getType();
+  mlir::Type outType = getType();
+  if (fir::isa_volatile_type(inType) != fir::isa_volatile_type(outType))
+    return emitOpError("cannot convert between volatile and non-volatile "
+                       "types, use fir.volatile_cast instead ")
+           << inType << " / " << outType;
+  if (canBeConverted(inType, outType))
     return mlir::success();
   return emitOpError("invalid type conversion")
          << getValue().getType() << " / " << getType();
@@ -1787,6 +1842,10 @@ llvm::LogicalResult fir::EmboxOp::verify() {
     return emitOpError("slice must not be provided for a scalar");
   if (getSourceBox() && !mlir::isa<fir::ClassType>(getResult().getType()))
     return emitOpError("source_box must be used with fir.class result type");
+  if (fir::isa_volatile_type(getMemref().getType()) !=
+      fir::isa_volatile_type(getResult().getType()))
+    return emitOpError("cannot convert between volatile and non-volatile "
+                       "types, use fir.volatile_cast instead");
   return mlir::success();
 }
 
@@ -2599,6 +2658,15 @@ void fir::LoadOp::print(mlir::OpAsmPrinter &p) {
   p << " : " << getMemref().getType();
 }
 
+void fir::LoadOp::getEffects(
+    llvm::SmallVectorImpl<
+        mlir::SideEffects::EffectInstance<mlir::MemoryEffects::Effect>>
+        &effects) {
+  effects.emplace_back(mlir::MemoryEffects::Read::get(), &getMemrefMutable(),
+                       mlir::SideEffects::DefaultResource::get());
+  addVolatileMemoryEffects({getMemref().getType()}, effects);
+}
+
 //===----------------------------------------------------------------------===//
 // DoLoopOp
 //===----------------------------------------------------------------------===//
@@ -3951,6 +4019,15 @@ void fir::StoreOp::build(mlir::OpBuilder &builder, mlir::OperationState &result,
   build(builder, result, value, memref, {});
 }
 
+void fir::StoreOp::getEffects(
+    llvm::SmallVectorImpl<
+        mlir::SideEffects::EffectInstance<mlir::MemoryEffects::Effect>>
+        &effects) {
+  effects.emplace_back(mlir::MemoryEffects::Write::get(), &getMemrefMutable(),
+                       mlir::SideEffects::DefaultResource::get());
+  addVolatileMemoryEffects({getMemref().getType()}, effects);
+}
+
 //===----------------------------------------------------------------------===//
 // CopyOp
 //===----------------------------------------------------------------------===//
@@ -3971,6 +4048,19 @@ llvm::LogicalResult fir::CopyOp::verify() {
   return mlir::success();
 }
 
+void fir::CopyOp::getEffects(
+    llvm::SmallVectorImpl<
+        mlir::SideEffects::EffectInstance<mlir::MemoryEffects::Effect>>
+        &effects) {
+  effects.emplace_back(mlir::MemoryEffects::Read::get(), &getSourceMutable(),
+                       mlir::SideEffects::DefaultResource::get());
+  effects.emplace_back(mlir::MemoryEffects::Write::get(),
+                       &getDestinationMutable(),
+                       mlir::SideEffects::DefaultResource::get());
+  addVolatileMemoryEffects({getDestination().getType(), getSource().getType()},
+                           effects);
+}
+
 //===----------------------------------------------------------------------===//
 // StringLitOp
 //===----------------------------------------------------------------------===//

flang/test/Fir/cse.fir

@@ -55,3 +55,23 @@ func.func @fun(%a : !fir.ref<i64>) -> i64 {
   %5 = arith.subi %4, %4 : i64
   return %5 : i64
 }
+
+// -----
+
+// Check that the redundant ops on volatile operands are PRESERVED.
+func.func @fun(%arg0: !fir.ref<i64, volatile>) -> i64 {
+    %0 = fir.load %arg0 : !fir.ref<i64, volatile>
+    %1 = fir.load %arg0 : !fir.ref<i64, volatile>
+    %2 = arith.addi %0, %1 : i64
+    fir.store %2 to %arg0 : !fir.ref<i64, volatile>
+    fir.store %2 to %arg0 : !fir.ref<i64, volatile>
+    return %2 : i64
+}
+// CHECK-LABEL:   func.func @fun(%arg0: !fir.ref<i64, volatile>) -> i64 {
+// CHECK:           %[[VAL_1:.*]] = fir.load %arg0 : !fir.ref<i64, volatile>
+// CHECK:           %[[VAL_2:.*]] = fir.load %arg0 : !fir.ref<i64, volatile>
+// CHECK:           %[[VAL_3:.*]] = arith.addi %[[VAL_1]], %[[VAL_2]] : i64
+// CHECK:           fir.store %[[VAL_3]] to %arg0 : !fir.ref<i64, volatile>
+// CHECK:           fir.store %[[VAL_3]] to %arg0 : !fir.ref<i64, volatile>
+// CHECK:           return %[[VAL_3]] : i64
+// CHECK:         }

flang/test/Fir/invalid.fir

@@ -1257,3 +1257,57 @@ func.func @dc_invalid_reduction(%arg0: index, %arg1: index) {
   }
   return
 }
+
+// -----
+
+// 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 {{'fir.convert' op cannot convert between volatile and non-volatile types, use fir.volatile_cast instead}}
+  %0 = fir.convert %arg0 : (!fir.ref<i32>) -> !fir.ref<i32, volatile>
+  return %0 : !fir.ref<i32, volatile>
+}
+
+// -----
+
+// Should fail when volatility changes from a fir.convert
+func.func @bad_convert_volatile2(%arg0: !fir.ref<i32, volatile>) -> !fir.ref<i32> {
+  // expected-error@+1 {{'fir.convert' op cannot convert between volatile and non-volatile types, use fir.volatile_cast instead}}
+  %0 = fir.convert %arg0 : (!fir.ref<i32, volatile>) -> !fir.ref<i32>
+  return %0 : !fir.ref<i32>
+}
+
+// -----
+
+// Should fail when the element type and the containing type change
+func.func @bad_convert_volatile3(%arg0: !fir.ref<i32>) -> !fir.box<i64> {
+  // expected-error@+1 {{'fir.volatile_cast' op types must be identical except for volatility}}
+  %0 = fir.volatile_cast %arg0 : (!fir.ref<i32>) -> !fir.box<i64>
+  return %0 : !fir.box<i64>
+}
+
+// -----
+
+// Should fail when the containing type changes
+func.func @bad_convert_volatile4(%arg0: !fir.ref<i32>) -> !fir.box<i32> {
+  // expected-error@+1 {{'fir.volatile_cast' op types must be identical except for volatility}}
+  %0 = fir.volatile_cast %arg0 : (!fir.ref<i32>) -> !fir.box<i32>
+  return %0 : !fir.box<i32>
+}
+
+// -----
+
+// Should fail when the containing type changes
+func.func @bad_convert_volatile5(%arg0: !fir.ref<i32>) -> !fir.box<i32, volatile> {
+  // expected-error@+1 {{'fir.volatile_cast' op types must be identical except for volatility}}
+  %0 = fir.volatile_cast %arg0 : (!fir.ref<i32>) -> !fir.box<i32, volatile>
+  return %0 : !fir.box<i32, volatile>
+}
+
+// -----
+
+// Should fail when the element type changes
+func.func @bad_convert_volatile6(%arg0: !fir.ref<i32>) -> !fir.ref<i64> {
+  // expected-error@+1 {{'fir.volatile_cast' op types must be identical except for volatility}}
+  %0 = fir.volatile_cast %arg0 : (!fir.ref<i32>) -> !fir.ref<i64>
+  return %0 : !fir.ref<i64>
+}