[flang-commits] [flang] [Flang][NFCI] Use abstraction for binary scalar data (PR #212956)
via flang-commits
flang-commits at lists.llvm.org
Thu Jul 30 03:36:15 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-semantics
Author: Michael Kruse (Meinersbur)
<details>
<summary>Changes</summary>
There is currently the assumption that the binary representation of the scalar data classes (Integer, Real, Complex, Logical) is identical to the binary representation of native types. For instance `Integer<64>` can be reinterpreted casted to a `int64_t` and can be serialized using a `memcpy`. This will not be the case anymore with #<!-- -->206907. This first PR introduces `LoadRawBytes` and `StoreRawBytes` abstractions that can be adapted when the binary data layout of the scalar data classes change. No functional change intended.
The binary representation is assumed for these uses:
1. Data serialization in initial-image.h/.cpp
2. Calling native math functions to constant-fold functions such as `sin` in initial-image.cpp. An abstraction layer has been created in host.h/host.cpp to convert between host-native types and the scalar data classes.
Some implementation notes:
a. I originally used `CharacterUtils` but it is in a private header files. Instead I created `CharacterValue` which similar to what #<!-- -->206907 will introduce to allow a consistent interface between the scalar value classes (such as `kind()` returning the current runtime kind)
b `bytesStored` is another such common interface. It it meant to resemble `llvm::DataLayout::getTypeStoreSize` (there is also `getTypeAllocSize`, `llvm::Type::getPrimitiveSizeInBits()`, `llvm::Type::getScalarSizeInBits()`)
c. initial-image.cpp/.h is inconsistent in when it uses number of bytes and when it uses number of characters
d. `Complex<10>` might be inconcistent with the binary representation of `long double` due to padding; host.h/host.cpp acknowledges it, initial-image.h/.cpp ignores it.
---
Full diff: https://github.com/llvm/llvm-project/pull/212956.diff
9 Files Affected:
- (added) flang/include/flang/Evaluate/character-value.h (+71)
- (modified) flang/include/flang/Evaluate/complex.h (+27)
- (modified) flang/include/flang/Evaluate/initial-image.h (+27-19)
- (modified) flang/include/flang/Evaluate/integer.h (+27)
- (modified) flang/include/flang/Evaluate/logical.h (+17)
- (modified) flang/include/flang/Evaluate/real.h (+17)
- (modified) flang/lib/Evaluate/character.h (+29)
- (modified) flang/lib/Evaluate/host.h (+6-11)
- (modified) flang/lib/Evaluate/initial-image.cpp (+17-21)
``````````diff
diff --git a/flang/include/flang/Evaluate/character-value.h b/flang/include/flang/Evaluate/character-value.h
new file mode 100644
index 0000000000000..79feef0406064
--- /dev/null
+++ b/flang/include/flang/Evaluate/character-value.h
@@ -0,0 +1,71 @@
+//===-- include/flang/Evaluate/character-tools.h ----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef FORTRAN_EVALUATE_CHARACTER_TOOLS_H_
+#define FORTRAN_EVALUATE_CHARACTER_TOOLS_H_
+
+#include "flang/Evaluate/type.h"
+#include <string>
+
+namespace Fortran::evaluate {
+
+/// Simple wrapper around a std::string/std:u16string/std::u32string
+template <int KIND> class CharacterValue {
+ using Character = Scalar<Type<TypeCategory::Character, KIND>>;
+ using CharT = typename Character::value_type;
+
+public:
+ CLASS_BOILERPLATE(CharacterValue)
+ CharacterValue(const Character &v) : word_(v) {}
+ CharacterValue(Character &&v) : word_(std::move(v)) {}
+
+ /// Returns the number of characters stored; not the number of bytes
+ auto size() const { return word_.size(); }
+
+ /// Reads a string of characters from \p raw. \p is the number of bytes to
+ /// read; must be a multiple of the size of a single character.
+ static Character FromRawBytes(const void *raw, std::size_t size) {
+ CHECK(size % sizeof(CharT) == 0);
+ Character s;
+ if (size > 0) {
+ s.assign(static_cast<const CharT *>(raw), size / sizeof(CharT));
+ }
+ return s;
+ }
+
+ /// Writes a string of characters to \p dst. \o is the the number of bytes to
+ /// be written; must be a multiple of the size of a single character. If \p s
+ /// is smaller that \p size, the rest of the memory is set to spaces. If \p s
+ /// is shorter than size, only the first characters are written.
+ /// If \p changes points to bool, it will be set to true if any bytes at \p
+ /// dst have changed.
+ void StoreRawBytes(void *dst, std::size_t size, bool *changed = nullptr) {
+ CHECK(size % sizeof(CharT) == 0);
+ if (size > 0) {
+ std::size_t payloadSize{std::min(size, sizeof(CharT) * word_.size())};
+ std::size_t padSize{size - payloadSize};
+
+ Character strWithPadding{word_};
+ strWithPadding.append(padSize / sizeof(CharT), ' ');
+
+ if (changed) {
+ if (std::memcmp(dst, strWithPadding.data(), size) == 0) {
+ return;
+ }
+ *changed = true;
+ }
+ std::memcpy(dst, strWithPadding.data(), size);
+ }
+ }
+
+private:
+ Character word_;
+};
+
+} // namespace Fortran::evaluate
+#endif // FORTRAN_EVALUATE_CHARACTER_TOOLS_H_
diff --git a/flang/include/flang/Evaluate/complex.h b/flang/include/flang/Evaluate/complex.h
index 9781db9a25a64..f68726e60f4d0 100644
--- a/flang/include/flang/Evaluate/complex.h
+++ b/flang/include/flang/Evaluate/complex.h
@@ -98,6 +98,33 @@ template <typename REAL_TYPE> class Complex {
std::string DumpHexadecimal() const;
llvm::raw_ostream &AsFortran(llvm::raw_ostream &, int kind) const;
+ /// Number of bytes that FromRawBytes/StoreRawBytes would accesses.
+ /// Note that this can be different from `sizeof(*this)` because of padding
+ /// the compiler may introduce between the real and imageinary part.
+ /// FromRawBytes/StoreRawBytes assume a packed layout, without padding.
+ constexpr static std::size_t bytesStored() { return 2 * Part::bytesStored(); }
+
+ /// De-serializes a complex from \p raw. \p expectedSize must match the the
+ /// number of bytes to be read.
+ static Complex FromRawBytes(const void *raw, std::size_t expectedSize) {
+ CHECK(bytesStored() == expectedSize);
+ const char *data{static_cast<const char *>(raw)};
+ Part realPart{Part ::FromRawBytes(data, Part::bytesStored())};
+ Part imagPart{
+ Part::FromRawBytes(data + Part::bytesStored(), Part::bytesStored())};
+ return {realPart, imagPart};
+ }
+
+ /// Serializes this complex to \p dst. \p expectedSize must match the the
+ /// number of bytes to be written. If \p changed points to a boolean, it will
+ /// be set to true if any bytes at \p dst have changed.
+ void StoreRawBytes(void *dst, size_t expectedSize, bool *changed) const {
+ CHECK(expectedSize == bytesStored());
+ re_.StoreRawBytes(dst, Part::bytesStored(), changed);
+ im_.StoreRawBytes(static_cast<char *>(dst) + Part::bytesStored(),
+ Part::bytesStored(), changed);
+ }
+
// TODO: unit testing
private:
diff --git a/flang/include/flang/Evaluate/initial-image.h b/flang/include/flang/Evaluate/initial-image.h
index 9a767db95f6c6..2b4c89239887d 100644
--- a/flang/include/flang/Evaluate/initial-image.h
+++ b/flang/include/flang/Evaluate/initial-image.h
@@ -14,12 +14,30 @@
// initializer for a symbol.
#include "expression.h"
+#include "flang/Evaluate/character-value.h"
#include <map>
#include <optional>
#include <vector>
namespace Fortran::evaluate {
+template <typename SCALAR>
+inline void StoreSerialValues(char *dst, llvm::ArrayRef<SCALAR> values,
+ size_t elementSize, bool *changed = nullptr) {
+ for (auto [i, v] : llvm::enumerate(values)) {
+ v.StoreRawBytes(dst + i * elementSize, elementSize, changed);
+ }
+}
+
+template <typename SCALAR>
+inline void LoadSerialValues(
+ const char *src, llvm::MutableArrayRef<SCALAR> values, size_t stride) {
+ for (auto it : llvm::enumerate(values)) {
+ it.value() =
+ SCALAR::FromRawBytes(src + stride * it.index(), SCALAR::bytesStored());
+ }
+}
+
class InitialImage {
public:
enum Result {
@@ -56,14 +74,10 @@ class InitialImage {
return OkNoChange;
} else {
// TODO endianness
- auto *to{&data_.at(offset)};
- const auto *from{&x.values().at(0)};
- if (std::memcmp(to, from, bytes) == 0) {
- return OkNoChange;
- } else {
- std::memcpy(to, from, bytes);
- return Ok;
- }
+ bool changed{false};
+ StoreSerialValues<Scalar<T>>(&data_.at(offset),
+ llvm::ArrayRef<Scalar<T>>(x.values()), *elementBytes, &changed);
+ return changed ? Ok : OkNoChange;
}
}
}
@@ -87,23 +101,17 @@ class InitialImage {
} else {
Result result{OkNoChange};
for (auto at{x.lbounds()}; elements-- > 0; x.IncrementSubscripts(at)) {
- auto scalar{x.At(at)}; // this is a std string; size() in chars
+ CharacterValue<KIND> scalar{x.At(at)};
auto scalarBytes{scalar.size() * KIND};
if (scalarBytes != elementBytes) {
result = LengthMismatch;
}
- // Blank padding when short
- for (; scalarBytes < elementBytes; scalarBytes += KIND) {
- scalar += ' ';
- }
// TODO endianness
auto *to{&data_.at(offset)};
- const auto *from{scalar.data()};
- if (std::memcmp(to, from, elementBytes) != 0) {
- std::memcpy(to, from, elementBytes);
- if (result == OkNoChange) {
- result = Ok;
- }
+ bool changed{false};
+ scalar.StoreRawBytes(to, elementBytes, &changed);
+ if (changed && result == OkNoChange) {
+ result = Ok;
}
offset += elementBytes;
}
diff --git a/flang/include/flang/Evaluate/integer.h b/flang/include/flang/Evaluate/integer.h
index 5953fc81cb111..2b237d19adcac 100644
--- a/flang/include/flang/Evaluate/integer.h
+++ b/flang/include/flang/Evaluate/integer.h
@@ -1017,6 +1017,33 @@ class Integer {
return result;
}
+ /// Number of bytes that FromRawBytes/StoreRawBytes would accesses.
+ static constexpr std::size_t bytesStored() { return sizeof(Integer{}); }
+
+ /// De-serializes an integer from \p raw. \p expectedSize must match the the
+ /// number of bytes to be read.
+ static Integer FromRawBytes(const void *raw, std::size_t expectedSize) {
+ CHECK(expectedSize == bytesStored());
+ Integer result;
+ std::memcpy(&result, raw, bytesStored());
+ return result;
+ }
+
+ /// Serializes this integer to \p dst. \p expectedSize must match the the
+ /// number of bytes to be written. If \p changed points to a boolean, it will
+ /// be set to true if any bytes at \p dst have changed.
+ void StoreRawBytes(
+ void *dst, size_t expectedSize, bool *changed = nullptr) const {
+ CHECK(expectedSize == bytesStored());
+ if (changed) {
+ if (std::memcmp(dst, this, bytesStored()) == 0) {
+ return;
+ }
+ *changed = true;
+ }
+ std::memcpy(dst, this, bytesStored());
+ }
+
private:
// A private constructor, selected by the use of nullptr,
// that is used by member functions when it would be a waste
diff --git a/flang/include/flang/Evaluate/logical.h b/flang/include/flang/Evaluate/logical.h
index 5996853215e30..174c2070a1fa8 100644
--- a/flang/include/flang/Evaluate/logical.h
+++ b/flang/include/flang/Evaluate/logical.h
@@ -93,6 +93,23 @@ template <int BITS, bool IS_LIKE_C = true> class Logical {
return {word_.IEOR(that.word_)};
}
+ /// Number of bytes that FromRawBytes/StoreRawBytes would accesses.
+ static constexpr std::size_t bytesStored() { return Word::bytesStored(); }
+
+ /// De-serializes a logical from \p raw. \p expectedSize must match the the
+ /// number of bytes to be read.
+ static Logical FromRawBytes(const void *raw, std::size_t expectedSize) {
+ return Logical{Word::FromRawBytes(raw, expectedSize)};
+ }
+
+ /// Serializes this logical to \p dst. \p expectedSize must match the the
+ /// number of bytes to be written. If \p changed points to a boolean, it will
+ /// be set to true if any bytes at \p dst have changed.
+ void StoreRawBytes(
+ void *dst, size_t expectedSize, bool *changed = nullptr) const {
+ word_.StoreRawBytes(dst, expectedSize, changed);
+ }
+
private:
static constexpr Word canonicalTrue{IsLikeC ? 1 : -std::uint64_t{1}};
static constexpr Word canonicalFalse{0};
diff --git a/flang/include/flang/Evaluate/real.h b/flang/include/flang/Evaluate/real.h
index 391d4e057f134..aa0311156649a 100644
--- a/flang/include/flang/Evaluate/real.h
+++ b/flang/include/flang/Evaluate/real.h
@@ -452,6 +452,23 @@ template <typename WORD, int PREC> class Real {
llvm::raw_ostream &, int kind, bool minimal = false) const;
std::string AsFortran(int kind, bool minimal = false) const;
+ /// Number of bytes that FromRawBytes/StoreRawBytes would accesses.
+ static constexpr std::size_t bytesStored() { return Word::bytesStored(); }
+
+ /// De-serializes a real from \p raw. \p expectedSize must match the the
+ /// number of bytes to be read.
+ static Real FromRawBytes(const void *raw, std::size_t expectedSize) {
+ return Real{Word::FromRawBytes(raw, expectedSize)};
+ }
+
+ /// Serializes this real to \p dst. \p expectedSize must match the the number
+ /// of bytes to be written. If \p changed points to a boolean, it will be set
+ /// to true if any bytes at \p dst have changed.
+ void StoreRawBytes(
+ void *dst, size_t expectedSize, bool *changed = nullptr) const {
+ word_.StoreRawBytes(dst, expectedSize, changed);
+ }
+
private:
using Significand = Integer<significandBits>; // no implicit bit
diff --git a/flang/lib/Evaluate/character.h b/flang/lib/Evaluate/character.h
index 2d6747741161b..d1c9df5afd89e 100644
--- a/flang/lib/Evaluate/character.h
+++ b/flang/lib/Evaluate/character.h
@@ -111,6 +111,35 @@ template <int KIND> class CharacterUtils {
return str.substr(0, LEN_TRIM(str));
}
+ static Character FromRawBytes(const void *raw, std::size_t size) {
+ CHECK(size % sizeof(CharT) == 0);
+ Character s;
+ if (size > 0) {
+ s.assign(static_cast<const CharT *>(raw), size / sizeof(CharT));
+ }
+ return s;
+ }
+
+ static void StoreRawBytes(void *dst, const Character &s, std::size_t size,
+ bool *changed = nullptr) {
+ CHECK(size % sizeof(CharT) == 0);
+ if (size > 0) {
+ std::size_t payloadSize{std::min(size, sizeof(CharT) * s.size())};
+ std::size_t padSize{size - payloadSize};
+
+ Character strWithPadding{s};
+ strWithPadding.append(padSize / sizeof(CharT), ' ');
+
+ if (changed) {
+ if (std::memcmp(dst, strWithPadding.data(), size) == 0) {
+ return;
+ }
+ *changed = true;
+ }
+ std::memcpy(dst, strWithPadding.data(), size);
+ }
+ }
+
private:
// Following helpers assume that character encodings contain ASCII
static constexpr CharT Space() { return 0x20; }
diff --git a/flang/lib/Evaluate/host.h b/flang/lib/Evaluate/host.h
index 7f6bf76bb5c53..a28e1bbfea61d 100644
--- a/flang/lib/Evaluate/host.h
+++ b/flang/lib/Evaluate/host.h
@@ -73,13 +73,13 @@ template <typename FTN_T>
inline constexpr Scalar<FTN_T> CastHostToFortran(const HostType<FTN_T> &x) {
static_assert(HostTypeExists<FTN_T>());
if constexpr (FTN_T::category == TypeCategory::Complex &&
- sizeof(Scalar<FTN_T>) != sizeof(HostType<FTN_T>)) {
+ Scalar<FTN_T>::bytesStored() != sizeof(HostType<FTN_T>)) {
// X87 is usually padded to 12 or 16bytes. Need to cast piecewise for
// complex
return Scalar<FTN_T>{CastHostToFortran<typename FTN_T::Part>(std::real(x)),
CastHostToFortran<typename FTN_T::Part>(std::imag(x))};
} else {
- return *reinterpret_cast<const Scalar<FTN_T> *>(&x);
+ return Scalar<FTN_T>::FromRawBytes(&x, sizeof(x));
}
}
@@ -91,16 +91,11 @@ inline constexpr HostType<FTN_T> CastFortranToHost(const Scalar<FTN_T> &x) {
using FortranPartType = typename FTN_T::Part;
return HostType<FTN_T>{CastFortranToHost<FortranPartType>(x.REAL()),
CastFortranToHost<FortranPartType>(x.AIMAG())};
- } else if constexpr (std::is_same_v<FTN_T, Type<TypeCategory::Real, 10>>) {
- // x87 80-bit floating-point occupies 16 bytes as a C "long double";
- // copy the data to avoid a legitimate (but benign due to little-endianness)
- // warning from GCC >= 11.2.0.
- HostType<FTN_T> y;
- std::memcpy(&y, &x, sizeof x);
- return y;
} else {
- static_assert(sizeof x == sizeof(HostType<FTN_T>));
- return *reinterpret_cast<const HostType<FTN_T> *>(&x);
+ CHECK(x.bytesStored() == sizeof(HostType<FTN_T>));
+ HostType<FTN_T> result;
+ x.StoreRawBytes(&result, sizeof(result));
+ return result;
}
}
diff --git a/flang/lib/Evaluate/initial-image.cpp b/flang/lib/Evaluate/initial-image.cpp
index 050c55e399b57..d922ae68fdbe1 100644
--- a/flang/lib/Evaluate/initial-image.cpp
+++ b/flang/lib/Evaluate/initial-image.cpp
@@ -164,6 +164,8 @@ class AsConstantHelper {
using Char = typename Scalar::value_type;
auto at{static_cast<std::size_t>(offset_ + j * stride)};
auto chunk{length};
+ // FIXME: chunk is a number of characters, data_.size() is a number of
+ // bytes
if (at + chunk > image_.data_.size()) {
CHECK(padWithZero_);
if (at >= image_.data_.size()) {
@@ -172,10 +174,8 @@ class AsConstantHelper {
chunk = image_.data_.size() - at;
}
}
- if (chunk > 0) {
- const Char *data{reinterpret_cast<const Char *>(&image_.data_[at])};
- typedValue[j].assign(data, chunk);
- }
+ typedValue[j] = CharacterValue<T::kind>::FromRawBytes(
+ &image_.data_[at], chunk * T::kind);
if (chunk < length && padWithZero_) {
typedValue[j].append(length - chunk, Char{});
}
@@ -184,23 +184,19 @@ class AsConstantHelper {
Const{length, std::move(typedValue), std::move(extents_)});
} else {
// Lengthless intrinsic type
- CHECK(sizeof(Scalar) <= stride);
- for (std::size_t j{0}; j < elements; ++j) {
- auto at{static_cast<std::size_t>(offset_ + j * stride)};
- std::size_t chunk{sizeof(Scalar)};
- if (at + chunk > image_.data_.size()) {
- CHECK(padWithZero_);
- if (at >= image_.data_.size()) {
- chunk = 0;
- } else {
- chunk = image_.data_.size() - at;
- }
- }
- // TODO endianness
- if (chunk > 0) {
- std::memcpy(&typedValue[j], &image_.data_[at], chunk);
- }
- }
+ // There is a test (Evaluate/folding10.f90) where this
+ // wants wants to read 2 elements of kind 8 out of an image_.data_ of
+ // size 12. Fortunately, the second element seems to be unused.
+ size_t scalarSize{evaluate::Scalar<T>::bytesStored()};
+ size_t length{std::min(elements,
+ (image_.data_.size() - offset_ - scalarSize + stride) / stride)};
+ CHECK(length == elements || padWithZero_);
+ // TODO endianness
+ LoadSerialValues(image_.data_.data() + offset_,
+ llvm::MutableArrayRef<evaluate::Scalar<T>>(typedValue)
+ .slice(0, length),
+ stride);
+
return AsGenericExpr(Const{std::move(typedValue), std::move(extents_)});
}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/212956
More information about the flang-commits
mailing list