Skip to content

Commit 887783e

Browse files
authored
[flang][runtime] Invert component/element loops in assignment (#78341)
The general implementation of intrinsic assignment of derived types in the runtime support library has a doubly-nested loop: an outer loop that traverses the components and inner loops than traverse the array elements. It's done this way to amortize the per-component overhead. However, this turns out to be wrong when the program cares about the order in which defined assignment subroutines are called; the Fortran standard allows less latitude here than we need to invert the ordering in this way when any component is itself an array. So invert the two loops: traverse the array elements, and for each element, traverse its components.
1 parent 78ef032 commit 887783e

File tree

1 file changed

+36
-46
lines changed

1 file changed

+36
-46
lines changed

flang/runtime/assign.cpp

Lines changed: 36 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -393,53 +393,45 @@ RT_API_ATTRS static void Assign(
393393
// Copy the data components (incl. the parent) first.
394394
const Descriptor &componentDesc{updatedToDerived->component()};
395395
std::size_t numComponents{componentDesc.Elements()};
396-
for (std::size_t k{0}; k < numComponents; ++k) {
397-
const auto &comp{
398-
*componentDesc.ZeroBasedIndexedElement<typeInfo::Component>(
399-
k)}; // TODO: exploit contiguity here
400-
// Use PolymorphicLHS for components so that the right things happen
401-
// when the components are polymorphic; when they're not, they're both
402-
// not, and their declared types will match.
403-
int nestedFlags{MaybeReallocate | PolymorphicLHS};
404-
if (flags & ComponentCanBeDefinedAssignment) {
405-
nestedFlags |= CanBeDefinedAssignment | ComponentCanBeDefinedAssignment;
406-
}
407-
switch (comp.genre()) {
408-
case typeInfo::Component::Genre::Data:
409-
if (comp.category() == TypeCategory::Derived) {
410-
StaticDescriptor<maxRank, true, 10 /*?*/> statDesc[2];
411-
Descriptor &toCompDesc{statDesc[0].descriptor()};
412-
Descriptor &fromCompDesc{statDesc[1].descriptor()};
413-
for (std::size_t j{0}; j < toElements; ++j,
414-
to.IncrementSubscripts(toAt), from.IncrementSubscripts(fromAt)) {
396+
for (std::size_t j{0}; j < toElements;
397+
++j, to.IncrementSubscripts(toAt), from.IncrementSubscripts(fromAt)) {
398+
for (std::size_t k{0}; k < numComponents; ++k) {
399+
const auto &comp{
400+
*componentDesc.ZeroBasedIndexedElement<typeInfo::Component>(
401+
k)}; // TODO: exploit contiguity here
402+
// Use PolymorphicLHS for components so that the right things happen
403+
// when the components are polymorphic; when they're not, they're both
404+
// not, and their declared types will match.
405+
int nestedFlags{MaybeReallocate | PolymorphicLHS};
406+
if (flags & ComponentCanBeDefinedAssignment) {
407+
nestedFlags |=
408+
CanBeDefinedAssignment | ComponentCanBeDefinedAssignment;
409+
}
410+
switch (comp.genre()) {
411+
case typeInfo::Component::Genre::Data:
412+
if (comp.category() == TypeCategory::Derived) {
413+
StaticDescriptor<maxRank, true, 10 /*?*/> statDesc[2];
414+
Descriptor &toCompDesc{statDesc[0].descriptor()};
415+
Descriptor &fromCompDesc{statDesc[1].descriptor()};
415416
comp.CreatePointerDescriptor(toCompDesc, to, terminator, toAt);
416417
comp.CreatePointerDescriptor(
417418
fromCompDesc, from, terminator, fromAt);
418419
Assign(toCompDesc, fromCompDesc, terminator, nestedFlags);
419-
}
420-
} else { // Component has intrinsic type; simply copy raw bytes
421-
std::size_t componentByteSize{comp.SizeInBytes(to)};
422-
for (std::size_t j{0}; j < toElements; ++j,
423-
to.IncrementSubscripts(toAt), from.IncrementSubscripts(fromAt)) {
420+
} else { // Component has intrinsic type; simply copy raw bytes
421+
std::size_t componentByteSize{comp.SizeInBytes(to)};
424422
Fortran::runtime::memmove(to.Element<char>(toAt) + comp.offset(),
425423
from.Element<const char>(fromAt) + comp.offset(),
426424
componentByteSize);
427425
}
428-
}
429-
break;
430-
case typeInfo::Component::Genre::Pointer: {
431-
std::size_t componentByteSize{comp.SizeInBytes(to)};
432-
for (std::size_t j{0}; j < toElements; ++j,
433-
to.IncrementSubscripts(toAt), from.IncrementSubscripts(fromAt)) {
426+
break;
427+
case typeInfo::Component::Genre::Pointer: {
428+
std::size_t componentByteSize{comp.SizeInBytes(to)};
434429
Fortran::runtime::memmove(to.Element<char>(toAt) + comp.offset(),
435430
from.Element<const char>(fromAt) + comp.offset(),
436431
componentByteSize);
437-
}
438-
} break;
439-
case typeInfo::Component::Genre::Allocatable:
440-
case typeInfo::Component::Genre::Automatic:
441-
for (std::size_t j{0}; j < toElements; ++j,
442-
to.IncrementSubscripts(toAt), from.IncrementSubscripts(fromAt)) {
432+
} break;
433+
case typeInfo::Component::Genre::Allocatable:
434+
case typeInfo::Component::Genre::Automatic: {
443435
auto *toDesc{reinterpret_cast<Descriptor *>(
444436
to.Element<char>(toAt) + comp.offset())};
445437
const auto *fromDesc{reinterpret_cast<const Descriptor *>(
@@ -470,18 +462,16 @@ RT_API_ATTRS static void Assign(
470462
// The actual deallocation may be avoided, if the existing
471463
// location can be reoccupied.
472464
Assign(*toDesc, *fromDesc, terminator, nestedFlags | DeallocateLHS);
465+
} break;
473466
}
474-
break;
475467
}
476-
}
477-
// Copy procedure pointer components
478-
const Descriptor &procPtrDesc{updatedToDerived->procPtr()};
479-
std::size_t numProcPtrs{procPtrDesc.Elements()};
480-
for (std::size_t k{0}; k < numProcPtrs; ++k) {
481-
const auto &procPtr{
482-
*procPtrDesc.ZeroBasedIndexedElement<typeInfo::ProcPtrComponent>(k)};
483-
for (std::size_t j{0}; j < toElements; ++j, to.IncrementSubscripts(toAt),
484-
from.IncrementSubscripts(fromAt)) {
468+
// Copy procedure pointer components
469+
const Descriptor &procPtrDesc{updatedToDerived->procPtr()};
470+
std::size_t numProcPtrs{procPtrDesc.Elements()};
471+
for (std::size_t k{0}; k < numProcPtrs; ++k) {
472+
const auto &procPtr{
473+
*procPtrDesc.ZeroBasedIndexedElement<typeInfo::ProcPtrComponent>(
474+
k)};
485475
Fortran::runtime::memmove(to.Element<char>(toAt) + procPtr.offset,
486476
from.Element<const char>(fromAt) + procPtr.offset,
487477
sizeof(typeInfo::ProcedurePointer));

0 commit comments

Comments
 (0)