-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[flang][runtime] Preserve type when remapping monomorphic pointers #149427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-flang-fir-hlfir Author: Peter Klausler (klausler) ChangesPointer associations unconditionally update the element byte size and derived type of the pointer's descriptor. This is okay when the pointer is polymorphic, but not when a pointer is associated with an extended type. To communicate this monomorphic case to the runtime, add new entry points, so as to not break forward binary compatibility. Fixes #149353. Full diff: https://github.com/llvm/llvm-project/pull/149427.diff 9 Files Affected:
diff --git a/flang-rt/include/flang-rt/runtime/descriptor.h b/flang-rt/include/flang-rt/runtime/descriptor.h
index 68106f3462c9b..bc5a5b5f14697 100644
--- a/flang-rt/include/flang-rt/runtime/descriptor.h
+++ b/flang-rt/include/flang-rt/runtime/descriptor.h
@@ -478,7 +478,8 @@ class Descriptor {
const SubscriptValue *upper = nullptr,
const SubscriptValue *stride = nullptr);
- RT_API_ATTRS void ApplyMold(const Descriptor &, int rank);
+ RT_API_ATTRS void ApplyMold(
+ const Descriptor &, int rank, bool isMonomorphic = false);
RT_API_ATTRS void Check() const;
diff --git a/flang-rt/lib/runtime/descriptor.cpp b/flang-rt/lib/runtime/descriptor.cpp
index b723acdd27bd5..e735116bc7c28 100644
--- a/flang-rt/lib/runtime/descriptor.cpp
+++ b/flang-rt/lib/runtime/descriptor.cpp
@@ -252,18 +252,21 @@ RT_API_ATTRS bool Descriptor::EstablishPointerSection(const Descriptor &source,
return CFI_section(&raw_, &source.raw_, lower, upper, stride) == CFI_SUCCESS;
}
-RT_API_ATTRS void Descriptor::ApplyMold(const Descriptor &mold, int rank) {
- raw_.elem_len = mold.raw_.elem_len;
+RT_API_ATTRS void Descriptor::ApplyMold(
+ const Descriptor &mold, int rank, bool isMonomorphic) {
raw_.rank = rank;
- raw_.type = mold.raw_.type;
for (int j{0}; j < rank && j < mold.raw_.rank; ++j) {
GetDimension(j) = mold.GetDimension(j);
}
- if (auto *addendum{Addendum()}) {
- if (auto *moldAddendum{mold.Addendum()}) {
- *addendum = *moldAddendum;
- } else {
- INTERNAL_CHECK(!addendum->derivedType());
+ if (!isMonomorphic) {
+ raw_.elem_len = mold.raw_.elem_len;
+ raw_.type = mold.raw_.type;
+ if (auto *addendum{Addendum()}) {
+ if (auto *moldAddendum{mold.Addendum()}) {
+ *addendum = *moldAddendum;
+ } else {
+ INTERNAL_CHECK(!addendum->derivedType());
+ }
}
}
}
diff --git a/flang-rt/lib/runtime/pointer.cpp b/flang-rt/lib/runtime/pointer.cpp
index 04487abd3272e..5aea501e00487 100644
--- a/flang-rt/lib/runtime/pointer.cpp
+++ b/flang-rt/lib/runtime/pointer.cpp
@@ -71,6 +71,20 @@ void RTDEF(PointerAssociate)(Descriptor &pointer, const Descriptor &target) {
pointer.raw().attribute = CFI_attribute_pointer;
}
+void RTDEF(PointerAssociateMonomorphic)(
+ Descriptor &pointer, const Descriptor &target) {
+ DescriptorAddendum *addendum{pointer.Addendum()};
+ const typeInfo::DerivedType *derived{
+ addendum ? addendum->derivedType() : nullptr};
+ std::size_t elementBytes{pointer.ElementBytes()};
+ RTNAME(PointerAssociate)(pointer, target);
+ // Restore the original type information for a monomophic pointer
+ if (addendum) {
+ addendum->set_derivedType(derived);
+ }
+ pointer.raw().elem_len = elementBytes;
+}
+
void RTDEF(PointerAssociateLowerBounds)(Descriptor &pointer,
const Descriptor &target, const Descriptor &lowerBounds) {
pointer = target;
@@ -87,9 +101,9 @@ void RTDEF(PointerAssociateLowerBounds)(Descriptor &pointer,
}
}
-void RTDEF(PointerAssociateRemapping)(Descriptor &pointer,
+static void RT_API_ATTRS PointerRemapping(Descriptor &pointer,
const Descriptor &target, const Descriptor &bounds, const char *sourceFile,
- int sourceLine) {
+ int sourceLine, bool isMonomorphic) {
Terminator terminator{sourceFile, sourceLine};
SubscriptValue byteStride{/*captured from first dimension*/};
std::size_t boundElementBytes{bounds.ElementBytes()};
@@ -99,7 +113,7 @@ void RTDEF(PointerAssociateRemapping)(Descriptor &pointer,
// the ranks may mismatch. Use target as a mold for initializing
// the pointer descriptor.
INTERNAL_CHECK(static_cast<std::size_t>(pointer.rank()) == boundsRank);
- pointer.ApplyMold(target, boundsRank);
+ pointer.ApplyMold(target, boundsRank, isMonomorphic);
pointer.set_base_addr(target.raw().base_addr);
pointer.raw().attribute = CFI_attribute_pointer;
for (unsigned j{0}; j < boundsRank; ++j) {
@@ -124,6 +138,19 @@ void RTDEF(PointerAssociateRemapping)(Descriptor &pointer,
}
}
+void RTDEF(PointerAssociateRemapping)(Descriptor &pointer,
+ const Descriptor &target, const Descriptor &bounds, const char *sourceFile,
+ int sourceLine) {
+ PointerRemapping(
+ pointer, target, bounds, sourceFile, sourceLine, /*isMonomorphic=*/false);
+}
+void RTDEF(PointerAssociateRemappingMonomorphic)(Descriptor &pointer,
+ const Descriptor &target, const Descriptor &bounds, const char *sourceFile,
+ int sourceLine) {
+ PointerRemapping(
+ pointer, target, bounds, sourceFile, sourceLine, /*isMonomorphic=*/true);
+}
+
RT_API_ATTRS void *AllocateValidatedPointerPayload(
std::size_t byteSize, int allocatorIdx) {
// Add space for a footer to validate during deallocation.
diff --git a/flang/include/flang/Lower/Runtime.h b/flang/include/flang/Lower/Runtime.h
index 77e98a1e019e7..6c6b27fafefd4 100644
--- a/flang/include/flang/Lower/Runtime.h
+++ b/flang/include/flang/Lower/Runtime.h
@@ -67,10 +67,11 @@ void genUnlockStatement(AbstractConverter &, const parser::UnlockStmt &);
void genPauseStatement(AbstractConverter &, const parser::PauseStmt &);
void genPointerAssociate(fir::FirOpBuilder &, mlir::Location,
- mlir::Value pointer, mlir::Value target);
+ mlir::Value pointer, mlir::Value target,
+ bool isMonomorphic);
void genPointerAssociateRemapping(fir::FirOpBuilder &, mlir::Location,
mlir::Value pointer, mlir::Value target,
- mlir::Value bounds);
+ mlir::Value bounds, bool isMonomorphic);
void genPointerAssociateLowerBounds(fir::FirOpBuilder &, mlir::Location,
mlir::Value pointer, mlir::Value target,
mlir::Value lbounds);
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Intrinsics.h b/flang/include/flang/Optimizer/Builder/Runtime/Intrinsics.h
index 9ca4b2baeaa65..a01dad8e40d0a 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/Intrinsics.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/Intrinsics.h
@@ -34,10 +34,11 @@ mlir::Value genAssociated(fir::FirOpBuilder &, mlir::Location,
mlir::Value pointer, mlir::Value target);
void genPointerAssociate(fir::FirOpBuilder &, mlir::Location,
- mlir::Value pointer, mlir::Value target);
+ mlir::Value pointer, mlir::Value target,
+ bool isMonomorphic);
void genPointerAssociateRemapping(fir::FirOpBuilder &, mlir::Location,
mlir::Value pointer, mlir::Value target,
- mlir::Value bounds);
+ mlir::Value bounds, bool isMonomorphic);
mlir::Value genCpuTime(fir::FirOpBuilder &, mlir::Location);
void genDateAndTime(fir::FirOpBuilder &, mlir::Location,
diff --git a/flang/include/flang/Runtime/pointer.h b/flang/include/flang/Runtime/pointer.h
index 83472ee59d2ab..9bca99cc3d1d9 100644
--- a/flang/include/flang/Runtime/pointer.h
+++ b/flang/include/flang/Runtime/pointer.h
@@ -52,7 +52,10 @@ void RTDECL(PointerAssociateScalar)(Descriptor &, void *);
// Associates a pointer with a target of the same rank, possibly with new lower
// bounds, which are passed in a vector whose length must equal the rank.
+// Use the Monomorphic form if the pointer's type shouldn't change.
void RTDECL(PointerAssociate)(Descriptor &, const Descriptor &target);
+void RTDECL(PointerAssociateMonomorphic)(
+ Descriptor &, const Descriptor &target);
void RTDECL(PointerAssociateLowerBounds)(
Descriptor &, const Descriptor &target, const Descriptor &lowerBounds);
@@ -62,6 +65,9 @@ void RTDECL(PointerAssociateLowerBounds)(
void RTDECL(PointerAssociateRemapping)(Descriptor &, const Descriptor &target,
const Descriptor &bounds, const char *sourceFile = nullptr,
int sourceLine = 0);
+void RTDECL(PointerAssociateRemappingMonomorphic)(Descriptor &,
+ const Descriptor &target, const Descriptor &bounds,
+ const char *sourceFile = nullptr, int sourceLine = 0);
// Data pointer allocation and deallocation
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 33c1f1e7a3c3a..f4da50174464a 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -4517,7 +4517,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
boundsDesc);
return;
}
- Fortran::lower::genPointerAssociate(*builder, loc, lhs, rhs);
+ Fortran::lower::genPointerAssociate(*builder, loc, lhs, rhs,
+ /*isMonomorphic=*/false);
return;
}
@@ -4703,8 +4704,9 @@ class FirConverter : public Fortran::lower::AbstractConverter {
mlir::Value lhs = lhsMutableBox.getAddr();
mlir::Value rhs = fir::getBase(genExprBox(loc, assign.rhs, stmtCtx));
mlir::Value boundsDesc = createBoundArray(lbounds, ubounds, loc);
- Fortran::lower::genPointerAssociateRemapping(*builder, loc, lhs, rhs,
- boundsDesc);
+ Fortran::lower::genPointerAssociateRemapping(
+ *builder, loc, lhs, rhs, boundsDesc,
+ lhsType && !lhsType->IsPolymorphic());
return;
}
if (!lowerToHighLevelFIR() && explicitIterationSpace()) {
diff --git a/flang/lib/Lower/Runtime.cpp b/flang/lib/Lower/Runtime.cpp
index 2be5ef76e46b8..bf3f5305c17d8 100644
--- a/flang/lib/Lower/Runtime.cpp
+++ b/flang/lib/Lower/Runtime.cpp
@@ -205,22 +205,28 @@ void Fortran::lower::genPauseStatement(
void Fortran::lower::genPointerAssociate(fir::FirOpBuilder &builder,
mlir::Location loc,
mlir::Value pointer,
- mlir::Value target) {
+ mlir::Value target,
+ bool isMonomorphic) {
mlir::func::FuncOp func =
- fir::runtime::getRuntimeFunc<mkRTKey(PointerAssociate)>(loc, builder);
+ isMonomorphic
+ ? fir::runtime::getRuntimeFunc<mkRTKey(PointerAssociateMonomorphic)>(
+ loc, builder)
+ : fir::runtime::getRuntimeFunc<mkRTKey(PointerAssociate)>(loc,
+ builder);
llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments(
builder, loc, func.getFunctionType(), pointer, target);
builder.create<fir::CallOp>(loc, func, args);
}
-void Fortran::lower::genPointerAssociateRemapping(fir::FirOpBuilder &builder,
- mlir::Location loc,
- mlir::Value pointer,
- mlir::Value target,
- mlir::Value bounds) {
+void Fortran::lower::genPointerAssociateRemapping(
+ fir::FirOpBuilder &builder, mlir::Location loc, mlir::Value pointer,
+ mlir::Value target, mlir::Value bounds, bool isMonomorphic) {
mlir::func::FuncOp func =
- fir::runtime::getRuntimeFunc<mkRTKey(PointerAssociateRemapping)>(loc,
- builder);
+ isMonomorphic
+ ? fir::runtime::getRuntimeFunc<mkRTKey(
+ PointerAssociateRemappingMonomorphic)>(loc, builder)
+ : fir::runtime::getRuntimeFunc<mkRTKey(PointerAssociateRemapping)>(
+ loc, builder);
auto fTy = func.getFunctionType();
auto sourceFile = fir::factory::locationToFilename(builder, loc);
auto sourceLine =
diff --git a/flang/test/Lower/polymorphic.f90 b/flang/test/Lower/polymorphic.f90
index b7be5f685d9e3..1c1bc78e9b34a 100644
--- a/flang/test/Lower/polymorphic.f90
+++ b/flang/test/Lower/polymorphic.f90
@@ -178,6 +178,17 @@ subroutine polymorphic_to_nonpolymorphic(p)
! CHECK-LABEL: func.func @_QMpolymorphic_testPpolymorphic_to_nonpolymorphic
! Just checking that FIR is generated without error.
+ subroutine nonpolymorphic_to_polymorphic(p, t)
+ type p1
+ end type
+ type(p1), pointer :: p(:)
+ class(p1), target :: t(:)
+ p(0:1) => t
+ end subroutine
+
+! CHECK-LABEL: func.func @_QMpolymorphic_testPnonpolymorphic_to_polymorphic
+! CHECK: fir.call @_FortranAPointerAssociateRemappingMonomorphic
+
! Test that lowering does not crash for function return with unlimited
! polymoprhic value.
|
Pointer remappings unconditionally update the element byte size and derived type of the pointer's descriptor. This is okay when the pointer is polymorphic, but not when a pointer is associated with an extended type. To communicate this monomorphic case to the runtime, add a new entry point so as to not break forward binary compatibility. Fixes llvm#149353.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Pointer remappings unconditionally update the element byte size and derived type of the pointer's descriptor. This is okay when the pointer is polymorphic, but not when a pointer is associated with an extended type.
To communicate this monomorphic case to the runtime, add a new entry point so as to not break forward binary compatibility.