diff --git a/flang/include/flang/Optimizer/Builder/FIRBuilder.h b/flang/include/flang/Optimizer/Builder/FIRBuilder.h index ddd4ef7114a63..8fca4272636b1 100644 --- a/flang/include/flang/Optimizer/Builder/FIRBuilder.h +++ b/flang/include/flang/Optimizer/Builder/FIRBuilder.h @@ -397,6 +397,11 @@ 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 index ed301016ad01c..f3dbf47351ab8 100644 --- a/flang/include/flang/Optimizer/Dialect/FIROps.h +++ b/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 { + mlir::StringRef getName() final { return "VolatileMemoryResource"; } +}; + class CoordinateIndicesAdaptor; using IntOrValue = llvm::PointerUnion; diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td index 753e4bd18dc6d..c7ac2c0397413 100644 --- a/flang/include/flang/Optimizer/Dialect/FIROps.td +++ b/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]> { 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:$memref, + let arguments = (ins AnyReferenceLike:$memref, OptionalAttr:$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]> { 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:$memref, + AnyReferenceLike:$memref, OptionalAttr:$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]> { 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:$source, - Arg:$destination, + let arguments = (ins AnyRefOfConstantSizeAggregateType:$source, + AnyRefOfConstantSizeAggregateType:$destination, OptionalAttr:$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 to -def fir_ArrayLoadOp : fir_Op<"array_load", [AttrSizedOperandSegments]> { +def fir_ArrayLoadOp : fir_Op<"array_load", [AttrSizedOperandSegments, + DeclareOpInterfaceMethods]> { let summary = "Load an array as a value."; @@ -1412,7 +1415,7 @@ def fir_ArrayLoadOp : fir_Op<"array_load", [AttrSizedOperandSegments]> { }]; let arguments = (ins - Arg:$memref, + AnyRefOrBox:$memref, Optional:$shape, Optional:$slice, Variadic:$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]> { 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:$memref, + AnyRefOrBox:$memref, Optional:$slice, Variadic:$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"; diff --git a/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h b/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h index f7f0a3067b318..b01d72199bf1d 100644 --- a/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h +++ b/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(op)) - return !load->getAttr("volatile"); - if (auto arrLoad = mlir::dyn_cast(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> + &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. diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index 09bc1babf079c..8f5d799ea4c57 100644 --- a/flang/lib/Lower/ConvertExprToHLFIR.cpp +++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp @@ -415,8 +415,13 @@ class HlfirDesignatorBuilder { .Case([&](fir::SequenceType seqTy) -> mlir::Type { return fir::SequenceType::get(seqTy.getShape(), newEleTy); }) - .Case([&](auto t) -> mlir::Type { + .Case( + [&](auto t) -> mlir::Type { + using FIRT = decltype(t); + return FIRT::get(changeElementType(t.getEleTy(), newEleTy), + t.isVolatile()); + }) + .Case([&](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 index 7fc30ca125a87..cfacb4b47854f 100644 --- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp +++ b/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(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(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); } } diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp index 2d8017d0318d2..5eaf6b7fc240d 100644 --- a/flang/lib/Optimizer/Dialect/FIROps.cpp +++ b/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 fir::ArrayLoadOp::getExtents() { return {}; } +void fir::ArrayLoadOp::getEffects( + llvm::SmallVectorImpl< + mlir::SideEffects::EffectInstance> + &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(eleTy); @@ -935,6 +945,15 @@ llvm::LogicalResult fir::ArrayMergeStoreOp::verify() { return mlir::success(); } +void fir::ArrayMergeStoreOp::getEffects( + llvm::SmallVectorImpl< + mlir::SideEffects::EffectInstance> + &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(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(fromType) + .Case( + [&](auto type) { + using TYPE = decltype(type); + return mlir::isa(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(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> + &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> + &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> + &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 //===----------------------------------------------------------------------===// diff --git a/flang/test/Fir/cse.fir b/flang/test/Fir/cse.fir index 8813b7c411f50..590a9681f7405 100644 --- a/flang/test/Fir/cse.fir +++ b/flang/test/Fir/cse.fir @@ -55,3 +55,23 @@ func.func @fun(%a : !fir.ref) -> 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 { + %0 = fir.load %arg0 : !fir.ref + %1 = fir.load %arg0 : !fir.ref + %2 = arith.addi %0, %1 : i64 + fir.store %2 to %arg0 : !fir.ref + fir.store %2 to %arg0 : !fir.ref + return %2 : i64 +} +// CHECK-LABEL: func.func @fun(%arg0: !fir.ref) -> i64 { +// CHECK: %[[VAL_1:.*]] = fir.load %arg0 : !fir.ref +// CHECK: %[[VAL_2:.*]] = fir.load %arg0 : !fir.ref +// CHECK: %[[VAL_3:.*]] = arith.addi %[[VAL_1]], %[[VAL_2]] : i64 +// CHECK: fir.store %[[VAL_3]] to %arg0 : !fir.ref +// CHECK: fir.store %[[VAL_3]] to %arg0 : !fir.ref +// CHECK: return %[[VAL_3]] : i64 +// CHECK: } diff --git a/flang/test/Fir/invalid.fir b/flang/test/Fir/invalid.fir index 88906890a9237..447a6c68b4b0a 100644 --- a/flang/test/Fir/invalid.fir +++ b/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) -> !fir.ref { + // 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) -> !fir.ref + return %0 : !fir.ref +} + +// ----- + +// Should fail when volatility changes from a fir.convert +func.func @bad_convert_volatile2(%arg0: !fir.ref) -> !fir.ref { + // 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) -> !fir.ref + return %0 : !fir.ref +} + +// ----- + +// Should fail when the element type and the containing type change +func.func @bad_convert_volatile3(%arg0: !fir.ref) -> !fir.box { + // expected-error@+1 {{'fir.volatile_cast' op types must be identical except for volatility}} + %0 = fir.volatile_cast %arg0 : (!fir.ref) -> !fir.box + return %0 : !fir.box +} + +// ----- + +// Should fail when the containing type changes +func.func @bad_convert_volatile4(%arg0: !fir.ref) -> !fir.box { + // expected-error@+1 {{'fir.volatile_cast' op types must be identical except for volatility}} + %0 = fir.volatile_cast %arg0 : (!fir.ref) -> !fir.box + return %0 : !fir.box +} + +// ----- + +// Should fail when the containing type changes +func.func @bad_convert_volatile5(%arg0: !fir.ref) -> !fir.box { + // expected-error@+1 {{'fir.volatile_cast' op types must be identical except for volatility}} + %0 = fir.volatile_cast %arg0 : (!fir.ref) -> !fir.box + return %0 : !fir.box +} + +// ----- + +// Should fail when the element type changes +func.func @bad_convert_volatile6(%arg0: !fir.ref) -> !fir.ref { + // expected-error@+1 {{'fir.volatile_cast' op types must be identical except for volatility}} + %0 = fir.volatile_cast %arg0 : (!fir.ref) -> !fir.ref + return %0 : !fir.ref +}