[flang] Add lowering of volatile references #132486
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-codegen Author: Asher Mancinelli (ashermancinelli) ChangesFlang currently lacks support for volatile variables. For some cases, the compiler produces TODO error messages and others are ignored. Some of our tests are like the example from C.4 Clause 8 notes: The VOLATILE attribute (8.5.20) and require volatile variables. This change is a minimal draft of support for volatility in Fortran. This PR does not include some important features, like support for volatility on boxes and other non-reference reference-like types. This commit only supports volatility for If this is the right direction, I'll break this up into a few chunks, add more tests, and share a smaller PR. Patch is 23.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132486.diff 15 Files Affected:
diff --git a/flang/include/flang/Optimizer/Builder/FIRBuilder.h b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
index 003b4358572c1..870709a5d55b6 100644
--- a/flang/include/flang/Optimizer/Builder/FIRBuilder.h
+++ b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
@@ -150,7 +150,7 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
mlir::Block *getAllocaBlock();
/// Safely create a reference type to the type `eleTy`.
- mlir::Type getRefType(mlir::Type eleTy);
+ mlir::Type getRefType(mlir::Type eleTy, bool isVolatile = false);
/// Create a sequence of `eleTy` with `rank` dimensions of unknown size.
mlir::Type getVarLenSeqTy(mlir::Type eleTy, unsigned rank = 1);
diff --git a/flang/include/flang/Optimizer/Dialect/FIRType.h b/flang/include/flang/Optimizer/Dialect/FIRType.h
index 76e0aa352bcd9..8261c67e4559d 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRType.h
+++ b/flang/include/flang/Optimizer/Dialect/FIRType.h
@@ -111,6 +111,12 @@ inline bool isa_ref_type(mlir::Type t) {
fir::LLVMPointerType>(t);
}
+inline bool isa_volatile_ref_type(mlir::Type t) {
+ if (auto refTy = mlir::dyn_cast_or_null<fir::ReferenceType>(t))
+ return refTy.isVolatile();
+ return false;
+}
+
/// Is `t` a boxed type?
inline bool isa_box_type(mlir::Type t) {
return mlir::isa<fir::BaseBoxType, fir::BoxCharType, fir::BoxProcType>(t);
diff --git a/flang/include/flang/Optimizer/Dialect/FIRTypes.td b/flang/include/flang/Optimizer/Dialect/FIRTypes.td
index fd5bbbe44751f..0584c175b36ff 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRTypes.td
+++ b/flang/include/flang/Optimizer/Dialect/FIRTypes.td
@@ -363,18 +363,22 @@ def fir_ReferenceType : FIR_Type<"Reference", "ref"> {
The type of a reference to an entity in memory.
}];
- let parameters = (ins "mlir::Type":$eleTy);
+ let parameters = (ins
+ "mlir::Type":$eleTy,
+ DefaultValuedParameter<"bool", "false">:$isVol);
let skipDefaultBuilders = 1;
let builders = [
- TypeBuilderWithInferredContext<(ins "mlir::Type":$elementType), [{
- return Base::get(elementType.getContext(), elementType);
+ TypeBuilderWithInferredContext<(ins "mlir::Type":$elementType, CArg<"bool", "false">:$isVol), [{
+ return Base::get(elementType.getContext(), elementType, isVol);
}]>,
];
let extraClassDeclaration = [{
mlir::Type getElementType() const { return getEleTy(); }
+ bool isVolatile() const { return (bool)getIsVol(); }
+ static llvm::StringRef getVolatileKeyword() { return "volatile"; }
}];
let genVerifyDecl = 1;
diff --git a/flang/lib/Lower/CallInterface.cpp b/flang/lib/Lower/CallInterface.cpp
index 226ba1e52c968..4ee28fbeb9a0c 100644
--- a/flang/lib/Lower/CallInterface.cpp
+++ b/flang/lib/Lower/CallInterface.cpp
@@ -1112,7 +1112,6 @@ class Fortran::lower::CallInterfaceImpl {
if (obj.attrs.test(Attrs::Value))
isValueAttr = true; // TODO: do we want an mlir::Attribute as well?
if (obj.attrs.test(Attrs::Volatile)) {
- TODO(loc, "VOLATILE in procedure interface");
addMLIRAttr(fir::getVolatileAttrName());
}
// obj.attrs.test(Attrs::Asynchronous) does not impact the way the argument
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index dc00e0b13f583..3ac10596df5ae 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -223,8 +223,37 @@ class HlfirDesignatorBuilder {
designatorNode, getConverter().getFoldingContext(),
/*namedConstantSectionsAreAlwaysContiguous=*/false))
return fir::BoxType::get(resultValueType);
+
+ bool isVolatile = false;
+
+ // Check if the base type is volatile
+ if (partInfo.base.has_value()) {
+ mlir::Type baseType = partInfo.base.value().getType();
+ isVolatile = fir::isa_volatile_ref_type(baseType);
+ }
+
+ auto isVolatileSymbol = [](const Fortran::semantics::Symbol &symbol) {
+ return symbol.GetUltimate().attrs().test(
+ Fortran::semantics::Attr::VOLATILE);
+ };
+
+ // Check if this should be a volatile reference
+ if constexpr (std::is_same_v<std::decay_t<T>,
+ Fortran::evaluate::SymbolRef>) {
+ if (isVolatileSymbol(designatorNode.get()))
+ isVolatile = true;
+ } else if constexpr (std::is_same_v<std::decay_t<T>,
+ Fortran::evaluate::Component>) {
+ if (isVolatileSymbol(designatorNode.GetLastSymbol()))
+ isVolatile = true;
+ }
+
+ // If it's a reference to a ref, account for it
+ if (auto refTy = mlir::dyn_cast<fir::ReferenceType>(resultValueType))
+ resultValueType = refTy.getEleTy();
+
// Other designators can be handled as raw addresses.
- return fir::ReferenceType::get(resultValueType);
+ return fir::ReferenceType::get(resultValueType, isVolatile);
}
template <typename T>
@@ -414,10 +443,16 @@ class HlfirDesignatorBuilder {
.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 {
- using FIRT = decltype(t);
- return FIRT::get(changeElementType(t.getEleTy(), newEleTy));
+ // TODO: handle volatility for other types
+ .Case<fir::PointerType, fir::HeapType, fir::BoxType, fir::ClassType>(
+ [&](auto t) -> mlir::Type {
+ using FIRT = decltype(t);
+ return FIRT::get(changeElementType(t.getEleTy(), newEleTy));
+ })
+ .Case<fir::ReferenceType>([&](fir::ReferenceType refTy) -> mlir::Type {
+ return fir::ReferenceType::get(
+ changeElementType(refTy.getEleTy(), newEleTy),
+ refTy.isVolatile());
})
.Default([newEleTy](mlir::Type t) -> mlir::Type { return newEleTy; });
}
@@ -1808,6 +1843,7 @@ class HlfirBuilder {
auto &expr = std::get<const Fortran::lower::SomeExpr &>(iter);
auto &baseOp = std::get<hlfir::EntityWithAttributes>(iter);
std::string name = converter.getRecordTypeFieldName(sym);
+ const bool isVolatile = fir::isa_volatile_ref_type(baseOp.getType());
// Generate DesignateOp for the component.
// The designator's result type is just a reference to the component type,
@@ -1818,7 +1854,7 @@ class HlfirBuilder {
assert(compType && "failed to retrieve component type");
mlir::Value compShape =
designatorBuilder.genComponentShape(sym, compType);
- mlir::Type designatorType = builder.getRefType(compType);
+ mlir::Type designatorType = builder.getRefType(compType, isVolatile);
mlir::Type fieldElemType = hlfir::getFortranElementType(compType);
llvm::SmallVector<mlir::Value, 1> typeParams;
diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index b7f8a8d3a9d56..02ded29606885 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -104,9 +104,9 @@ fir::FirOpBuilder::getNamedGlobal(mlir::ModuleOp modOp,
return modOp.lookupSymbol<fir::GlobalOp>(name);
}
-mlir::Type fir::FirOpBuilder::getRefType(mlir::Type eleTy) {
+mlir::Type fir::FirOpBuilder::getRefType(mlir::Type eleTy, bool isVolatile) {
assert(!mlir::isa<fir::ReferenceType>(eleTy) && "cannot be a reference type");
- return fir::ReferenceType::get(eleTy);
+ return fir::ReferenceType::get(eleTy, isVolatile);
}
mlir::Type fir::FirOpBuilder::getVarLenSeqTy(mlir::Type eleTy, unsigned rank) {
diff --git a/flang/lib/Optimizer/Builder/HLFIRTools.cpp b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
index 85fd742db6beb..aec88ec97b514 100644
--- a/flang/lib/Optimizer/Builder/HLFIRTools.cpp
+++ b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
@@ -819,7 +819,8 @@ mlir::Type hlfir::getVariableElementType(hlfir::Entity variable) {
} else if (fir::isRecordWithTypeParameters(eleTy)) {
return fir::BoxType::get(eleTy);
}
- return fir::ReferenceType::get(eleTy);
+ const bool isVolatile = fir::isa_volatile_ref_type(variable.getType());
+ return fir::ReferenceType::get(eleTy, isVolatile);
}
mlir::Type hlfir::getEntityElementType(hlfir::Entity entity) {
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index b54b497ee4ba1..90f2474dafca3 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -3224,6 +3224,8 @@ struct LoadOpConversion : public fir::FIROpConversion<fir::LoadOp> {
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Type llvmLoadTy = convertObjectType(load.getType());
+ const bool isVolatile =
+ fir::isa_volatile_ref_type(load.getMemref().getType());
if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(load.getType())) {
// fir.box is a special case because it is considered an ssa value in
// fir, but it is lowered as a pointer to a descriptor. So
@@ -3253,7 +3255,7 @@ struct LoadOpConversion : public fir::FIROpConversion<fir::LoadOp> {
mlir::Value boxSize =
computeBoxSize(loc, boxTypePair, inputBoxStorage, rewriter);
auto memcpy = rewriter.create<mlir::LLVM::MemcpyOp>(
- loc, newBoxStorage, inputBoxStorage, boxSize, /*isVolatile=*/false);
+ loc, newBoxStorage, inputBoxStorage, boxSize, isVolatile);
if (std::optional<mlir::ArrayAttr> optionalTag = load.getTbaa())
memcpy.setTBAATags(*optionalTag);
@@ -3261,8 +3263,10 @@ struct LoadOpConversion : public fir::FIROpConversion<fir::LoadOp> {
attachTBAATag(memcpy, boxTy, boxTy, nullptr);
rewriter.replaceOp(load, newBoxStorage);
} else {
+ // TODO: are we losing any attributes from the load op?
+ auto memref = adaptor.getOperands()[0];
auto loadOp = rewriter.create<mlir::LLVM::LoadOp>(
- load.getLoc(), llvmLoadTy, adaptor.getOperands(), load->getAttrs());
+ load.getLoc(), llvmLoadTy, memref, /*alignment=*/0, isVolatile);
if (std::optional<mlir::ArrayAttr> optionalTag = load.getTbaa())
loadOp.setTBAATags(*optionalTag);
else
@@ -3540,6 +3544,8 @@ struct StoreOpConversion : public fir::FIROpConversion<fir::StoreOp> {
mlir::Value llvmValue = adaptor.getValue();
mlir::Value llvmMemref = adaptor.getMemref();
mlir::LLVM::AliasAnalysisOpInterface newOp;
+ const bool isVolatile =
+ fir::isa_volatile_ref_type(store.getMemref().getType());
if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(storeTy)) {
mlir::Type llvmBoxTy = lowerTy().convertBoxTypeAsStruct(boxTy);
// Always use memcpy because LLVM is not as effective at optimizing
@@ -3547,10 +3553,11 @@ struct StoreOpConversion : public fir::FIROpConversion<fir::StoreOp> {
TypePair boxTypePair{boxTy, llvmBoxTy};
mlir::Value boxSize =
computeBoxSize(loc, boxTypePair, llvmValue, rewriter);
- newOp = rewriter.create<mlir::LLVM::MemcpyOp>(
- loc, llvmMemref, llvmValue, boxSize, /*isVolatile=*/false);
+ newOp = rewriter.create<mlir::LLVM::MemcpyOp>(loc, llvmMemref, llvmValue,
+ boxSize, isVolatile);
} else {
- newOp = rewriter.create<mlir::LLVM::StoreOp>(loc, llvmValue, llvmMemref);
+ newOp = rewriter.create<mlir::LLVM::StoreOp>(loc, llvmValue, llvmMemref,
+ /*alignment=*/0, isVolatile);
}
if (std::optional<mlir::ArrayAttr> optionalTag = store.getTbaa())
newOp.setTBAATags(*optionalTag);
diff --git a/flang/lib/Optimizer/Dialect/FIRType.cpp b/flang/lib/Optimizer/Dialect/FIRType.cpp
index f3f969ba401e5..90942522d9073 100644
--- a/flang/lib/Optimizer/Dialect/FIRType.cpp
+++ b/flang/lib/Optimizer/Dialect/FIRType.cpp
@@ -649,12 +649,17 @@ mlir::Type changeElementType(mlir::Type type, mlir::Type newElementType,
.Case<fir::SequenceType>([&](fir::SequenceType seqTy) -> mlir::Type {
return fir::SequenceType::get(seqTy.getShape(), newElementType);
})
- .Case<fir::PointerType, fir::HeapType, fir::ReferenceType,
- fir::ClassType>([&](auto t) -> mlir::Type {
- using FIRT = decltype(t);
- return FIRT::get(
- changeElementType(t.getEleTy(), newElementType, turnBoxIntoClass));
+ .Case<fir::ReferenceType>([&](fir::ReferenceType refTy) -> mlir::Type {
+ auto newEleTy = changeElementType(refTy.getEleTy(), newElementType,
+ turnBoxIntoClass);
+ return fir::ReferenceType::get(newEleTy, refTy.isVolatile());
})
+ .Case<fir::PointerType, fir::HeapType, fir::ClassType>(
+ [&](auto t) -> mlir::Type {
+ using FIRT = decltype(t);
+ return FIRT::get(changeElementType(t.getEleTy(), newElementType,
+ turnBoxIntoClass));
+ })
.Case<fir::BoxType>([&](fir::BoxType t) -> mlir::Type {
mlir::Type newInnerType =
changeElementType(t.getEleTy(), newElementType, false);
@@ -1057,18 +1062,38 @@ unsigned fir::RecordType::getFieldIndex(llvm::StringRef ident) {
// ReferenceType
//===----------------------------------------------------------------------===//
-// `ref` `<` type `>`
+// `ref` `<` type (`, volatile` $volatile^)? `>`
mlir::Type fir::ReferenceType::parse(mlir::AsmParser &parser) {
- return parseTypeSingleton<fir::ReferenceType>(parser);
+ if (parser.parseLess())
+ return {};
+
+ mlir::Type eleTy;
+ if (parser.parseType(eleTy))
+ return {};
+
+ bool isVolatile = false;
+ if (!parser.parseOptionalComma()) {
+ if (parser.parseKeyword(getVolatileKeyword())) {
+ return {};
+ }
+ isVolatile = true;
+ }
+
+ if (parser.parseGreater())
+ return {};
+ return get(eleTy, isVolatile);
}
void fir::ReferenceType::print(mlir::AsmPrinter &printer) const {
- printer << "<" << getEleTy() << '>';
+ printer << "<" << getEleTy();
+ if (isVolatile())
+ printer << ", " << getVolatileKeyword();
+ printer << '>';
}
llvm::LogicalResult fir::ReferenceType::verify(
- llvm::function_ref<mlir::InFlightDiagnostic()> emitError,
- mlir::Type eleTy) {
+ llvm::function_ref<mlir::InFlightDiagnostic()> emitError, mlir::Type eleTy,
+ bool isVolatile) {
if (mlir::isa<ShapeType, ShapeShiftType, SliceType, FieldType, LenType,
ReferenceType, TypeDescType>(eleTy))
return emitError() << "cannot build a reference to type: " << eleTy << '\n';
@@ -1319,11 +1344,15 @@ changeTypeShape(mlir::Type type,
return fir::SequenceType::get(*newShape, seqTy.getEleTy());
return seqTy.getEleTy();
})
- .Case<fir::PointerType, fir::HeapType, fir::ReferenceType, fir::BoxType,
- fir::ClassType>([&](auto t) -> mlir::Type {
- using FIRT = decltype(t);
- return FIRT::get(changeTypeShape(t.getEleTy(), newShape));
+ .Case<fir::ReferenceType>([&](fir::ReferenceType rt) -> mlir::Type {
+ return fir::ReferenceType::get(changeTypeShape(rt.getEleTy(), newShape),
+ rt.isVolatile());
})
+ .Case<fir::PointerType, fir::HeapType, fir::BoxType, fir::ClassType>(
+ [&](auto t) -> mlir::Type {
+ using FIRT = decltype(t);
+ return FIRT::get(changeTypeShape(t.getEleTy(), newShape));
+ })
.Default([&](mlir::Type t) -> mlir::Type {
assert((fir::isa_trivial(t) || llvm::isa<fir::RecordType>(t) ||
llvm::isa<mlir::NoneType>(t) ||
diff --git a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
index 8851a3a7187b9..4a3308ff4e747 100644
--- a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
+++ b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
@@ -214,6 +214,13 @@ void hlfir::DeclareOp::build(mlir::OpBuilder &builder,
auto nameAttr = builder.getStringAttr(uniq_name);
mlir::Type inputType = memref.getType();
bool hasExplicitLbs = hasExplicitLowerBounds(shape);
+ if (fortran_attrs && mlir::isa<fir::ReferenceType>(inputType) &&
+ bitEnumContainsAny(fortran_attrs.getFlags(),
+ fir::FortranVariableFlagsEnum::fortran_volatile)) {
+ auto refType = mlir::cast<fir::ReferenceType>(inputType);
+ inputType = fir::ReferenceType::get(refType.getEleTy(), true);
+ memref = builder.create<fir::ConvertOp>(memref.getLoc(), inputType, memref);
+ }
mlir::Type hlfirVariableType =
getHLFIRVariableType(inputType, hasExplicitLbs);
build(builder, result, {hlfirVariableType, inputType}, memref, shape,
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
index 496a5560ac615..aa151f90ed0d1 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
@@ -418,7 +418,9 @@ class DesignateOpConversion
firstElementIndices.push_back(indices[i]);
i = i + (isTriplet ? 3 : 1);
}
- mlir::Type arrayCoorType = fir::ReferenceType::get(baseEleTy);
+ mlir::Type originalDesignateType = designate.getResult().getType();
+ const bool isVolatile = fir::isa_volatile_ref_type(originalDesignateType);
+ mlir::Type arrayCoorType = fir::ReferenceType::get(baseEleTy, isVolatile);
base = builder.create<fir::ArrayCoorOp>(
loc, arrayCoorType, base, shape,
/*slice=*/mlir::Value{}, firstElementIndices, firBaseTypeParameters);
@@ -441,6 +443,7 @@ class DesignateOpConversion
TODO(loc, "hlfir::designate load of pointer or allocatable");
mlir::Type designateResultType = designate.getResult().getType();
+ const bool isVolatile = fir::isa_volatile_ref_type(designateResultType);
llvm::SmallVector<mlir::Value> firBaseTypeParameters;
auto [base, shape] = hlfir::genVariableFirBaseShapeAndParams(
loc, builder, baseEntity, firBaseTypeParameters);
@@ -464,7 +467,7 @@ class DesignateOpConversion
mlir::Type componentType =
mlir::cast<fir::RecordType>(baseEleTy).getType(
designate.getComponent().value());
- mlir::Type coorTy = fir::ReferenceType::get(componentType);
+ mlir::Type coorTy = fir::ReferenceType::get(componentType, isVolatile);
base = builder.create<fir::CoordinateOp>(loc, coorTy, base, fieldIndex);
if (mlir::isa<fir::BaseBoxType>(componentType)) {
auto variableInterface = mlir::cast<fir::FortranVariableOpInterface>(
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
index 96a3622f4afee..020915179a670 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
@@ -1126,7 +1126,8 @@ class ReductionMaskConversion : public mlir::OpRewritePattern<Op> {
builder.create<fir::StoreOp>(loc, flagSet, flagRef);
mlir::Type resultElemTy =
hlfir::getFortranElementType(resultArr.getType());
- mlir::Type returnRefTy = builder.getRefType(resultElemTy);
+ mlir::Type returnRefTy = builder.getRefType(
+ resultElemTy, fir::isa_volatile_ref_type(flagRef.getType()));
mlir::IndexType idxTy = builder.getIndexType();
for (unsigned int i = 0; i < rank; ++i) {
@@ -1153,7 +1154,8 @@ class ReductionMaskConversion : public mlir::OpRewritePattern<Op> {
auto getAddrFn = [](fir::FirOpBuilder builder, mlir::Location loc,
const mlir::Type &resultElemType, mlir::Value resultArr,
mlir::Value index) {
- mlir::Type resultRefTy = builder.getRefType(resultElemType);
+ mlir::Type resultRefTy = builder.getRefType(
+ resultElemType, fir::isa_volatile_ref_type(resultArr.getType()));
mlir::Value oneIdx =
builder.createIntegerConstant(loc, builder.getIndexType(), 1);
index = builder.create<mlir::arith::AddIOp>(loc, index, oneIdx);
@@ -1162,8 +1164,9 @@ class ReductionMaskConversion : public mlir::OpRewritePattern<Op> {
};
// Initialize the resu...
[truncated]
|
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Asher Mancinelli (ashermancinelli) ChangesFlang currently lacks support for volatile variables. For some cases, the compiler produces TODO error messages and others are ignored. Some of our tests are like the example from C.4 Clause 8 notes: The VOLATILE attribute (8.5.20) and require volatile variables. This change is a minimal draft of support for volatility in Fortran. This PR does not include some important features, like support for volatility on boxes and other non-reference reference-like types. This commit only supports volatility for If this is the right direction, I'll break this up into a few chunks, add more tests, and share a smaller PR. Patch is 23.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132486.diff 15 Files Affected:
diff --git a/flang/include/flang/Optimizer/Builder/FIRBuilder.h b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
index 003b4358572c1..870709a5d55b6 100644
--- a/flang/include/flang/Optimizer/Builder/FIRBuilder.h
+++ b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
@@ -150,7 +150,7 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
mlir::Block *getAllocaBlock();
/// Safely create a reference type to the type `eleTy`.
- mlir::Type getRefType(mlir::Type eleTy);
+ mlir::Type getRefType(mlir::Type eleTy, bool isVolatile = false);
/// Create a sequence of `eleTy` with `rank` dimensions of unknown size.
mlir::Type getVarLenSeqTy(mlir::Type eleTy, unsigned rank = 1);
diff --git a/flang/include/flang/Optimizer/Dialect/FIRType.h b/flang/include/flang/Optimizer/Dialect/FIRType.h
index 76e0aa352bcd9..8261c67e4559d 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRType.h
+++ b/flang/include/flang/Optimizer/Dialect/FIRType.h
@@ -111,6 +111,12 @@ inline bool isa_ref_type(mlir::Type t) {
fir::LLVMPointerType>(t);
}
+inline bool isa_volatile_ref_type(mlir::Type t) {
+ if (auto refTy = mlir::dyn_cast_or_null<fir::ReferenceType>(t))
+ return refTy.isVolatile();
+ return false;
+}
+
/// Is `t` a boxed type?
inline bool isa_box_type(mlir::Type t) {
return mlir::isa<fir::BaseBoxType, fir::BoxCharType, fir::BoxProcType>(t);
diff --git a/flang/include/flang/Optimizer/Dialect/FIRTypes.td b/flang/include/flang/Optimizer/Dialect/FIRTypes.td
index fd5bbbe44751f..0584c175b36ff 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRTypes.td
+++ b/flang/include/flang/Optimizer/Dialect/FIRTypes.td
@@ -363,18 +363,22 @@ def fir_ReferenceType : FIR_Type<"Reference", "ref"> {
The type of a reference to an entity in memory.
}];
- let parameters = (ins "mlir::Type":$eleTy);
+ let parameters = (ins
+ "mlir::Type":$eleTy,
+ DefaultValuedParameter<"bool", "false">:$isVol);
let skipDefaultBuilders = 1;
let builders = [
- TypeBuilderWithInferredContext<(ins "mlir::Type":$elementType), [{
- return Base::get(elementType.getContext(), elementType);
+ TypeBuilderWithInferredContext<(ins "mlir::Type":$elementType, CArg<"bool", "false">:$isVol), [{
+ return Base::get(elementType.getContext(), elementType, isVol);
}]>,
];
let extraClassDeclaration = [{
mlir::Type getElementType() const { return getEleTy(); }
+ bool isVolatile() const { return (bool)getIsVol(); }
+ static llvm::StringRef getVolatileKeyword() { return "volatile"; }
}];
let genVerifyDecl = 1;
diff --git a/flang/lib/Lower/CallInterface.cpp b/flang/lib/Lower/CallInterface.cpp
index 226ba1e52c968..4ee28fbeb9a0c 100644
--- a/flang/lib/Lower/CallInterface.cpp
+++ b/flang/lib/Lower/CallInterface.cpp
@@ -1112,7 +1112,6 @@ class Fortran::lower::CallInterfaceImpl {
if (obj.attrs.test(Attrs::Value))
isValueAttr = true; // TODO: do we want an mlir::Attribute as well?
if (obj.attrs.test(Attrs::Volatile)) {
- TODO(loc, "VOLATILE in procedure interface");
addMLIRAttr(fir::getVolatileAttrName());
}
// obj.attrs.test(Attrs::Asynchronous) does not impact the way the argument
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index dc00e0b13f583..3ac10596df5ae 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -223,8 +223,37 @@ class HlfirDesignatorBuilder {
designatorNode, getConverter().getFoldingContext(),
/*namedConstantSectionsAreAlwaysContiguous=*/false))
return fir::BoxType::get(resultValueType);
+
+ bool isVolatile = false;
+
+ // Check if the base type is volatile
+ if (partInfo.base.has_value()) {
+ mlir::Type baseType = partInfo.base.value().getType();
+ isVolatile = fir::isa_volatile_ref_type(baseType);
+ }
+
+ auto isVolatileSymbol = [](const Fortran::semantics::Symbol &symbol) {
+ return symbol.GetUltimate().attrs().test(
+ Fortran::semantics::Attr::VOLATILE);
+ };
+
+ // Check if this should be a volatile reference
+ if constexpr (std::is_same_v<std::decay_t<T>,
+ Fortran::evaluate::SymbolRef>) {
+ if (isVolatileSymbol(designatorNode.get()))
+ isVolatile = true;
+ } else if constexpr (std::is_same_v<std::decay_t<T>,
+ Fortran::evaluate::Component>) {
+ if (isVolatileSymbol(designatorNode.GetLastSymbol()))
+ isVolatile = true;
+ }
+
+ // If it's a reference to a ref, account for it
+ if (auto refTy = mlir::dyn_cast<fir::ReferenceType>(resultValueType))
+ resultValueType = refTy.getEleTy();
+
// Other designators can be handled as raw addresses.
- return fir::ReferenceType::get(resultValueType);
+ return fir::ReferenceType::get(resultValueType, isVolatile);
}
template <typename T>
@@ -414,10 +443,16 @@ class HlfirDesignatorBuilder {
.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 {
- using FIRT = decltype(t);
- return FIRT::get(changeElementType(t.getEleTy(), newEleTy));
+ // TODO: handle volatility for other types
+ .Case<fir::PointerType, fir::HeapType, fir::BoxType, fir::ClassType>(
+ [&](auto t) -> mlir::Type {
+ using FIRT = decltype(t);
+ return FIRT::get(changeElementType(t.getEleTy(), newEleTy));
+ })
+ .Case<fir::ReferenceType>([&](fir::ReferenceType refTy) -> mlir::Type {
+ return fir::ReferenceType::get(
+ changeElementType(refTy.getEleTy(), newEleTy),
+ refTy.isVolatile());
})
.Default([newEleTy](mlir::Type t) -> mlir::Type { return newEleTy; });
}
@@ -1808,6 +1843,7 @@ class HlfirBuilder {
auto &expr = std::get<const Fortran::lower::SomeExpr &>(iter);
auto &baseOp = std::get<hlfir::EntityWithAttributes>(iter);
std::string name = converter.getRecordTypeFieldName(sym);
+ const bool isVolatile = fir::isa_volatile_ref_type(baseOp.getType());
// Generate DesignateOp for the component.
// The designator's result type is just a reference to the component type,
@@ -1818,7 +1854,7 @@ class HlfirBuilder {
assert(compType && "failed to retrieve component type");
mlir::Value compShape =
designatorBuilder.genComponentShape(sym, compType);
- mlir::Type designatorType = builder.getRefType(compType);
+ mlir::Type designatorType = builder.getRefType(compType, isVolatile);
mlir::Type fieldElemType = hlfir::getFortranElementType(compType);
llvm::SmallVector<mlir::Value, 1> typeParams;
diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index b7f8a8d3a9d56..02ded29606885 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -104,9 +104,9 @@ fir::FirOpBuilder::getNamedGlobal(mlir::ModuleOp modOp,
return modOp.lookupSymbol<fir::GlobalOp>(name);
}
-mlir::Type fir::FirOpBuilder::getRefType(mlir::Type eleTy) {
+mlir::Type fir::FirOpBuilder::getRefType(mlir::Type eleTy, bool isVolatile) {
assert(!mlir::isa<fir::ReferenceType>(eleTy) && "cannot be a reference type");
- return fir::ReferenceType::get(eleTy);
+ return fir::ReferenceType::get(eleTy, isVolatile);
}
mlir::Type fir::FirOpBuilder::getVarLenSeqTy(mlir::Type eleTy, unsigned rank) {
diff --git a/flang/lib/Optimizer/Builder/HLFIRTools.cpp b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
index 85fd742db6beb..aec88ec97b514 100644
--- a/flang/lib/Optimizer/Builder/HLFIRTools.cpp
+++ b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
@@ -819,7 +819,8 @@ mlir::Type hlfir::getVariableElementType(hlfir::Entity variable) {
} else if (fir::isRecordWithTypeParameters(eleTy)) {
return fir::BoxType::get(eleTy);
}
- return fir::ReferenceType::get(eleTy);
+ const bool isVolatile = fir::isa_volatile_ref_type(variable.getType());
+ return fir::ReferenceType::get(eleTy, isVolatile);
}
mlir::Type hlfir::getEntityElementType(hlfir::Entity entity) {
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index b54b497ee4ba1..90f2474dafca3 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -3224,6 +3224,8 @@ struct LoadOpConversion : public fir::FIROpConversion<fir::LoadOp> {
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Type llvmLoadTy = convertObjectType(load.getType());
+ const bool isVolatile =
+ fir::isa_volatile_ref_type(load.getMemref().getType());
if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(load.getType())) {
// fir.box is a special case because it is considered an ssa value in
// fir, but it is lowered as a pointer to a descriptor. So
@@ -3253,7 +3255,7 @@ struct LoadOpConversion : public fir::FIROpConversion<fir::LoadOp> {
mlir::Value boxSize =
computeBoxSize(loc, boxTypePair, inputBoxStorage, rewriter);
auto memcpy = rewriter.create<mlir::LLVM::MemcpyOp>(
- loc, newBoxStorage, inputBoxStorage, boxSize, /*isVolatile=*/false);
+ loc, newBoxStorage, inputBoxStorage, boxSize, isVolatile);
if (std::optional<mlir::ArrayAttr> optionalTag = load.getTbaa())
memcpy.setTBAATags(*optionalTag);
@@ -3261,8 +3263,10 @@ struct LoadOpConversion : public fir::FIROpConversion<fir::LoadOp> {
attachTBAATag(memcpy, boxTy, boxTy, nullptr);
rewriter.replaceOp(load, newBoxStorage);
} else {
+ // TODO: are we losing any attributes from the load op?
+ auto memref = adaptor.getOperands()[0];
auto loadOp = rewriter.create<mlir::LLVM::LoadOp>(
- load.getLoc(), llvmLoadTy, adaptor.getOperands(), load->getAttrs());
+ load.getLoc(), llvmLoadTy, memref, /*alignment=*/0, isVolatile);
if (std::optional<mlir::ArrayAttr> optionalTag = load.getTbaa())
loadOp.setTBAATags(*optionalTag);
else
@@ -3540,6 +3544,8 @@ struct StoreOpConversion : public fir::FIROpConversion<fir::StoreOp> {
mlir::Value llvmValue = adaptor.getValue();
mlir::Value llvmMemref = adaptor.getMemref();
mlir::LLVM::AliasAnalysisOpInterface newOp;
+ const bool isVolatile =
+ fir::isa_volatile_ref_type(store.getMemref().getType());
if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(storeTy)) {
mlir::Type llvmBoxTy = lowerTy().convertBoxTypeAsStruct(boxTy);
// Always use memcpy because LLVM is not as effective at optimizing
@@ -3547,10 +3553,11 @@ struct StoreOpConversion : public fir::FIROpConversion<fir::StoreOp> {
TypePair boxTypePair{boxTy, llvmBoxTy};
mlir::Value boxSize =
computeBoxSize(loc, boxTypePair, llvmValue, rewriter);
- newOp = rewriter.create<mlir::LLVM::MemcpyOp>(
- loc, llvmMemref, llvmValue, boxSize, /*isVolatile=*/false);
+ newOp = rewriter.create<mlir::LLVM::MemcpyOp>(loc, llvmMemref, llvmValue,
+ boxSize, isVolatile);
} else {
- newOp = rewriter.create<mlir::LLVM::StoreOp>(loc, llvmValue, llvmMemref);
+ newOp = rewriter.create<mlir::LLVM::StoreOp>(loc, llvmValue, llvmMemref,
+ /*alignment=*/0, isVolatile);
}
if (std::optional<mlir::ArrayAttr> optionalTag = store.getTbaa())
newOp.setTBAATags(*optionalTag);
diff --git a/flang/lib/Optimizer/Dialect/FIRType.cpp b/flang/lib/Optimizer/Dialect/FIRType.cpp
index f3f969ba401e5..90942522d9073 100644
--- a/flang/lib/Optimizer/Dialect/FIRType.cpp
+++ b/flang/lib/Optimizer/Dialect/FIRType.cpp
@@ -649,12 +649,17 @@ mlir::Type changeElementType(mlir::Type type, mlir::Type newElementType,
.Case<fir::SequenceType>([&](fir::SequenceType seqTy) -> mlir::Type {
return fir::SequenceType::get(seqTy.getShape(), newElementType);
})
- .Case<fir::PointerType, fir::HeapType, fir::ReferenceType,
- fir::ClassType>([&](auto t) -> mlir::Type {
- using FIRT = decltype(t);
- return FIRT::get(
- changeElementType(t.getEleTy(), newElementType, turnBoxIntoClass));
+ .Case<fir::ReferenceType>([&](fir::ReferenceType refTy) -> mlir::Type {
+ auto newEleTy = changeElementType(refTy.getEleTy(), newElementType,
+ turnBoxIntoClass);
+ return fir::ReferenceType::get(newEleTy, refTy.isVolatile());
})
+ .Case<fir::PointerType, fir::HeapType, fir::ClassType>(
+ [&](auto t) -> mlir::Type {
+ using FIRT = decltype(t);
+ return FIRT::get(changeElementType(t.getEleTy(), newElementType,
+ turnBoxIntoClass));
+ })
.Case<fir::BoxType>([&](fir::BoxType t) -> mlir::Type {
mlir::Type newInnerType =
changeElementType(t.getEleTy(), newElementType, false);
@@ -1057,18 +1062,38 @@ unsigned fir::RecordType::getFieldIndex(llvm::StringRef ident) {
// ReferenceType
//===----------------------------------------------------------------------===//
-// `ref` `<` type `>`
+// `ref` `<` type (`, volatile` $volatile^)? `>`
mlir::Type fir::ReferenceType::parse(mlir::AsmParser &parser) {
- return parseTypeSingleton<fir::ReferenceType>(parser);
+ if (parser.parseLess())
+ return {};
+
+ mlir::Type eleTy;
+ if (parser.parseType(eleTy))
+ return {};
+
+ bool isVolatile = false;
+ if (!parser.parseOptionalComma()) {
+ if (parser.parseKeyword(getVolatileKeyword())) {
+ return {};
+ }
+ isVolatile = true;
+ }
+
+ if (parser.parseGreater())
+ return {};
+ return get(eleTy, isVolatile);
}
void fir::ReferenceType::print(mlir::AsmPrinter &printer) const {
- printer << "<" << getEleTy() << '>';
+ printer << "<" << getEleTy();
+ if (isVolatile())
+ printer << ", " << getVolatileKeyword();
+ printer << '>';
}
llvm::LogicalResult fir::ReferenceType::verify(
- llvm::function_ref<mlir::InFlightDiagnostic()> emitError,
- mlir::Type eleTy) {
+ llvm::function_ref<mlir::InFlightDiagnostic()> emitError, mlir::Type eleTy,
+ bool isVolatile) {
if (mlir::isa<ShapeType, ShapeShiftType, SliceType, FieldType, LenType,
ReferenceType, TypeDescType>(eleTy))
return emitError() << "cannot build a reference to type: " << eleTy << '\n';
@@ -1319,11 +1344,15 @@ changeTypeShape(mlir::Type type,
return fir::SequenceType::get(*newShape, seqTy.getEleTy());
return seqTy.getEleTy();
})
- .Case<fir::PointerType, fir::HeapType, fir::ReferenceType, fir::BoxType,
- fir::ClassType>([&](auto t) -> mlir::Type {
- using FIRT = decltype(t);
- return FIRT::get(changeTypeShape(t.getEleTy(), newShape));
+ .Case<fir::ReferenceType>([&](fir::ReferenceType rt) -> mlir::Type {
+ return fir::ReferenceType::get(changeTypeShape(rt.getEleTy(), newShape),
+ rt.isVolatile());
})
+ .Case<fir::PointerType, fir::HeapType, fir::BoxType, fir::ClassType>(
+ [&](auto t) -> mlir::Type {
+ using FIRT = decltype(t);
+ return FIRT::get(changeTypeShape(t.getEleTy(), newShape));
+ })
.Default([&](mlir::Type t) -> mlir::Type {
assert((fir::isa_trivial(t) || llvm::isa<fir::RecordType>(t) ||
llvm::isa<mlir::NoneType>(t) ||
diff --git a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
index 8851a3a7187b9..4a3308ff4e747 100644
--- a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
+++ b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
@@ -214,6 +214,13 @@ void hlfir::DeclareOp::build(mlir::OpBuilder &builder,
auto nameAttr = builder.getStringAttr(uniq_name);
mlir::Type inputType = memref.getType();
bool hasExplicitLbs = hasExplicitLowerBounds(shape);
+ if (fortran_attrs && mlir::isa<fir::ReferenceType>(inputType) &&
+ bitEnumContainsAny(fortran_attrs.getFlags(),
+ fir::FortranVariableFlagsEnum::fortran_volatile)) {
+ auto refType = mlir::cast<fir::ReferenceType>(inputType);
+ inputType = fir::ReferenceType::get(refType.getEleTy(), true);
+ memref = builder.create<fir::ConvertOp>(memref.getLoc(), inputType, memref);
+ }
mlir::Type hlfirVariableType =
getHLFIRVariableType(inputType, hasExplicitLbs);
build(builder, result, {hlfirVariableType, inputType}, memref, shape,
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
index 496a5560ac615..aa151f90ed0d1 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
@@ -418,7 +418,9 @@ class DesignateOpConversion
firstElementIndices.push_back(indices[i]);
i = i + (isTriplet ? 3 : 1);
}
- mlir::Type arrayCoorType = fir::ReferenceType::get(baseEleTy);
+ mlir::Type originalDesignateType = designate.getResult().getType();
+ const bool isVolatile = fir::isa_volatile_ref_type(originalDesignateType);
+ mlir::Type arrayCoorType = fir::ReferenceType::get(baseEleTy, isVolatile);
base = builder.create<fir::ArrayCoorOp>(
loc, arrayCoorType, base, shape,
/*slice=*/mlir::Value{}, firstElementIndices, firBaseTypeParameters);
@@ -441,6 +443,7 @@ class DesignateOpConversion
TODO(loc, "hlfir::designate load of pointer or allocatable");
mlir::Type designateResultType = designate.getResult().getType();
+ const bool isVolatile = fir::isa_volatile_ref_type(designateResultType);
llvm::SmallVector<mlir::Value> firBaseTypeParameters;
auto [base, shape] = hlfir::genVariableFirBaseShapeAndParams(
loc, builder, baseEntity, firBaseTypeParameters);
@@ -464,7 +467,7 @@ class DesignateOpConversion
mlir::Type componentType =
mlir::cast<fir::RecordType>(baseEleTy).getType(
designate.getComponent().value());
- mlir::Type coorTy = fir::ReferenceType::get(componentType);
+ mlir::Type coorTy = fir::ReferenceType::get(componentType, isVolatile);
base = builder.create<fir::CoordinateOp>(loc, coorTy, base, fieldIndex);
if (mlir::isa<fir::BaseBoxType>(componentType)) {
auto variableInterface = mlir::cast<fir::FortranVariableOpInterface>(
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
index 96a3622f4afee..020915179a670 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
@@ -1126,7 +1126,8 @@ class ReductionMaskConversion : public mlir::OpRewritePattern<Op> {
builder.create<fir::StoreOp>(loc, flagSet, flagRef);
mlir::Type resultElemTy =
hlfir::getFortranElementType(resultArr.getType());
- mlir::Type returnRefTy = builder.getRefType(resultElemTy);
+ mlir::Type returnRefTy = builder.getRefType(
+ resultElemTy, fir::isa_volatile_ref_type(flagRef.getType()));
mlir::IndexType idxTy = builder.getIndexType();
for (unsigned int i = 0; i < rank; ++i) {
@@ -1153,7 +1154,8 @@ class ReductionMaskConversion : public mlir::OpRewritePattern<Op> {
auto getAddrFn = [](fir::FirOpBuilder builder, mlir::Location loc,
const mlir::Type &resultElemType, mlir::Value resultArr,
mlir::Value index) {
- mlir::Type resultRefTy = builder.getRefType(resultElemType);
+ mlir::Type resultRefTy = builder.getRefType(
+ resultElemType, fir::isa_volatile_ref_type(resultArr.getType()));
mlir::Value oneIdx =
builder.createIntegerConstant(loc, builder.getIndexType(), 1);
index = builder.create<mlir::arith::AddIOp>(loc, index, oneIdx);
@@ -1162,8 +1164,9 @@ class ReductionMaskConversion : public mlir::OpRewritePattern<Op> {
};
// Initialize the resu...
[truncated]
|
tblah
reviewed
Mar 24, 2025
vzakhari
reviewed
Mar 24, 2025
jeanPerier
reviewed
Mar 24, 2025
flang/test/Lower/volatile.fir
Outdated
Comment on lines
13
to
14
| hlfir.assign %7 to %2#0 : !fir.logical<4>, !fir.ref<!fir.logical<4>, volatile> | ||
| %8 = fir.load %2#0 : !fir.ref<!fir.logical<4>, volatile> |
There was a problem hiding this comment.
Nit: Do we have to augment the semantics/docs of hlfir.assign, fir.load, fir.storeon how they behave with a volatile type?
There was a problem hiding this comment.
I believe the semantic differences should be handled by updating memory effects, which are modeled by adding generic read and write effects to all ops taking a volatile reference (even those that technically only read). We might want to use read+write effects on a specific memory resource for volatile variables as Slava suggested.
The docs should be updated to reflect this too, thanks!
|
Thanks for the reviews, everyone! In response to @kiranchandramohan and @jeanPerier 's questions/comments:
There are several ops (copy, array_merge, embox, ext_embox) and at least one more type ( Also, Jean suggested on the discourse thread that the volatile argument should not have a default:
I like this idea |
Would you also need to change the runtime functions like |
Great question. Let me dig some more and get back to you. |
|
For Assign in particular, I think we can use Another thought: the only time casts between volatile and non-volatile types should happen automatically is at callsites. IIUC, if a variable is volatile in the caller's scope but not in the callee scope (or any other time there's a mismatch between argument types and the function declaration's types), then we should automatically add conversions. Kiran's suggestion of using a special cast op separate from |
Arrays may be volatile as well, right, so we will have the runtime calls in some cases? I believe our calls have the most conservative memory effects (currently), so passing volatile to them should not be a problem. I think we will have to account for
Sounds right. I do not have a ready answer, but we will need to clarify how |
Looking into this, thanks! Also: I'm not sure we will need the volatile attribute on the dummy arguments as they are set right now: The |
ashermancinelli
force-pushed
the
ajm/flang-support-volatile
branch
from
63d7672 to
e079baf
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ashermancinelli
commented
Mar 31, 2025
vzakhari
reviewed
Apr 1, 2025
|
Thanks for the review Slava! I'll address these in the morning. |
vzakhari
approved these changes
Apr 1, 2025
ashermancinelli
force-pushed
the
ajm/flang-support-volatile
branch
from
249addd to
dd0bfd1
Compare
ashermancinelli
commented
Apr 23, 2025
|
|
||
| // ----- | ||
|
|
||
| // Check that volatile hlfir assignments are PRESERVED. |
There was a problem hiding this comment.
@jeanPerier Thanks for the review on the other PR! This is the test I added for hlfir assignments. It may be worth adding tests for all/most other hlfir operations as well.
ashermancinelli
changed the title
This was referenced Apr 25, 2025
…flang-support-volatile
…li/llvm-project into ajm/flang-support-volatile
|
If this is the final patch in the volatile series, would you mind adding an entry in https://github.com/llvm/llvm-project/blob/main/flang/docs/ReleaseNotes.md and removing the entry for volatile in https://github.com/llvm/llvm-project/blob/main/flang/docs/FortranStandardsSupport.md ? |
Yes! Please let me know if the docs look okay to you. |
|
I updated the docs. |
Thank. Looks good. |
|
@ashermancinelli Flang complains The standard says in [8.5.20 VOLATILE attribute] So this code should be valid. The full test cases fails at verification of lowering. |
|
Hello @DanielCChen, thank you for the report. #138339 fixes your test case on the machines I just tested on, and I'd like to merge that as soon as I can. |
|
Got it! Thanks! I will verify the rest of the regression with that patch. |
|
@ashermancinelli |
|
Ah, you were asking about the warning. This series of patches only deals with the lowering of volatile entities, not anything in the frontend. @akuhlens added a warning last week I think, I'll ask him about this. |
Oh I see. Yeah, the warning should be removed. Thanks! I got the assert in lowering for another case. |
|
@DanielCChen PR #138611 fixes your issue. Sorry for not catching that. |
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
[RFC on discourse](https://discourse.llvm.org/t/rfc-volatile-representation-in-flang/85404/1) Flang currently lacks support for volatile variables. For some cases, the compiler produces TODO error messages and others are ignored. Some of our tests are like the example from _C.4 Clause 8 notes: The VOLATILE attribute (8.5.20)_ and require volatile variables. Prior commits: ``` c9ec1bc [flang] Handle volatility in lowering and codegen (llvm#135311) e42f860 [flang][nfc] Support volatility in Fir ops (llvm#134858) b2711e1 [flang][nfc] Support volatile on ref, box, and class types (llvm#134386) ```
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
[RFC on discourse](https://discourse.llvm.org/t/rfc-volatile-representation-in-flang/85404/1) Flang currently lacks support for volatile variables. For some cases, the compiler produces TODO error messages and others are ignored. Some of our tests are like the example from _C.4 Clause 8 notes: The VOLATILE attribute (8.5.20)_ and require volatile variables. Prior commits: ``` c9ec1bc [flang] Handle volatility in lowering and codegen (llvm#135311) e42f860 [flang][nfc] Support volatility in Fir ops (llvm#134858) b2711e1 [flang][nfc] Support volatile on ref, box, and class types (llvm#134386) ```
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
[RFC on discourse](https://discourse.llvm.org/t/rfc-volatile-representation-in-flang/85404/1) Flang currently lacks support for volatile variables. For some cases, the compiler produces TODO error messages and others are ignored. Some of our tests are like the example from _C.4 Clause 8 notes: The VOLATILE attribute (8.5.20)_ and require volatile variables. Prior commits: ``` c9ec1bc [flang] Handle volatility in lowering and codegen (llvm#135311) e42f860 [flang][nfc] Support volatility in Fir ops (llvm#134858) b2711e1 [flang][nfc] Support volatile on ref, box, and class types (llvm#134386) ```
GeorgeARM
pushed a commit
to GeorgeARM/llvm-project
that referenced
this pull request
May 7, 2025
[RFC on discourse](https://discourse.llvm.org/t/rfc-volatile-representation-in-flang/85404/1) Flang currently lacks support for volatile variables. For some cases, the compiler produces TODO error messages and others are ignored. Some of our tests are like the example from _C.4 Clause 8 notes: The VOLATILE attribute (8.5.20)_ and require volatile variables. Prior commits: ``` c9ec1bc [flang] Handle volatility in lowering and codegen (llvm#135311) e42f860 [flang][nfc] Support volatility in Fir ops (llvm#134858) b2711e1 [flang][nfc] Support volatile on ref, box, and class types (llvm#134386) ```
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
[RFC on discourse](https://discourse.llvm.org/t/rfc-volatile-representation-in-flang/85404/1) Flang currently lacks support for volatile variables. For some cases, the compiler produces TODO error messages and others are ignored. Some of our tests are like the example from _C.4 Clause 8 notes: The VOLATILE attribute (8.5.20)_ and require volatile variables. Prior commits: ``` c9ec1bc [flang] Handle volatility in lowering and codegen (llvm#135311) e42f860 [flang][nfc] Support volatility in Fir ops (llvm#134858) b2711e1 [flang][nfc] Support volatile on ref, box, and class types (llvm#134386) ```
akuhlens
added a commit
that referenced
this pull request
May 10, 2025
…component symbol (#138611) The standard says in [8.5.20 VOLATILE attribute]: If an object has the VOLATILE attribute, then all of its sub-objects also have the VOLATILE attribute. This code takes this into account and uses the volatility of the base of the designator instead of that of the component. In fact, fields in a structure are not allowed to have the volatile attribute. So given the code, `A%B => t`, symbol `B` could never directly have the volatile attribute, and the volatility of `A` indicates the volatility of `B`. This PR should address [the comments](#132486 (comment)) on this PR #132486
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
May 10, 2025
…instead of component symbol (#138611) The standard says in [8.5.20 VOLATILE attribute]: If an object has the VOLATILE attribute, then all of its sub-objects also have the VOLATILE attribute. This code takes this into account and uses the volatility of the base of the designator instead of that of the component. In fact, fields in a structure are not allowed to have the volatile attribute. So given the code, `A%B => t`, symbol `B` could never directly have the volatile attribute, and the volatility of `A` indicates the volatility of `B`. This PR should address [the comments](llvm/llvm-project#132486 (comment)) on this PR #132486
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RFC on discourse
Flang currently lacks support for volatile variables. For some cases, the compiler produces TODO error messages and others are ignored. Some of our tests are like the example from C.4 Clause 8 notes: The VOLATILE attribute (8.5.20) and require volatile variables.
Prior commits: