Skip to content

Commit 0ccef6a

Browse files
LeporacanthicusLeporacanthicus
authored
[flang] Make adapt.valuebyref attribute work again (#73658)
This got "lost" in the HLFIR transformation. This patch applies the old attribute to the AssociateOp that needs it, and forwards it to the AllocaOp that is generated when lowering to FIR.
1 parent 953d675 commit 0ccef6a

40 files changed

+162
-113
lines changed

‎flang/include/flang/Lower/ConvertExpr.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -225,15 +225,6 @@ mlir::Value createSubroutineCall(AbstractConverter &converter,
225225
SymMap &symMap, StatementContext &stmtCtx,
226226
bool isUserDefAssignment);
227227

228-
// Attribute for an alloca that is a trivial adaptor for converting a value to
229-
// pass-by-ref semantics for a VALUE parameter. The optimizer may be able to
230-
// eliminate these.
231-
inline mlir::NamedAttribute getAdaptToByRefAttr(fir::FirOpBuilder &builder) {
232-
return {mlir::StringAttr::get(builder.getContext(),
233-
fir::getAdaptToByRefAttrName()),
234-
builder.getUnitAttr()};
235-
}
236-
237228
mlir::Value addCrayPointerInst(mlir::Location loc, fir::FirOpBuilder &builder,
238229
mlir::Value ptrVal, mlir::Type ptrTy,
239230
mlir::Type pteTy);

‎flang/include/flang/Optimizer/Builder/HLFIRTools.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -239,11 +239,11 @@ fir::FortranVariableOpInterface genDeclare(mlir::Location loc,
239239
/// Generate an hlfir.associate to build a variable from an expression value.
240240
/// The type of the variable must be provided so that scalar logicals are
241241
/// properly typed when placed in memory.
242-
hlfir::AssociateOp genAssociateExpr(mlir::Location loc,
243-
fir::FirOpBuilder &builder,
244-
hlfir::Entity value,
245-
mlir::Type variableType,
246-
llvm::StringRef name);
242+
hlfir::AssociateOp
243+
genAssociateExpr(mlir::Location loc, fir::FirOpBuilder &builder,
244+
hlfir::Entity value, mlir::Type variableType,
245+
llvm::StringRef name,
246+
std::optional<mlir::NamedAttribute> attr = std::nullopt);
247247

248248
/// Get the raw address of a variable (simple fir.ref/fir.ptr, or fir.heap
249249
/// value). The returned value should be used with care, it does not contain any

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,6 @@ mlir::ParseResult parseSelector(mlir::OpAsmParser &parser,
3838
mlir::OpAsmParser::UnresolvedOperand &selector,
3939
mlir::Type &type);
4040

41-
static constexpr llvm::StringRef getAdaptToByRefAttrName() {
42-
return "adapt.valuebyref";
43-
}
44-
4541
static constexpr llvm::StringRef getNormalizedLowerBoundAttrName() {
4642
return "normalized.lb";
4743
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,22 @@ inline std::optional<std::int64_t> getIntIfConstant(mlir::Value value) {
141141
return {};
142142
}
143143

144+
static constexpr llvm::StringRef getAdaptToByRefAttrName() {
145+
return "adapt.valuebyref";
146+
}
147+
148+
// Attribute for an alloca that is a trivial adaptor for converting a value to
149+
// pass-by-ref semantics for a VALUE parameter. The optimizer may be able to
150+
// eliminate these.
151+
// Template is used to avoid compiler errors in places that don't include
152+
// FIRBuilder.h
153+
template <typename Builder>
154+
inline mlir::NamedAttribute getAdaptToByRefAttr(Builder &builder) {
155+
return {mlir::StringAttr::get(builder.getContext(),
156+
fir::getAdaptToByRefAttrName()),
157+
builder.getUnitAttr()};
158+
}
159+
144160
} // namespace fir
145161

146162
#endif // FORTRAN_OPTIMIZER_DIALECT_FIROPSSUPPORT_H

‎flang/include/flang/Optimizer/HLFIR/HLFIROps.td

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ def hlfir_AssociateOp : hlfir_Op<"associate", [AttrSizedOperandSegments,
660660
AnyFortranValue:$source,
661661
Optional<AnyShapeOrShiftType>:$shape,
662662
Variadic<AnyIntegerType>:$typeparams,
663-
Builtin_StringAttr:$uniq_name,
663+
OptionalAttr<Builtin_StringAttr>:$uniq_name,
664664
OptionalAttr<fir_FortranVariableFlagsAttr>:$fortran_attrs
665665
);
666666

@@ -672,9 +672,13 @@ def hlfir_AssociateOp : hlfir_Op<"associate", [AttrSizedOperandSegments,
672672
}];
673673

674674
let builders = [
675-
OpBuilder<(ins "mlir::Value":$source, "llvm::StringRef":$uniq_name,
675+
OpBuilder<(ins "mlir::Value":$source, CArg<"llvm::StringRef", "{}">:$uniq_name,
676676
CArg<"mlir::Value", "{}">:$shape, CArg<"mlir::ValueRange", "{}">:$typeparams,
677-
CArg<"fir::FortranVariableFlagsAttr", "{}">:$fortran_attrs)>];
677+
CArg<"fir::FortranVariableFlagsAttr", "{}">:$fortran_attrs)>,
678+
OpBuilder<(ins "mlir::Value":$memref, CArg<"mlir::Value", "{}">:$shape,
679+
CArg<"mlir::ValueRange", "{}">:$typeparams,
680+
CArg<"fir::FortranVariableFlagsAttr", "{}">:$fortran_attrs,
681+
CArg<"llvm::ArrayRef<mlir::NamedAttribute>", "{}">:$attributes)>];
678682

679683
let extraClassDeclaration = [{
680684
/// Override FortranVariableInterface default implementation

‎flang/lib/Lower/Bridge.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2083,10 +2083,10 @@ class FirConverter : public Fortran::lower::AbstractConverter {
20832083
assert(sym && "There must be a symbol to bind");
20842084
mlir::Type toTy = genType(*sym);
20852085
// FIXME: this should be a "per iteration" temporary.
2086-
mlir::Value tmp = builder->createTemporary(
2087-
loc, toTy, toStringRef(sym->name()),
2088-
llvm::ArrayRef<mlir::NamedAttribute>{
2089-
Fortran::lower::getAdaptToByRefAttr(*builder)});
2086+
mlir::Value tmp =
2087+
builder->createTemporary(loc, toTy, toStringRef(sym->name()),
2088+
llvm::ArrayRef<mlir::NamedAttribute>{
2089+
fir::getAdaptToByRefAttr(*builder)});
20902090
mlir::Value cast = builder->createConvert(loc, toTy, inducVar);
20912091
builder->create<fir::StoreOp>(loc, cast, tmp);
20922092
addSymbol(*sym, tmp, /*force=*/true);

‎flang/lib/Lower/ConvertCall.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include "flang/Lower/ConvertCall.h"
1414
#include "flang/Lower/Allocatable.h"
15+
#include "flang/Lower/ConvertExpr.h"
1516
#include "flang/Lower/ConvertExprToHLFIR.h"
1617
#include "flang/Lower/ConvertVariable.h"
1718
#include "flang/Lower/CustomIntrinsicCall.h"
@@ -958,8 +959,9 @@ static PreparedDummyArgument preparePresentUserCallActualArgument(
958959
// Make a copy in a temporary.
959960
auto copy = builder.create<hlfir::AsExprOp>(loc, entity);
960961
mlir::Type storageType = entity.getType();
962+
mlir::NamedAttribute byRefAttr = fir::getAdaptToByRefAttr(builder);
961963
hlfir::AssociateOp associate = hlfir::genAssociateExpr(
962-
loc, builder, hlfir::Entity{copy}, storageType, "adapt.valuebyref");
964+
loc, builder, hlfir::Entity{copy}, storageType, "", byRefAttr);
963965
entity = hlfir::Entity{associate.getBase()};
964966
// Register the temporary destruction after the call.
965967
preparedDummy.pushExprAssociateCleanUp(associate);
@@ -986,8 +988,9 @@ static PreparedDummyArgument preparePresentUserCallActualArgument(
986988
// The actual is an expression value, place it into a temporary
987989
// and register the temporary destruction after the call.
988990
mlir::Type storageType = converter.genType(expr);
991+
mlir::NamedAttribute byRefAttr = fir::getAdaptToByRefAttr(builder);
989992
hlfir::AssociateOp associate = hlfir::genAssociateExpr(
990-
loc, builder, entity, storageType, "adapt.valuebyref");
993+
loc, builder, entity, storageType, "", byRefAttr);
991994
entity = hlfir::Entity{associate.getBase()};
992995
preparedDummy.pushExprAssociateCleanUp(associate);
993996
if (mustSetDynamicTypeToDummyType) {
@@ -1395,8 +1398,9 @@ static std::optional<hlfir::EntityWithAttributes> genCustomIntrinsicRefCore(
13951398
CallContext &callContext) {
13961399
auto &builder = callContext.getBuilder();
13971400
const auto &loc = callContext.loc;
1398-
assert(intrinsic && Fortran::lower::intrinsicRequiresCustomOptionalHandling(
1399-
callContext.procRef, *intrinsic, callContext.converter));
1401+
assert(intrinsic &&
1402+
Fortran::lower::intrinsicRequiresCustomOptionalHandling(
1403+
callContext.procRef, *intrinsic, callContext.converter));
14001404

14011405
// helper to get a particular prepared argument
14021406
auto getArgument = [&](std::size_t i, bool loadArg) -> fir::ExtendedValue {

‎flang/lib/Lower/ConvertExpr.cpp

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,7 @@ placeScalarValueInMemory(fir::FirOpBuilder &builder, mlir::Location loc,
358358
mlir::Value val = builder.createConvert(loc, storageType, valBase);
359359
mlir::Value temp = builder.createTemporary(
360360
loc, storageType,
361-
llvm::ArrayRef<mlir::NamedAttribute>{
362-
Fortran::lower::getAdaptToByRefAttr(builder)});
361+
llvm::ArrayRef<mlir::NamedAttribute>{fir::getAdaptToByRefAttr(builder)});
363362
builder.create<fir::StoreOp>(loc, val, temp);
364363
return fir::substBase(exv, temp);
365364
}
@@ -2401,19 +2400,18 @@ class ScalarExprLowering {
24012400
mlir::Value len = charBox->getLen();
24022401
mlir::Value zero = builder.createIntegerConstant(loc, len.getType(), 0);
24032402
len = builder.create<mlir::arith::SelectOp>(loc, isPresent, len, zero);
2404-
mlir::Value temp = builder.createTemporary(
2405-
loc, type, /*name=*/{},
2406-
/*shape=*/{}, mlir::ValueRange{len},
2407-
llvm::ArrayRef<mlir::NamedAttribute>{
2408-
Fortran::lower::getAdaptToByRefAttr(builder)});
2403+
mlir::Value temp =
2404+
builder.createTemporary(loc, type, /*name=*/{},
2405+
/*shape=*/{}, mlir::ValueRange{len},
2406+
llvm::ArrayRef<mlir::NamedAttribute>{
2407+
fir::getAdaptToByRefAttr(builder)});
24092408
return fir::CharBoxValue{temp, len};
24102409
}
24112410
assert((fir::isa_trivial(type) || type.isa<fir::RecordType>()) &&
24122411
"must be simple scalar");
2413-
return builder.createTemporary(
2414-
loc, type,
2415-
llvm::ArrayRef<mlir::NamedAttribute>{
2416-
Fortran::lower::getAdaptToByRefAttr(builder)});
2412+
return builder.createTemporary(loc, type,
2413+
llvm::ArrayRef<mlir::NamedAttribute>{
2414+
fir::getAdaptToByRefAttr(builder)});
24172415
}
24182416

24192417
template <typename A>
@@ -4752,10 +4750,10 @@ class ArrayExprLowering {
47524750
} else {
47534751
// Store scalar value in a temp to fulfill VALUE attribute.
47544752
mlir::Value val = fir::getBase(asScalar(*expr));
4755-
mlir::Value temp = builder.createTemporary(
4756-
loc, val.getType(),
4757-
llvm::ArrayRef<mlir::NamedAttribute>{
4758-
Fortran::lower::getAdaptToByRefAttr(builder)});
4753+
mlir::Value temp =
4754+
builder.createTemporary(loc, val.getType(),
4755+
llvm::ArrayRef<mlir::NamedAttribute>{
4756+
fir::getAdaptToByRefAttr(builder)});
47594757
builder.create<fir::StoreOp>(loc, val, temp);
47604758
operands.emplace_back(
47614759
[=](IterSpace iters) -> ExtValue { return temp; });
@@ -5843,10 +5841,10 @@ class ArrayExprLowering {
58435841
base = builder.create<fir::ArrayFetchOp>(
58445842
loc, eleTy, arrLd, iters.iterVec(), arrLdTypeParams);
58455843
}
5846-
mlir::Value temp = builder.createTemporary(
5847-
loc, base.getType(),
5848-
llvm::ArrayRef<mlir::NamedAttribute>{
5849-
Fortran::lower::getAdaptToByRefAttr(builder)});
5844+
mlir::Value temp =
5845+
builder.createTemporary(loc, base.getType(),
5846+
llvm::ArrayRef<mlir::NamedAttribute>{
5847+
fir::getAdaptToByRefAttr(builder)});
58505848
builder.create<fir::StoreOp>(loc, base, temp);
58515849
return fir::factory::arraySectionElementToExtendedValue(
58525850
builder, loc, extMemref, temp, slice);

‎flang/lib/Lower/OpenMP.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2104,7 +2104,7 @@ createAndSetPrivatizedLoopVar(Fortran::lower::AbstractConverter &converter,
21042104
loc, tempTy, /*pinned=*/true, /*lengthParams=*/mlir::ValueRange{},
21052105
/*shapeParams*/ mlir::ValueRange{},
21062106
llvm::ArrayRef<mlir::NamedAttribute>{
2107-
Fortran::lower::getAdaptToByRefAttr(firOpBuilder)});
2107+
fir::getAdaptToByRefAttr(firOpBuilder)});
21082108
converter.bindSymbol(*sym, temp);
21092109
firOpBuilder.restoreInsertionPoint(insPt);
21102110
mlir::Value cvtVal = firOpBuilder.createConvert(loc, tempTy, indexVal);

‎flang/lib/Optimizer/Builder/Character.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "flang/Optimizer/Builder/DoLoopHelper.h"
1515
#include "flang/Optimizer/Builder/FIRBuilder.h"
1616
#include "flang/Optimizer/Builder/Todo.h"
17+
#include "flang/Optimizer/Dialect/FIROpsSupport.h"
1718
#include "llvm/Support/Debug.h"
1819
#include <optional>
1920

‎flang/lib/Optimizer/Builder/HLFIRTools.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "flang/Optimizer/Builder/HLFIRTools.h"
14+
#include "flang/Lower/ConvertExpr.h"
1415
#include "flang/Optimizer/Builder/Character.h"
1516
#include "flang/Optimizer/Builder/FIRBuilder.h"
1617
#include "flang/Optimizer/Builder/MutableBox.h"
@@ -232,11 +233,11 @@ hlfir::genDeclare(mlir::Location loc, fir::FirOpBuilder &builder,
232233
return mlir::cast<fir::FortranVariableOpInterface>(declareOp.getOperation());
233234
}
234235

235-
hlfir::AssociateOp hlfir::genAssociateExpr(mlir::Location loc,
236-
fir::FirOpBuilder &builder,
237-
hlfir::Entity value,
238-
mlir::Type variableType,
239-
llvm::StringRef name) {
236+
hlfir::AssociateOp
237+
hlfir::genAssociateExpr(mlir::Location loc, fir::FirOpBuilder &builder,
238+
hlfir::Entity value, mlir::Type variableType,
239+
llvm::StringRef name,
240+
std::optional<mlir::NamedAttribute> attr) {
240241
assert(value.isValue() && "must not be a variable");
241242
mlir::Value shape{};
242243
if (value.isArray())
@@ -259,6 +260,12 @@ hlfir::AssociateOp hlfir::genAssociateExpr(mlir::Location loc,
259260
}
260261
llvm::SmallVector<mlir::Value> lenParams;
261262
genLengthParameters(loc, builder, value, lenParams);
263+
if (attr) {
264+
assert(name.empty() && "It attribute is provided, no-name is expected");
265+
return builder.create<hlfir::AssociateOp>(loc, source, shape, lenParams,
266+
fir::FortranVariableFlagsAttr{},
267+
llvm::ArrayRef{*attr});
268+
}
262269
return builder.create<hlfir::AssociateOp>(loc, source, name, shape, lenParams,
263270
fir::FortranVariableFlagsAttr{});
264271
}
@@ -914,8 +921,9 @@ hlfir::translateToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
914921
}
915922

916923
if (entity.getType().isa<hlfir::ExprType>()) {
924+
mlir::NamedAttribute byRefAttr = fir::getAdaptToByRefAttr(builder);
917925
hlfir::AssociateOp associate = hlfir::genAssociateExpr(
918-
loc, builder, entity, entity.getType(), "adapt.valuebyref");
926+
loc, builder, entity, entity.getType(), "", byRefAttr);
919927
auto *bldr = &builder;
920928
hlfir::CleanupFunction cleanup = [bldr, loc, associate]() -> void {
921929
bldr->create<hlfir::EndAssociateOp>(loc, associate);
@@ -1175,7 +1183,7 @@ hlfir::genTypeAndKindConvert(mlir::Location loc, fir::FirOpBuilder &builder,
11751183
mlir::Value shapeShift =
11761184
builder.create<fir::ShapeShiftOp>(loc, shapeShiftType, lbAndExtents);
11771185
auto declareOp = builder.create<hlfir::DeclareOp>(
1178-
loc, associate.getFirBase(), associate.getUniqName(), shapeShift,
1186+
loc, associate.getFirBase(), *associate.getUniqName(), shapeShift,
11791187
associate.getTypeparams(), /*flags=*/fir::FortranVariableFlagsAttr{});
11801188
hlfir::Entity castWithLbounds =
11811189
mlir::cast<fir::FortranVariableOpInterface>(declareOp.getOperation());

‎flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,6 +1230,29 @@ void hlfir::AssociateOp::build(mlir::OpBuilder &builder,
12301230
typeparams, nameAttr, fortran_attrs);
12311231
}
12321232

1233+
void hlfir::AssociateOp::build(
1234+
mlir::OpBuilder &builder, mlir::OperationState &result, mlir::Value source,
1235+
mlir::Value shape, mlir::ValueRange typeparams,
1236+
fir::FortranVariableFlagsAttr fortran_attrs,
1237+
llvm::ArrayRef<mlir::NamedAttribute> attributes) {
1238+
mlir::Type dataType = getFortranElementOrSequenceType(source.getType());
1239+
1240+
// Preserve polymorphism of polymorphic expr.
1241+
mlir::Type firVarType;
1242+
auto sourceExprType = mlir::dyn_cast<hlfir::ExprType>(source.getType());
1243+
if (sourceExprType && sourceExprType.isPolymorphic())
1244+
firVarType = fir::ClassType::get(fir::HeapType::get(dataType));
1245+
else
1246+
firVarType = fir::ReferenceType::get(dataType);
1247+
1248+
mlir::Type hlfirVariableType =
1249+
DeclareOp::getHLFIRVariableType(firVarType, /*hasExplicitLbs=*/false);
1250+
mlir::Type i1Type = builder.getI1Type();
1251+
build(builder, result, {hlfirVariableType, firVarType, i1Type}, source, shape,
1252+
typeparams, {}, fortran_attrs);
1253+
result.addAttributes(attributes);
1254+
}
1255+
12331256
//===----------------------------------------------------------------------===//
12341257
// EndAssociateOp
12351258
//===----------------------------------------------------------------------===//

‎flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// on the heap.
1313
//===----------------------------------------------------------------------===//
1414

15+
#include "flang/Lower/ConvertExpr.h"
1516
#include "flang/Optimizer/Builder/Character.h"
1617
#include "flang/Optimizer/Builder/FIRBuilder.h"
1718
#include "flang/Optimizer/Builder/HLFIRTools.h"
@@ -452,7 +453,7 @@ struct AssociateOpConversion
452453
// %0 = hlfir.as_expr %x move %true :
453454
// (!fir.box<!fir.heap<!fir.type<_T{y:i32}>>>, i1) ->
454455
// !hlfir.expr<!fir.type<_T{y:i32}>>
455-
// %1:3 = hlfir.associate %0 {uniq_name = "adapt.valuebyref"} :
456+
// %1:3 = hlfir.associate %0 {adapt.valuebyref} :
456457
// (!hlfir.expr<!fir.type<_T{y:i32}>>) ->
457458
// (!fir.ref<!fir.type<_T{y:i32}>>,
458459
// !fir.ref<!fir.type<_T{y:i32}>>,
@@ -525,8 +526,15 @@ struct AssociateOpConversion
525526
return mlir::success();
526527
}
527528
if (isTrivialValue) {
528-
auto temp = builder.createTemporary(loc, bufferizedExpr.getType(),
529-
associate.getUniqName());
529+
llvm::SmallVector<mlir::NamedAttribute, 1> attrs;
530+
if (associate->hasAttr(fir::getAdaptToByRefAttrName())) {
531+
attrs.push_back(fir::getAdaptToByRefAttr(builder));
532+
}
533+
llvm::StringRef name = "";
534+
if (associate.getUniqName())
535+
name = *associate.getUniqName();
536+
auto temp =
537+
builder.createTemporary(loc, bufferizedExpr.getType(), name, attrs);
530538
builder.create<fir::StoreOp>(loc, bufferizedExpr, temp);
531539
mlir::Value mustFree = builder.createBool(loc, false);
532540
replaceWith(temp, temp, mustFree);

0 commit comments

Comments
 (0)