Revert "[flang][nfc] Support volatility in Fir ops (#134858)"

DavidSpickett · web-flow · commit e256292805f7 · 2025-04-09T15:41:07.000+01:00
This reverts commit e42f860.
diff --git a/flang/include/flang/Optimizer/Builder/FIRBuilder.h b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
@@ -397,11 +397,6 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
   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,
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.h b/flang/include/flang/Optimizer/Dialect/FIROps.h
@@ -50,12 +50,6 @@ 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>;
 
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -286,8 +286,7 @@ def fir_FreeMemOp : fir_Op<"freemem", [MemoryEffects<[MemFree]>]> {
   let assemblyFormat = "$heapref attr-dict `:` qualified(type($heapref))";
 }
 
-def fir_LoadOp : fir_OneResultOp<"load", [FirAliasTagOpInterface,
-                                          DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
+def fir_LoadOp : fir_OneResultOp<"load", [FirAliasTagOpInterface]> {
   let summary = "load a value from a memory reference";
   let description = [{
     Load a value from a memory reference into an ssa-value (virtual register).
@@ -303,7 +302,7 @@ def fir_LoadOp : fir_OneResultOp<"load", [FirAliasTagOpInterface,
     or null.
   }];
 
-  let arguments = (ins AnyReferenceLike:$memref,
+  let arguments = (ins Arg<AnyReferenceLike, "", [MemRead]>:$memref,
                   OptionalAttr<LLVM_TBAATagArrayAttr>:$tbaa);
 
   let builders = [OpBuilder<(ins "mlir::Value":$refVal)>,
@@ -316,8 +315,7 @@ def fir_LoadOp : fir_OneResultOp<"load", [FirAliasTagOpInterface,
   }];
 }
 
-def fir_StoreOp : fir_Op<"store", [FirAliasTagOpInterface,
-                                   DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
+def fir_StoreOp : fir_Op<"store", [FirAliasTagOpInterface]> {
   let summary = "store an SSA-value to a memory location";
 
   let description = [{
@@ -337,7 +335,7 @@ def fir_StoreOp : fir_Op<"store", [FirAliasTagOpInterface,
   }];
 
   let arguments = (ins AnyType:$value,
-                   AnyReferenceLike:$memref,
+                   Arg<AnyReferenceLike, "", [MemWrite]>:$memref,
                    OptionalAttr<LLVM_TBAATagArrayAttr>:$tbaa);
 
   let builders = [OpBuilder<(ins "mlir::Value":$value, "mlir::Value":$memref)>];
@@ -350,7 +348,7 @@ def fir_StoreOp : fir_Op<"store", [FirAliasTagOpInterface,
   }];
 }
 
-def fir_CopyOp : fir_Op<"copy", [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
+def fir_CopyOp : fir_Op<"copy", []> {
   let summary = "copy constant size memory";
 
   let description = [{
@@ -371,8 +369,8 @@ def fir_CopyOp : fir_Op<"copy", [DeclareOpInterfaceMethods<MemoryEffectsOpInterf
     TODO: add FirAliasTagOpInterface to carry TBAA.
   }];
 
-  let arguments = (ins AnyRefOfConstantSizeAggregateType:$source,
-                       AnyRefOfConstantSizeAggregateType:$destination,
+  let arguments = (ins Arg<AnyRefOfConstantSizeAggregateType, "", [MemRead]>:$source,
+                       Arg<AnyRefOfConstantSizeAggregateType, "", [MemWrite]>:$destination,
                        OptionalAttr<UnitAttr>:$no_overlap);
 
   let builders = [OpBuilder<(ins "mlir::Value":$source,
@@ -1375,8 +1373,7 @@ 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,
-                                            DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
+def fir_ArrayLoadOp : fir_Op<"array_load", [AttrSizedOperandSegments]> {
 
   let summary = "Load an array as a value.";
 
@@ -1415,7 +1412,7 @@ def fir_ArrayLoadOp : fir_Op<"array_load", [AttrSizedOperandSegments,
   }];
 
   let arguments = (ins
-    AnyRefOrBox:$memref,
+    Arg<AnyRefOrBox, "", [MemRead]>:$memref,
     Optional<AnyShapeOrShiftType>:$shape,
     Optional<fir_SliceType>:$slice,
     Variadic<AnyIntegerType>:$typeparams
@@ -1627,7 +1624,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 operations such as `array_amend` can be in
+    dominating source. Other array operation such as `array_amend` can be in
     between.
 
     TODO: The above restriction is not enforced. The design of the operation
@@ -1688,7 +1685,7 @@ def fir_ArrayAmendOp : fir_Op<"array_amend", [NoMemoryEffect]> {
 }
 
 def fir_ArrayMergeStoreOp : fir_Op<"array_merge_store",
-    [AttrSizedOperandSegments, DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
+    [AttrSizedOperandSegments]> {
 
   let summary = "Store merged array value to memory.";
 
@@ -1717,7 +1714,7 @@ def fir_ArrayMergeStoreOp : fir_Op<"array_merge_store",
   let arguments = (ins
     fir_SequenceType:$original,
     fir_SequenceType:$sequence,
-    AnyRefOrBox:$memref,
+    Arg<AnyRefOrBox, "", [MemWrite]>:$memref,
     Optional<fir_SliceType>:$slice,
     Variadic<AnyIntegerType>:$typeparams
   );
@@ -2755,22 +2752,6 @@ 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";
 
diff --git a/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h b/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
@@ -15,26 +15,13 @@
 
 namespace fir {
 
-/// 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 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;
 }
 
 /// Return true iff the Operation is a call.
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -415,13 +415,8 @@ class HlfirDesignatorBuilder {
         .Case<fir::SequenceType>([&](fir::SequenceType seqTy) -> mlir::Type {
           return fir::SequenceType::get(seqTy.getShape(), newEleTy);
         })
-        .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 {
+        .Case<fir::PointerType, fir::HeapType, fir::ReferenceType, fir::BoxType,
+              fir::ClassType>([&](auto t) -> mlir::Type {
           using FIRT = decltype(t);
           return FIRT::get(changeElementType(t.getEleTy(), newEleTy));
         })
diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -577,17 +577,6 @@ 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) {
@@ -750,20 +739,19 @@ 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, isVolatile);
+    boxTy = fir::BoxType::get(elementType);
     if (isPolymorphic) {
       elementType = fir::updateTypeForUnlimitedPolymorphic(elementType);
       if (isAssumedType)
-        boxTy = fir::BoxType::get(elementType, isVolatile);
+        boxTy = fir::BoxType::get(elementType);
       else
-        boxTy = fir::ClassType::get(elementType, isVolatile);
+        boxTy = fir::ClassType::get(elementType);
     }
   }
 
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
diff --git a/flang/test/Fir/cse.fir b/flang/test/Fir/cse.fir
diff --git a/flang/test/Fir/invalid.fir b/flang/test/Fir/invalid.fir
