[flang-commits] [flang] [flang][hlfir] Make the parent type the first component (PR #69348)
via flang-commits
flang-commits at lists.llvm.org
Tue Oct 17 09:00:51 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-semantics
Author: None (jeanPerier)
<details>
<summary>Changes</summary>
Type extension is currently handled in FIR by inlining the parents components as the first member of the record type.
This is not correct from a memory layout point of view since the storage size of the parent type may be bigger than the sum of the size of its component (due to alignment requirement). To avoid making FIR types target dependent and fix this issue, make the parent component a single component with the parent type at the beginning of the record type.
This also simplifies addressing since parent component is now a "normal" component that can be designated with hlfir.designate.
StructureComponent lowering however is a bit more complex since the symbols in the structure component may refer to subcomponents of parent types.
Notes:
1. The fix is only done in HLFIR for now, a similar fix should be done in ConvertExpr.cpp to fix the path without HLFIR (I will likely still do it in a new patch since it would be an annoying bug to investigate for people testing flang without HLFIR).
2. The private component extra mangling is useless after this patch. I will remove it after 1.
3. The "parent component" TODO in constant CTOR is free to implement for HLFIR after this patch, but I would rather remove it and test it in a different patch.
---
Patch is 78.26 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/69348.diff
15 Files Affected:
- (modified) flang/include/flang/Lower/ConvertType.h (+30)
- (modified) flang/include/flang/Semantics/tools.h (+1-1)
- (modified) flang/include/flang/Semantics/type.h (+2)
- (modified) flang/lib/Lower/ConvertConstant.cpp (+139-71)
- (modified) flang/lib/Lower/ConvertExprToHLFIR.cpp (+54-78)
- (modified) flang/lib/Lower/ConvertType.cpp (+66-24)
- (modified) flang/lib/Lower/ConvertVariable.cpp (+98-57)
- (modified) flang/lib/Semantics/type.cpp (+4)
- (modified) flang/test/Lower/HLFIR/calls-constant-expr-arg-polymorphic.f90 (+5-5)
- (modified) flang/test/Lower/HLFIR/local-end-of-scope-component-dealloc.f90 (+6-6)
- (modified) flang/test/Lower/HLFIR/parent-component-ref.f90 (+21-18)
- (modified) flang/test/Lower/HLFIR/private-components.f90 (+2-2)
- (modified) flang/test/Lower/HLFIR/structure-constructor.f90 (+18-19)
- (modified) flang/test/Lower/HLFIR/type-bound-call-mismatch.f90 (+2-2)
- (modified) flang/test/Lower/HLFIR/type-info.f90 (+4-4)
``````````diff
diff --git a/flang/include/flang/Lower/ConvertType.h b/flang/include/flang/Lower/ConvertType.h
index f86cc0023579c43..61cbc7d7aa303cc 100644
--- a/flang/include/flang/Lower/ConvertType.h
+++ b/flang/include/flang/Lower/ConvertType.h
@@ -48,6 +48,8 @@ struct SomeType;
namespace semantics {
class Symbol;
class DerivedTypeSpec;
+class DerivedTypeDetails;
+class Scope;
} // namespace semantics
namespace lower {
@@ -97,6 +99,34 @@ class TypeBuilder {
using namespace evaluate;
FOR_EACH_SPECIFIC_TYPE(extern template class TypeBuilder, )
+/// A helper class to reverse iterate through the component names of a derived
+/// type, including the parent component and the component of the parents. This
+/// is useful to deal with StructureConstructor lowering.
+class ComponentReverseIterator {
+public:
+ ComponentReverseIterator(const Fortran::semantics::DerivedTypeSpec &derived) {
+ setCurrentType(derived);
+ }
+ /// Does the current type has a component with \name (does not look-up the
+ /// components of the parent if any). If there is a match, the iterator
+ /// is advanced to the search result.
+ bool lookup(const Fortran::parser::CharBlock &name) {
+ componentIt = std::find(componentIt, componentItEnd, name);
+ return componentIt != componentItEnd;
+ };
+
+ /// Advance iterator to the last components of the current type parent.
+ const Fortran::semantics::DerivedTypeSpec &advanceToParentType();
+
+private:
+ void setCurrentType(const Fortran::semantics::DerivedTypeSpec &derived);
+ const Fortran::semantics::DerivedTypeSpec *currentParentType = nullptr;
+ const Fortran::semantics::DerivedTypeDetails *currentTypeDetails = nullptr;
+ using name_iterator =
+ std::list<Fortran::parser::CharBlock>::const_reverse_iterator;
+ name_iterator componentIt{};
+ name_iterator componentItEnd{};
+};
} // namespace lower
} // namespace Fortran
diff --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index 0b5c3dde2e72082..e3deb2da1be04ab 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -509,7 +509,7 @@ template <ComponentKind componentKind> class ComponentIterator {
explicit ComponentPathNode(const DerivedTypeSpec &derived)
: derived_{derived} {
if constexpr (componentKind == ComponentKind::Scope) {
- const Scope &scope{DEREF(derived.scope())};
+ const Scope &scope{DEREF(derived.GetScope())};
nameIterator_ = scope.cbegin();
nameEnd_ = scope.cend();
} else {
diff --git a/flang/include/flang/Semantics/type.h b/flang/include/flang/Semantics/type.h
index 5228c15066f6f39..8965d29d8889dad 100644
--- a/flang/include/flang/Semantics/type.h
+++ b/flang/include/flang/Semantics/type.h
@@ -261,6 +261,8 @@ class DerivedTypeSpec {
const SourceName &name() const { return name_; }
const Symbol &typeSymbol() const { return typeSymbol_; }
const Scope *scope() const { return scope_; }
+ // Return scope_ if it is set, or the typeSymbol_ scope otherwise.
+ const Scope *GetScope() const;
void set_scope(const Scope &);
void ReplaceScope(const Scope &);
const RawParameters &rawParameters() const { return rawParameters_; }
diff --git a/flang/lib/Lower/ConvertConstant.cpp b/flang/lib/Lower/ConvertConstant.cpp
index 6e7a60e42abfe66..c935c90121941f3 100644
--- a/flang/lib/Lower/ConvertConstant.cpp
+++ b/flang/lib/Lower/ConvertConstant.cpp
@@ -347,6 +347,83 @@ genConstantValue(Fortran::lower::AbstractConverter &converter,
mlir::Location loc,
const Fortran::lower::SomeExpr &constantExpr);
+static mlir::Value genStructureComponentInit(
+ Fortran::lower::AbstractConverter &converter, mlir::Location loc,
+ const Fortran::semantics::Symbol &sym, const Fortran::lower::SomeExpr &expr,
+ mlir::Value res) {
+ fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+ fir::RecordType recTy = mlir::cast<fir::RecordType>(res.getType());
+ std::string name = converter.getRecordTypeFieldName(sym);
+ mlir::Type componentTy = recTy.getType(name);
+ auto fieldTy = fir::FieldType::get(recTy.getContext());
+ assert(componentTy && "failed to retrieve component");
+ // FIXME: type parameters must come from the derived-type-spec
+ auto field = builder.create<fir::FieldIndexOp>(
+ loc, fieldTy, name, recTy,
+ /*typeParams=*/mlir::ValueRange{} /*TODO*/);
+
+ if (Fortran::semantics::IsAllocatable(sym))
+ TODO(loc, "allocatable component in structure constructor");
+
+ if (Fortran::semantics::IsPointer(sym)) {
+ mlir::Value initialTarget =
+ Fortran::lower::genInitialDataTarget(converter, loc, componentTy, expr);
+ res = builder.create<fir::InsertValueOp>(
+ loc, recTy, res, initialTarget,
+ builder.getArrayAttr(field.getAttributes()));
+ return res;
+ }
+
+ if (Fortran::lower::isDerivedTypeWithLenParameters(sym))
+ TODO(loc, "component with length parameters in structure constructor");
+
+ // Special handling for scalar c_ptr/c_funptr constants. The array constant
+ // must fall through to genConstantValue() below.
+ if (Fortran::semantics::IsBuiltinCPtr(sym) && sym.Rank() == 0 &&
+ (Fortran::evaluate::GetLastSymbol(expr) ||
+ Fortran::evaluate::IsNullPointer(expr))) {
+ // Builtin c_ptr and c_funptr have special handling because designators
+ // and NULL() are handled as initial values for them as an extension
+ // (otherwise only c_ptr_null/c_funptr_null are allowed and these are
+ // replaced by structure constructors by semantics, so GetLastSymbol
+ // returns nothing).
+
+ // The Ev::Expr is an initializer that is a pointer target (e.g., 'x' or
+ // NULL()) that must be inserted into an intermediate cptr record value's
+ // address field, which ought to be an intptr_t on the target.
+ mlir::Value addr = fir::getBase(
+ Fortran::lower::genExtAddrInInitializer(converter, loc, expr));
+ if (addr.getType().isa<fir::BoxProcType>())
+ addr = builder.create<fir::BoxAddrOp>(loc, addr);
+ assert((fir::isa_ref_type(addr.getType()) ||
+ addr.getType().isa<mlir::FunctionType>()) &&
+ "expect reference type for address field");
+ assert(fir::isa_derived(componentTy) &&
+ "expect C_PTR, C_FUNPTR to be a record");
+ auto cPtrRecTy = componentTy.cast<fir::RecordType>();
+ llvm::StringRef addrFieldName = Fortran::lower::builtin::cptrFieldName;
+ mlir::Type addrFieldTy = cPtrRecTy.getType(addrFieldName);
+ auto addrField = builder.create<fir::FieldIndexOp>(
+ loc, fieldTy, addrFieldName, componentTy,
+ /*typeParams=*/mlir::ValueRange{});
+ mlir::Value castAddr = builder.createConvert(loc, addrFieldTy, addr);
+ auto undef = builder.create<fir::UndefOp>(loc, componentTy);
+ addr = builder.create<fir::InsertValueOp>(
+ loc, componentTy, undef, castAddr,
+ builder.getArrayAttr(addrField.getAttributes()));
+ res = builder.create<fir::InsertValueOp>(
+ loc, recTy, res, addr, builder.getArrayAttr(field.getAttributes()));
+ return res;
+ }
+
+ mlir::Value val = fir::getBase(genConstantValue(converter, loc, expr));
+ assert(!fir::isa_ref_type(val.getType()) && "expecting a constant value");
+ mlir::Value castVal = builder.createConvert(loc, componentTy, val);
+ res = builder.create<fir::InsertValueOp>(
+ loc, recTy, res, castVal, builder.getArrayAttr(field.getAttributes()));
+ return res;
+}
+
// Generate a StructureConstructor inlined (returns raw fir.type<T> value,
// not the address of a global constant).
static mlir::Value genInlinedStructureCtorLitImpl(
@@ -354,84 +431,75 @@ static mlir::Value genInlinedStructureCtorLitImpl(
const Fortran::evaluate::StructureConstructor &ctor, mlir::Type type) {
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
auto recTy = type.cast<fir::RecordType>();
- auto fieldTy = fir::FieldType::get(type.getContext());
- mlir::Value res = builder.create<fir::UndefOp>(loc, recTy);
-
- for (const auto &[sym, expr] : ctor.values()) {
- // Parent components need more work because they do not appear in the
- // fir.rec type.
- if (sym->test(Fortran::semantics::Symbol::Flag::ParentComp))
- TODO(loc, "parent component in structure constructor");
-
- std::string name = converter.getRecordTypeFieldName(sym);
- mlir::Type componentTy = recTy.getType(name);
- assert(componentTy && "failed to retrieve component");
- // FIXME: type parameters must come from the derived-type-spec
- auto field = builder.create<fir::FieldIndexOp>(
- loc, fieldTy, name, type,
- /*typeParams=*/mlir::ValueRange{} /*TODO*/);
- if (Fortran::semantics::IsAllocatable(sym))
- TODO(loc, "allocatable component in structure constructor");
-
- if (Fortran::semantics::IsPointer(sym)) {
- mlir::Value initialTarget = Fortran::lower::genInitialDataTarget(
- converter, loc, componentTy, expr.value());
- res = builder.create<fir::InsertValueOp>(
- loc, recTy, res, initialTarget,
- builder.getArrayAttr(field.getAttributes()));
- continue;
+ if (!converter.getLoweringOptions().getLowerToHighLevelFIR()) {
+ mlir::Value res = builder.create<fir::UndefOp>(loc, recTy);
+ for (const auto &[sym, expr] : ctor.values()) {
+ // Parent components need more work because they do not appear in the
+ // fir.rec type.
+ if (sym->test(Fortran::semantics::Symbol::Flag::ParentComp))
+ TODO(loc, "parent component in structure constructor");
+ res = genStructureComponentInit(converter, loc, sym, expr.value(), res);
}
+ return res;
+ }
- if (Fortran::lower::isDerivedTypeWithLenParameters(sym))
- TODO(loc, "component with length parameters in structure constructor");
-
- // Special handling for scalar c_ptr/c_funptr constants. The array constant
- // must fall through to genConstantValue() below.
- if (Fortran::semantics::IsBuiltinCPtr(sym) && sym->Rank() == 0 &&
- (Fortran::evaluate::GetLastSymbol(expr.value()) ||
- Fortran::evaluate::IsNullPointer(expr.value()))) {
- // Builtin c_ptr and c_funptr have special handling because designators
- // and NULL() are handled as initial values for them as an extension
- // (otherwise only c_ptr_null/c_funptr_null are allowed and these are
- // replaced by structure constructors by semantics, so GetLastSymbol
- // returns nothing).
-
- // The Ev::Expr is an initializer that is a pointer target (e.g., 'x' or
- // NULL()) that must be inserted into an intermediate cptr record value's
- // address field, which ought to be an intptr_t on the target.
- mlir::Value addr = fir::getBase(Fortran::lower::genExtAddrInInitializer(
- converter, loc, expr.value()));
- if (addr.getType().isa<fir::BoxProcType>())
- addr = builder.create<fir::BoxAddrOp>(loc, addr);
- assert((fir::isa_ref_type(addr.getType()) ||
- addr.getType().isa<mlir::FunctionType>()) &&
- "expect reference type for address field");
- assert(fir::isa_derived(componentTy) &&
- "expect C_PTR, C_FUNPTR to be a record");
- auto cPtrRecTy = componentTy.cast<fir::RecordType>();
- llvm::StringRef addrFieldName = Fortran::lower::builtin::cptrFieldName;
- mlir::Type addrFieldTy = cPtrRecTy.getType(addrFieldName);
- auto addrField = builder.create<fir::FieldIndexOp>(
- loc, fieldTy, addrFieldName, componentTy,
- /*typeParams=*/mlir::ValueRange{});
- mlir::Value castAddr = builder.createConvert(loc, addrFieldTy, addr);
- auto undef = builder.create<fir::UndefOp>(loc, componentTy);
- addr = builder.create<fir::InsertValueOp>(
- loc, componentTy, undef, castAddr,
- builder.getArrayAttr(addrField.getAttributes()));
+ auto fieldTy = fir::FieldType::get(recTy.getContext());
+ mlir::Value res{};
+ // When the first structure component values belong to some parent type PT
+ // and the next values belong to a type extension ET, a new undef for ET must
+ // be created and the previous PT value inserted into it. There may
+ // be empty parent types in between ET and PT, hence the list and while loop.
+ auto insertParentValueIntoExtension = [&](mlir::Type typeExtension) {
+ assert(res && "res must be set");
+ llvm::SmallVector<mlir::Type> parentTypes = {typeExtension};
+ while (true) {
+ fir::RecordType last = mlir::cast<fir::RecordType>(parentTypes.back());
+ mlir::Type next =
+ last.getType(0); // parent components are first in HLFIR.
+ if (next != res.getType())
+ parentTypes.push_back(next);
+ else
+ break;
+ }
+ for (mlir::Type parentType : llvm::reverse(parentTypes)) {
+ auto undef = builder.create<fir::UndefOp>(loc, parentType);
+ fir::RecordType parentRecTy = mlir::cast<fir::RecordType>(parentType);
+ auto field = builder.create<fir::FieldIndexOp>(
+ loc, fieldTy, parentRecTy.getTypeList()[0].first, parentType,
+ /*typeParams=*/mlir::ValueRange{} /*TODO*/);
res = builder.create<fir::InsertValueOp>(
- loc, recTy, res, addr, builder.getArrayAttr(field.getAttributes()));
- continue;
+ loc, recTy, undef, res, builder.getArrayAttr(field.getAttributes()));
}
+ };
- mlir::Value val =
- fir::getBase(genConstantValue(converter, loc, expr.value()));
- assert(!fir::isa_ref_type(val.getType()) && "expecting a constant value");
- mlir::Value castVal = builder.createConvert(loc, componentTy, val);
- res = builder.create<fir::InsertValueOp>(
- loc, recTy, res, castVal, builder.getArrayAttr(field.getAttributes()));
+ const Fortran::semantics::DerivedTypeSpec *curentType = nullptr;
+ for (const auto &[sym, expr] : ctor.values()) {
+ // This TODO is not needed here anymore, but should be removed in a separate
+ // patch.
+ if (sym->test(Fortran::semantics::Symbol::Flag::ParentComp))
+ TODO(loc, "parent component in structure constructor");
+ const Fortran::semantics::DerivedTypeSpec *componentParentType =
+ sym->owner().derivedTypeSpec();
+ assert(componentParentType && "failed to retrieve component parent type");
+ if (!res) {
+ mlir::Type parentType = converter.genType(*componentParentType);
+ curentType = componentParentType;
+ res = builder.create<fir::UndefOp>(loc, parentType);
+ } else if (*componentParentType != *curentType) {
+ mlir::Type parentType = converter.genType(*componentParentType);
+ insertParentValueIntoExtension(parentType);
+ curentType = componentParentType;
+ }
+ res = genStructureComponentInit(converter, loc, sym, expr.value(), res);
}
+
+ if (!res) // structure constructor for empty type.
+ return builder.create<fir::UndefOp>(loc, recTy);
+
+ // The last component may belong to a parent type.
+ if (res.getType() != recTy)
+ insertParentValueIntoExtension(recTy);
return res;
}
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index 236a3639d8dc2e8..4cf29c9aecbf577 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -303,63 +303,19 @@ class HlfirDesignatorBuilder {
}
fir::FortranVariableOpInterface
- gen(const Fortran::evaluate::Component &component,
- bool skipParentComponent = false) {
+ gen(const Fortran::evaluate::Component &component) {
if (Fortran::semantics::IsAllocatableOrPointer(component.GetLastSymbol()))
return genWholeAllocatableOrPointerComponent(component);
- if (component.GetLastSymbol().test(
- Fortran::semantics::Symbol::Flag::ParentComp)) {
- if (skipParentComponent)
- // Inner parent components can be skipped: x%parent_comp%i is equivalent
- // to "x%i" in FIR (all the parent components are part of the FIR type
- // of "x").
- return genDataRefAndSkipParentComponents(component.base());
- // This is a leaf "x%parent_comp" or "x(subscripts)%parent_comp" and
- // cannot be skipped: the designator must be lowered to the parent type.
- // This cannot be represented with an hlfir.designate since "parent_comp"
- // name is meaningless in the fir.record type of "x". Instead, an
- // hlfir.parent_comp is generated.
- fir::FirOpBuilder &builder = getBuilder();
- hlfir::Entity base = genDataRefAndSkipParentComponents(component.base());
- base = derefPointersAndAllocatables(loc, builder, base);
- mlir::Value shape;
- if (base.isArray())
- shape = hlfir::genShape(loc, builder, base);
- const Fortran::semantics::DeclTypeSpec *declTypeSpec =
- component.GetLastSymbol().GetType();
- assert(declTypeSpec && declTypeSpec->AsDerived() &&
- "parent component symbols must have a derived type");
- mlir::Type componentType = Fortran::lower::translateDerivedTypeToFIRType(
- getConverter(), *declTypeSpec->AsDerived());
- mlir::Type resultType =
- changeElementType(base.getElementOrSequenceType(), componentType);
- // Note that the result is monomorphic even if the base is polymorphic:
- // the dynamic type of the parent component reference is the parent type.
- // If the base is an array, it is however most likely not contiguous.
- if (base.isArray() || fir::isRecordWithTypeParameters(componentType))
- resultType = fir::BoxType::get(resultType);
- else
- resultType = fir::ReferenceType::get(resultType);
- if (fir::isRecordWithTypeParameters(componentType))
- TODO(loc, "parent component reference with a parametrized parent type");
- auto parentComp = builder.create<hlfir::ParentComponentOp>(
- loc, resultType, base, shape, /*typeParams=*/mlir::ValueRange{});
- return mlir::cast<fir::FortranVariableOpInterface>(
- parentComp.getOperation());
- }
PartInfo partInfo;
mlir::Type resultType = visit(component, partInfo);
return genDesignate(resultType, partInfo, component);
}
fir::FortranVariableOpInterface
- genDataRefAndSkipParentComponents(const Fortran::evaluate::DataRef &dataRef) {
- return std::visit(Fortran::common::visitors{
- [&](const Fortran::evaluate::Component &component) {
- return gen(component, /*skipParentComponent=*/true);
- },
- [&](const auto &x) { return gen(x); }},
- dataRef.u);
+ gen(const Fortran::evaluate::DataRef &dataRef) {
+ return std::visit(
+ Fortran::common::visitors{[&](const auto &x) { return gen(x); }},
+ dataRef.u);
}
fir::FortranVariableOpInterface
@@ -725,7 +681,7 @@ class HlfirDesignatorBuilder {
// coarray-ref, or another component, this creates another hlfir.designate
// for it. hlfir.designate is not meant to represent more than one
// part-ref.
- partInfo.base = genDataRefAndSkipParentComponents(component.base());
+ partInfo.base = gen(component.base());
// If the base is an allocatable/pointer, dereference it here since the
// component ref designates its target.
partInfo.base =
@@ -739,9 +695,6 @@ class HlfirDesignatorBuilder {
// Lower the information about the component (type, length parameters and
// shape).
const Fortran::semantics::Symbol &componentSym = component.GetLastSymbol();
- assert(
- !componentSym.test(Fortran::semantics::Symbol::Flag::ParentComp) &&
- "parent components are skipped and must not reach visitComponentImpl");
partInfo.componentName = converter.getRecordTypeFieldName(componentSym);
auto recordType =
hlfir::getFortranElementType(baseType).cast<fir::RecordType>();
@@ -1697,7 +1650,7 @@ class HlfirBuilder {
// Construct an entity holding the value specified by the
// StructureConstructor. The initialization of the temporary entity
// is done component by component with the help of HLFIR operations
- // ParentComponentOp, DesignateOp and AssignOp.
+ // DesignateOp and AssignOp.
hlfir::EntityWithAttributes
gen(const Fortran::evaluate::StructureConstructor &ctor) {
mlir::Location loc = getLoc();
@@ -1720,35 +1673,58 @@ class HlfirBuilder {
mlir:...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/69348
More information about the flang-commits
mailing list