Skip to content

Commit 4ddc756

Browse files
jeanPerierjeanPerier
authored
Revert "[flang] correctly deal with bind(c) derived type result ABI" (#111858)
Reverts #111678 Causes ARM failure in test suite. TYPE(C_PTR) result should not regress even if struct ABI no implemented for the target. https://lab.llvm.org/buildbot/#/builders/143/builds/2731 I need to revisit this.
1 parent 97a4324 commit 4ddc756

File tree

7 files changed

+40
-419
lines changed

7 files changed

+40
-419
lines changed

‎flang/include/flang/Optimizer/CodeGen/Target.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,6 @@ class CodeGenSpecifics {
126126
structArgumentType(mlir::Location loc, fir::RecordType recTy,
127127
const Marshalling &previousArguments) const = 0;
128128

129-
/// Type representation of a `fir.type<T>` type argument when returned by
130-
/// value. Such value may need to be converted to a hidden reference argument.
131-
virtual Marshalling structReturnType(mlir::Location loc,
132-
fir::RecordType eleTy) const = 0;
133-
134129
/// Type representation of a `boxchar<n>` type argument when passed by value.
135130
/// An argument value may need to be passed as a (safe) reference argument.
136131
///

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

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -177,27 +177,6 @@ inline mlir::NamedAttribute getAdaptToByRefAttr(Builder &builder) {
177177
}
178178

179179
bool isDummyArgument(mlir::Value v);
180-
181-
template <fir::FortranProcedureFlagsEnum Flag>
182-
inline bool hasProcedureAttr(fir::FortranProcedureFlagsEnumAttr flags) {
183-
return flags && bitEnumContainsAny(flags.getValue(), Flag);
184-
}
185-
186-
template <fir::FortranProcedureFlagsEnum Flag>
187-
inline bool hasProcedureAttr(mlir::Operation *op) {
188-
if (auto firCallOp = mlir::dyn_cast<fir::CallOp>(op))
189-
return hasProcedureAttr<Flag>(firCallOp.getProcedureAttrsAttr());
190-
if (auto firCallOp = mlir::dyn_cast<fir::DispatchOp>(op))
191-
return hasProcedureAttr<Flag>(firCallOp.getProcedureAttrsAttr());
192-
return hasProcedureAttr<Flag>(
193-
op->getAttrOfType<fir::FortranProcedureFlagsEnumAttr>(
194-
getFortranProcedureFlagsAttrName()));
195-
}
196-
197-
inline bool hasBindcAttr(mlir::Operation *op) {
198-
return hasProcedureAttr<fir::FortranProcedureFlagsEnum::bind_c>(op);
199-
}
200-
201180
} // namespace fir
202181

203182
#endif // FORTRAN_OPTIMIZER_DIALECT_FIROPSSUPPORT_H

‎flang/lib/Optimizer/CodeGen/Target.cpp

Lines changed: 6 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,6 @@ struct GenericTarget : public CodeGenSpecifics {
100100
TODO(loc, "passing VALUE BIND(C) derived type for this target");
101101
}
102102

103-
CodeGenSpecifics::Marshalling
104-
structReturnType(mlir::Location loc, fir::RecordType ty) const override {
105-
TODO(loc, "returning BIND(C) derived type for this target");
106-
}
107-
108103
CodeGenSpecifics::Marshalling
109104
integerArgumentType(mlir::Location loc,
110105
mlir::IntegerType argTy) const override {
@@ -538,17 +533,14 @@ struct TargetX86_64 : public GenericTarget<TargetX86_64> {
538533
/// When \p recTy is a one field record type that can be passed
539534
/// like the field on its own, returns the field type. Returns
540535
/// a null type otherwise.
541-
mlir::Type passAsFieldIfOneFieldStruct(fir::RecordType recTy,
542-
bool allowComplex = false) const {
536+
mlir::Type passAsFieldIfOneFieldStruct(fir::RecordType recTy) const {
543537
auto typeList = recTy.getTypeList();
544538
if (typeList.size() != 1)
545539
return {};
546540
mlir::Type fieldType = typeList[0].second;
547541
if (mlir::isa<mlir::FloatType, mlir::IntegerType, fir::LogicalType>(
548542
fieldType))
549543
return fieldType;
550-
if (allowComplex && mlir::isa<mlir::ComplexType>(fieldType))
551-
return fieldType;
552544
if (mlir::isa<fir::CharacterType>(fieldType)) {
553545
// Only CHARACTER(1) are expected in BIND(C) contexts, which is the only
554546
// contexts where derived type may be passed in registers.
@@ -601,7 +593,7 @@ struct TargetX86_64 : public GenericTarget<TargetX86_64> {
601593
postMerge(byteOffset, Lo, Hi);
602594
if (Lo == ArgClass::Memory || Lo == ArgClass::X87 ||
603595
Lo == ArgClass::ComplexX87)
604-
return passOnTheStack(loc, recTy, /*isResult=*/false);
596+
return passOnTheStack(loc, recTy);
605597
int neededIntRegisters = 0;
606598
int neededSSERegisters = 0;
607599
if (Lo == ArgClass::SSE)
@@ -617,7 +609,7 @@ struct TargetX86_64 : public GenericTarget<TargetX86_64> {
617609
// all in registers or all on the stack).
618610
if (!hasEnoughRegisters(loc, neededIntRegisters, neededSSERegisters,
619611
previousArguments))
620-
return passOnTheStack(loc, recTy, /*isResult=*/false);
612+
return passOnTheStack(loc, recTy);
621613

622614
if (auto fieldType = passAsFieldIfOneFieldStruct(recTy)) {
623615
CodeGenSpecifics::Marshalling marshal;
@@ -649,65 +641,17 @@ struct TargetX86_64 : public GenericTarget<TargetX86_64> {
649641
return marshal;
650642
}
651643

652-
CodeGenSpecifics::Marshalling
653-
structReturnType(mlir::Location loc, fir::RecordType recTy) const override {
654-
std::uint64_t byteOffset = 0;
655-
ArgClass Lo, Hi;
656-
Lo = Hi = ArgClass::NoClass;
657-
byteOffset = classifyStruct(loc, recTy, byteOffset, Lo, Hi);
658-
mlir::MLIRContext *context = recTy.getContext();
659-
postMerge(byteOffset, Lo, Hi);
660-
if (Lo == ArgClass::Memory)
661-
return passOnTheStack(loc, recTy, /*isResult=*/true);
662-
663-
// Note that X87/ComplexX87 are passed in memory, but returned via %st0
664-
// %st1 registers. Here, they are returned as fp80 or {fp80, fp80} by
665-
// passAsFieldIfOneFieldStruct, and LLVM will use the expected registers.
666-
667-
// Note that {_Complex long double} is not 100% clear from an ABI
668-
// perspective because the aggregate post merger rules say it should be
669-
// passed in memory because it is bigger than 2 eight bytes. This has the
670-
// funny effect of
671-
// {_Complex long double} return to be dealt with differently than
672-
// _Complex long double.
673-
674-
if (auto fieldType =
675-
passAsFieldIfOneFieldStruct(recTy, /*allowComplex=*/true)) {
676-
if (auto complexType = mlir::dyn_cast<mlir::ComplexType>(fieldType))
677-
return complexReturnType(loc, complexType.getElementType());
678-
CodeGenSpecifics::Marshalling marshal;
679-
marshal.emplace_back(fieldType, AT{});
680-
return marshal;
681-
}
682-
683-
if (Hi == ArgClass::NoClass || Hi == ArgClass::SSEUp) {
684-
// Return a single integer or floating point argument.
685-
mlir::Type lowType = pickLLVMArgType(loc, context, Lo, byteOffset);
686-
CodeGenSpecifics::Marshalling marshal;
687-
marshal.emplace_back(lowType, AT{});
688-
return marshal;
689-
}
690-
// Will be returned in two different registers. Generate {lowTy, HiTy} for
691-
// the LLVM IR result type.
692-
CodeGenSpecifics::Marshalling marshal;
693-
mlir::Type lowType = pickLLVMArgType(loc, context, Lo, 8u);
694-
mlir::Type hiType = pickLLVMArgType(loc, context, Hi, byteOffset - 8u);
695-
marshal.emplace_back(mlir::TupleType::get(context, {lowType, hiType}),
696-
AT{});
697-
return marshal;
698-
}
699-
700644
/// Marshal an argument that must be passed on the stack.
701-
CodeGenSpecifics::Marshalling
702-
passOnTheStack(mlir::Location loc, mlir::Type ty, bool isResult) const {
645+
CodeGenSpecifics::Marshalling passOnTheStack(mlir::Location loc,
646+
mlir::Type ty) const {
703647
CodeGenSpecifics::Marshalling marshal;
704648
auto sizeAndAlign =
705649
fir::getTypeSizeAndAlignmentOrCrash(loc, ty, getDataLayout(), kindMap);
706650
// The stack is always 8 byte aligned (note 14 in 3.2.3).
707651
unsigned short align =
708652
std::max(sizeAndAlign.second, static_cast<unsigned short>(8));
709653
marshal.emplace_back(fir::ReferenceType::get(ty),
710-
AT{align, /*byval=*/!isResult, /*sret=*/isResult});
654+
AT{align, /*byval=*/true, /*sret=*/false});
711655
return marshal;
712656
}
713657
};

‎flang/lib/Optimizer/CodeGen/TargetRewrite.cpp

Lines changed: 30 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -142,16 +142,20 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
142142

143143
mlir::ModuleOp getModule() { return getOperation(); }
144144

145-
template <typename Ty, typename Callback>
145+
template <typename A, typename B, typename C>
146146
std::optional<std::function<mlir::Value(mlir::Operation *)>>
147-
rewriteCallResultType(mlir::Location loc, mlir::Type originalResTy,
148-
Ty &newResTys,
149-
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs,
150-
Callback &newOpers, mlir::Value &savedStackPtr,
151-
fir::CodeGenSpecifics::Marshalling &m) {
152-
// Currently, targets mandate COMPLEX or STRUCT is a single aggregate or
153-
// packed scalar, including the sret case.
154-
assert(m.size() == 1 && "return type not supported on this target");
147+
rewriteCallComplexResultType(
148+
mlir::Location loc, A ty, B &newResTys,
149+
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs, C &newOpers,
150+
mlir::Value &savedStackPtr) {
151+
if (noComplexConversion) {
152+
newResTys.push_back(ty);
153+
return std::nullopt;
154+
}
155+
auto m = specifics->complexReturnType(loc, ty.getElementType());
156+
// Currently targets mandate COMPLEX is a single aggregate or packed
157+
// scalar, including the sret case.
158+
assert(m.size() == 1 && "target of complex return not supported");
155159
auto resTy = std::get<mlir::Type>(m[0]);
156160
auto attr = std::get<fir::CodeGenSpecifics::Attributes>(m[0]);
157161
if (attr.isSRet()) {
@@ -166,7 +170,7 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
166170
newInTyAndAttrs.push_back(m[0]);
167171
newOpers.push_back(stack);
168172
return [=](mlir::Operation *) -> mlir::Value {
169-
auto memTy = fir::ReferenceType::get(originalResTy);
173+
auto memTy = fir::ReferenceType::get(ty);
170174
auto cast = rewriter->create<fir::ConvertOp>(loc, memTy, stack);
171175
return rewriter->create<fir::LoadOp>(loc, cast);
172176
};
@@ -176,41 +180,11 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
176180
// We are going to generate an alloca, so save the stack pointer.
177181
if (!savedStackPtr)
178182
savedStackPtr = genStackSave(loc);
179-
return this->convertValueInMemory(loc, call->getResult(0), originalResTy,
183+
return this->convertValueInMemory(loc, call->getResult(0), ty,
180184
/*inputMayBeBigger=*/true);
181185
};
182186
}
183187

184-
template <typename Ty, typename Callback>
185-
std::optional<std::function<mlir::Value(mlir::Operation *)>>
186-
rewriteCallComplexResultType(
187-
mlir::Location loc, mlir::ComplexType ty, Ty &newResTys,
188-
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs, Callback &newOpers,
189-
mlir::Value &savedStackPtr) {
190-
if (noComplexConversion) {
191-
newResTys.push_back(ty);
192-
return std::nullopt;
193-
}
194-
auto m = specifics->complexReturnType(loc, ty.getElementType());
195-
return rewriteCallResultType(loc, ty, newResTys, newInTyAndAttrs, newOpers,
196-
savedStackPtr, m);
197-
}
198-
199-
template <typename Ty, typename Callback>
200-
std::optional<std::function<mlir::Value(mlir::Operation *)>>
201-
rewriteCallStructResultType(
202-
mlir::Location loc, fir::RecordType recTy, Ty &newResTys,
203-
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs, Callback &newOpers,
204-
mlir::Value &savedStackPtr) {
205-
if (noStructConversion) {
206-
newResTys.push_back(recTy);
207-
return std::nullopt;
208-
}
209-
auto m = specifics->structReturnType(loc, recTy);
210-
return rewriteCallResultType(loc, recTy, newResTys, newInTyAndAttrs,
211-
newOpers, savedStackPtr, m);
212-
}
213-
214188
void passArgumentOnStackOrWithNewType(
215189
mlir::Location loc, fir::CodeGenSpecifics::TypeAndAttr newTypeAndAttr,
216190
mlir::Type oldType, mlir::Value oper,
@@ -382,11 +356,6 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
382356
newInTyAndAttrs, newOpers,
383357
savedStackPtr);
384358
})
385-
.template Case<fir::RecordType>([&](fir::RecordType recTy) {
386-
wrap = rewriteCallStructResultType(loc, recTy, newResTys,
387-
newInTyAndAttrs, newOpers,
388-
savedStackPtr);
389-
})
390359
.Default([&](mlir::Type ty) { newResTys.push_back(ty); });
391360
} else if (fnTy.getResults().size() > 1) {
392361
TODO(loc, "multiple results not supported yet");
@@ -593,24 +562,6 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
593562
}
594563
}
595564

596-
template <typename Ty>
597-
void
598-
lowerStructSignatureRes(mlir::Location loc, fir::RecordType recTy,
599-
Ty &newResTys,
600-
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs) {
601-
if (noComplexConversion) {
602-
newResTys.push_back(recTy);
603-
return;
604-
} else {
605-
for (auto &tup : specifics->structReturnType(loc, recTy)) {
606-
if (std::get<fir::CodeGenSpecifics::Attributes>(tup).isSRet())
607-
newInTyAndAttrs.push_back(tup);
608-
else
609-
newResTys.push_back(std::get<mlir::Type>(tup));
610-
}
611-
}
612-
}
613-
614565
void
615566
lowerStructSignatureArg(mlir::Location loc, fir::RecordType recTy,
616567
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs) {
@@ -644,9 +595,6 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
644595
.Case<mlir::ComplexType>([&](mlir::ComplexType ty) {
645596
lowerComplexSignatureRes(loc, ty, newResTys, newInTyAndAttrs);
646597
})
647-
.Case<fir::RecordType>([&](fir::RecordType ty) {
648-
lowerStructSignatureRes(loc, ty, newResTys, newInTyAndAttrs);
649-
})
650598
.Default([&](mlir::Type ty) { newResTys.push_back(ty); });
651599
}
652600
llvm::SmallVector<mlir::Type> trailingInTys;
@@ -748,8 +696,7 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
748696
for (auto ty : func.getResults())
749697
if ((mlir::isa<fir::BoxCharType>(ty) && !noCharacterConversion) ||
750698
(fir::isa_complex(ty) && !noComplexConversion) ||
751-
(mlir::isa<mlir::IntegerType>(ty) && hasCCallingConv) ||
752-
(mlir::isa<fir::RecordType>(ty) && !noStructConversion)) {
699+
(mlir::isa<mlir::IntegerType>(ty) && hasCCallingConv)) {
753700
LLVM_DEBUG(llvm::dbgs() << "rewrite " << signature << " for target\n");
754701
return false;
755702
}
@@ -823,9 +770,6 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
823770
rewriter->getUnitAttr()));
824771
newResTys.push_back(retTy);
825772
})
826-
.Case<fir::RecordType>([&](fir::RecordType recTy) {
827-
doStructReturn(func, recTy, newResTys, newInTyAndAttrs, fixups);
828-
})
829773
.Default([&](mlir::Type ty) { newResTys.push_back(ty); });
830774

831775
// Saved potential shift in argument. Handling of result can add arguments
@@ -1118,12 +1062,21 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
11181062
return false;
11191063
}
11201064

1065+
/// Convert a complex return value. This can involve converting the return
1066+
/// value to a "hidden" first argument or packing the complex into a wide
1067+
/// GPR.
11211068
template <typename Ty, typename FIXUPS>
1122-
void doReturn(mlir::func::FuncOp func, Ty &newResTys,
1123-
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs,
1124-
FIXUPS &fixups, fir::CodeGenSpecifics::Marshalling &m) {
1125-
assert(m.size() == 1 &&
1126-
"expect result to be turned into single argument or result so far");
1069+
void doComplexReturn(mlir::func::FuncOp func, mlir::ComplexType cmplx,
1070+
Ty &newResTys,
1071+
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs,
1072+
FIXUPS &fixups) {
1073+
if (noComplexConversion) {
1074+
newResTys.push_back(cmplx);
1075+
return;
1076+
}
1077+
auto m =
1078+
specifics->complexReturnType(func.getLoc(), cmplx.getElementType());
1079+
assert(m.size() == 1);
11271080
auto &tup = m[0];
11281081
auto attr = std::get<fir::CodeGenSpecifics::Attributes>(tup);
11291082
auto argTy = std::get<mlir::Type>(tup);
@@ -1164,36 +1117,6 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
11641117
newResTys.push_back(argTy);
11651118
}
11661119

1167-
/// Convert a complex return value. This can involve converting the return
1168-
/// value to a "hidden" first argument or packing the complex into a wide
1169-
/// GPR.
1170-
template <typename Ty, typename FIXUPS>
1171-
void doComplexReturn(mlir::func::FuncOp func, mlir::ComplexType cmplx,
1172-
Ty &newResTys,
1173-
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs,
1174-
FIXUPS &fixups) {
1175-
if (noComplexConversion) {
1176-
newResTys.push_back(cmplx);
1177-
return;
1178-
}
1179-
auto m =
1180-
specifics->complexReturnType(func.getLoc(), cmplx.getElementType());
1181-
doReturn(func, newResTys, newInTyAndAttrs, fixups, m);
1182-
}
1183-
1184-
template <typename Ty, typename FIXUPS>
1185-
void doStructReturn(mlir::func::FuncOp func, fir::RecordType recTy,
1186-
Ty &newResTys,
1187-
fir::CodeGenSpecifics::Marshalling &newInTyAndAttrs,
1188-
FIXUPS &fixups) {
1189-
if (noStructConversion) {
1190-
newResTys.push_back(recTy);
1191-
return;
1192-
}
1193-
auto m = specifics->structReturnType(func.getLoc(), recTy);
1194-
doReturn(func, newResTys, newInTyAndAttrs, fixups, m);
1195-
}
1196-
11971120
template <typename FIXUPS>
11981121
void
11991122
createFuncOpArgFixups(mlir::func::FuncOp func,

0 commit comments

Comments
 (0)