Skip to content

Revert "[flang][nfc] Support volatility in Fir ops" #135034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions flang/include/flang/Optimizer/Builder/FIRBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions flang/include/flang/Optimizer/Dialect/FIROps.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand Down
43 changes: 12 additions & 31 deletions flang/include/flang/Optimizer/Dialect/FIROps.td
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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)>,
Expand All @@ -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 = [{
Expand All @@ -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)>];
Expand All @@ -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 = [{
Expand All @@ -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,
Expand Down Expand Up @@ -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.";

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.";

Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -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";

Expand Down
27 changes: 7 additions & 20 deletions flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 2 additions & 7 deletions flang/lib/Lower/ConvertExprToHLFIR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
})
Expand Down
18 changes: 3 additions & 15 deletions flang/lib/Optimizer/Builder/FIRBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}

Expand Down
Loading