[flang][runtime][NFC] Allow different memmove function in assign #114301
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clementval
changed the title
llvmbot
added
flang:runtime
flang
|
@llvm/pr-subscribers-flang-runtime Author: Valentin Clement (バレンタイン クレメン) (clementval) Changes
Full diff: https://github.com/llvm/llvm-project/pull/114301.diff 2 Files Affected:
diff --git a/flang/include/flang/Runtime/assign.h b/flang/include/flang/Runtime/assign.h
index a1cc9eaf4355f6..ddd4d46f99ce3d 100644
--- a/flang/include/flang/Runtime/assign.h
+++ b/flang/include/flang/Runtime/assign.h
@@ -24,11 +24,34 @@
#define FORTRAN_RUNTIME_ASSIGN_H_
#include "flang/Runtime/entry-names.h"
+#include "flang/Runtime/freestanding-tools.h"
namespace Fortran::runtime {
class Descriptor;
+class Terminator;
+
+enum AssignFlags {
+ NoAssignFlags = 0,
+ MaybeReallocate = 1 << 0,
+ NeedFinalization = 1 << 1,
+ CanBeDefinedAssignment = 1 << 2,
+ ComponentCanBeDefinedAssignment = 1 << 3,
+ ExplicitLengthCharacterLHS = 1 << 4,
+ PolymorphicLHS = 1 << 5,
+ DeallocateLHS = 1 << 6
+};
+
+using MemmoveFct = void *(*)(void *, const void *, std::size_t);
+
+static void *MemmoveWrapper(void *dest, const void *src, std::size_t count) {
+ return Fortran::runtime::memmove(dest, src, count);
+}
+
+RT_API_ATTRS void Assign(Descriptor &to, const Descriptor &from,
+ Terminator &terminator, int flags, MemmoveFct memmoveFct = &MemmoveWrapper);
extern "C" {
+
// API for lowering assignment
void RTDECL(Assign)(Descriptor &to, const Descriptor &from,
const char *sourceFile = nullptr, int sourceLine = 0);
diff --git a/flang/runtime/assign.cpp b/flang/runtime/assign.cpp
index d558ada51cd21a..8f31fc4d127168 100644
--- a/flang/runtime/assign.cpp
+++ b/flang/runtime/assign.cpp
@@ -17,17 +17,6 @@
namespace Fortran::runtime {
-enum AssignFlags {
- NoAssignFlags = 0,
- MaybeReallocate = 1 << 0,
- NeedFinalization = 1 << 1,
- CanBeDefinedAssignment = 1 << 2,
- ComponentCanBeDefinedAssignment = 1 << 3,
- ExplicitLengthCharacterLHS = 1 << 4,
- PolymorphicLHS = 1 << 5,
- DeallocateLHS = 1 << 6
-};
-
// Predicate: is the left-hand side of an assignment an allocated allocatable
// that must be deallocated?
static inline RT_API_ATTRS bool MustDeallocateLHS(
@@ -250,8 +239,8 @@ static RT_API_ATTRS void BlankPadCharacterAssignment(Descriptor &to,
// of elements, but their shape need not to conform (the assignment is done in
// element sequence order). This facilitates some internal usages, like when
// dealing with array constructors.
-RT_API_ATTRS static void Assign(
- Descriptor &to, const Descriptor &from, Terminator &terminator, int flags) {
+RT_API_ATTRS void Assign(Descriptor &to, const Descriptor &from,
+ Terminator &terminator, int flags, MemmoveFct memmoveFct) {
bool mustDeallocateLHS{(flags & DeallocateLHS) ||
MustDeallocateLHS(to, from, terminator, flags)};
DescriptorAddendum *toAddendum{to.Addendum()};
@@ -423,14 +412,14 @@ RT_API_ATTRS static void Assign(
Assign(toCompDesc, fromCompDesc, terminator, nestedFlags);
} else { // Component has intrinsic type; simply copy raw bytes
std::size_t componentByteSize{comp.SizeInBytes(to)};
- Fortran::runtime::memmove(to.Element<char>(toAt) + comp.offset(),
+ memmoveFct(to.Element<char>(toAt) + comp.offset(),
from.Element<const char>(fromAt) + comp.offset(),
componentByteSize);
}
break;
case typeInfo::Component::Genre::Pointer: {
std::size_t componentByteSize{comp.SizeInBytes(to)};
- Fortran::runtime::memmove(to.Element<char>(toAt) + comp.offset(),
+ memmoveFct(to.Element<char>(toAt) + comp.offset(),
from.Element<const char>(fromAt) + comp.offset(),
componentByteSize);
} break;
@@ -476,14 +465,14 @@ RT_API_ATTRS static void Assign(
const auto &procPtr{
*procPtrDesc.ZeroBasedIndexedElement<typeInfo::ProcPtrComponent>(
k)};
- Fortran::runtime::memmove(to.Element<char>(toAt) + procPtr.offset,
+ memmoveFct(to.Element<char>(toAt) + procPtr.offset,
from.Element<const char>(fromAt) + procPtr.offset,
sizeof(typeInfo::ProcedurePointer));
}
}
} else { // intrinsic type, intrinsic assignment
if (isSimpleMemmove()) {
- Fortran::runtime::memmove(to.raw().base_addr, from.raw().base_addr,
+ memmoveFct(to.raw().base_addr, from.raw().base_addr,
toElements * toElementBytes);
} else if (toElementBytes > fromElementBytes) { // blank padding
switch (to.type().raw()) {
@@ -507,8 +496,8 @@ RT_API_ATTRS static void Assign(
} else { // elemental copies, possibly with character truncation
for (std::size_t n{toElements}; n-- > 0;
to.IncrementSubscripts(toAt), from.IncrementSubscripts(fromAt)) {
- Fortran::runtime::memmove(to.Element<char>(toAt),
- from.Element<const char>(fromAt), toElementBytes);
+ memmoveFct(to.Element<char>(toAt), from.Element<const char>(fromAt),
+ toElementBytes);
}
}
}
|
clementval
commented
Oct 31, 2024
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
clementval
commented
Oct 31, 2024
Renaud-K
approved these changes
Oct 31, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/152/builds/583 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/9626 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/11656 Here is the relevant piece of the build log for the reference |
clementval
added a commit
that referenced
this pull request
Nov 1, 2024
clementval
added a commit
that referenced
this pull request
Nov 1, 2024
klausler
reviewed
Nov 1, 2024
|
|
||
| using MemmoveFct = void *(*)(void *, const void *, std::size_t); | ||
|
|
||
| static RT_API_ATTRS void *MemmoveWrapper( |
There was a problem hiding this comment.
Why is the wrapper necessary?
There was a problem hiding this comment.
When building on the device the compiler is complaining when we address directly std:: function. Same was done in the allocator-registry
llvm-project/flang/include/flang/Runtime/allocator-registry.h
Lines 37 to 40 in c1df376
There was a problem hiding this comment.
This is the one that landed 7792dbe. Wrapper is not used on the host build.
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
…m#114301) - Add a parameter to the `Assign` function to be able to use a different `memmove` function. This is preparatory work to be able to use the `Assign` function between host and device data. - Expose the `Assign` function so it can be used from different files. - The new `memmoveFct` is not used in `BlankPadCharacterAssignment` yet since it is not clear if there is a need. It will be updated in case it is needed.
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
…m#114301) - Add a parameter to the `Assign` function to be able to use a different `memmove` function. This is preparatory work to be able to use the `Assign` function between host and device data. - Expose the `Assign` function so it can be used from different files. - The new `memmoveFct` is not used in `BlankPadCharacterAssignment` yet since it is not clear if there is a need. It will be updated in case it is needed.
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a parameter to the
Assignfunction to be able to use a differentmemmovefunction. This is preparatory work to be able to use theAssignfunction between host and device data.Expose the
Assignfunction so it can be used from different files.The new
memmoveFctis not used inBlankPadCharacterAssignmentyet since it is not clear if there is a need. It will be updated in case it is needed.