Skip to content

[flang] Remove runtime dependence on C++ support for types #134164

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

Merged
merged 1 commit into from
Apr 4, 2025

Conversation

klausler
Copy link
Contributor

@klausler klausler commented Apr 2, 2025

Fortran::runtime::Descriptor::BytesFor() only works for Fortran intrinsic types for which a C++ type counterpart exists, so it crashes on some types that are legitimate Fortran types like REAL(2). Move some logic from Evaluate into a new header in flang/Common, then use it to avoid this needless dependence on C++.

Fortran::runtime::Descriptor::BytesFor() only works for Fortran
intrinsic types for which a C++ type counterpart exists, so it
crashes on some types that are legitimate Fortran types like
REAL(2).  Move some logic from Evaluate into a new header in
flang/Common, then use it to avoid this needless dependence on C++.
@klausler klausler requested a review from vzakhari April 2, 2025 22:29
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:semantics labels Apr 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 2, 2025

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-fir-hlfir

Author: Peter Klausler (klausler)

Changes

Fortran::runtime::Descriptor::BytesFor() only works for Fortran intrinsic types for which a C++ type counterpart exists, so it crashes on some types that are legitimate Fortran types like REAL(2). Move some logic from Evaluate into a new header in flang/Common, then use it to avoid this needless dependence on C++.


Full diff: https://github.com/llvm/llvm-project/pull/134164.diff

12 Files Affected:

  • (modified) flang-rt/lib/runtime/descriptor.cpp (+4-11)
  • (modified) flang/include/flang/Common/Fortran-consts.h (+1-1)
  • (modified) flang/include/flang/Common/real.h (-1)
  • (added) flang/include/flang/Common/type-kinds.h (+63)
  • (modified) flang/include/flang/Evaluate/target.h (+2-1)
  • (modified) flang/include/flang/Evaluate/type.h (+4-25)
  • (modified) flang/lib/Evaluate/target.cpp (+5-14)
  • (modified) flang/lib/Evaluate/tools.cpp (+2-1)
  • (modified) flang/lib/Evaluate/type.cpp (+3-2)
  • (modified) flang/lib/Lower/ConvertType.cpp (+5-4)
  • (modified) flang/lib/Semantics/expression.cpp (+3-1)
  • (modified) flang/lib/Semantics/type.cpp (+2-1)
diff --git a/flang-rt/lib/runtime/descriptor.cpp b/flang-rt/lib/runtime/descriptor.cpp
index a1f4b044bddd7..495e25e96aded 100644
--- a/flang-rt/lib/runtime/descriptor.cpp
+++ b/flang-rt/lib/runtime/descriptor.cpp
@@ -13,8 +13,8 @@
 #include "flang-rt/runtime/derived.h"
 #include "flang-rt/runtime/stat.h"
 #include "flang-rt/runtime/terminator.h"
-#include "flang-rt/runtime/tools.h"
 #include "flang-rt/runtime/type-info.h"
+#include "flang/Common/type-kinds.h"
 #include <cassert>
 #include <cstdlib>
 #include <cstring>
@@ -61,18 +61,11 @@ RT_API_ATTRS void Descriptor::Establish(TypeCode t, std::size_t elementBytes,
   }
 }
 
-namespace {
-template <TypeCategory CAT, int KIND> struct TypeSizeGetter {
-  constexpr RT_API_ATTRS std::size_t operator()() const {
-    CppTypeFor<CAT, KIND> arr[2];
-    return sizeof arr / 2;
-  }
-};
-} // namespace
-
 RT_API_ATTRS std::size_t Descriptor::BytesFor(TypeCategory category, int kind) {
   Terminator terminator{__FILE__, __LINE__};
-  return ApplyType<TypeSizeGetter, std::size_t>(category, kind, terminator);
+  int bytes{common::TypeSizeInBytes(category, kind)};
+  RUNTIME_CHECK(terminator, bytes > 0);
+  return bytes;
 }
 
 RT_API_ATTRS void Descriptor::Establish(TypeCategory c, int kind, void *p,
diff --git a/flang/include/flang/Common/Fortran-consts.h b/flang/include/flang/Common/Fortran-consts.h
index 3ce5b6ac7b686..74ef1c85d2c86 100644
--- a/flang/include/flang/Common/Fortran-consts.h
+++ b/flang/include/flang/Common/Fortran-consts.h
@@ -9,7 +9,7 @@
 #ifndef FORTRAN_COMMON_FORTRAN_CONSTS_H_
 #define FORTRAN_COMMON_FORTRAN_CONSTS_H_
 
-#include "enum-set.h"
+#include "enum-class.h"
 #include <cstdint>
 
 namespace Fortran::common {
diff --git a/flang/include/flang/Common/real.h b/flang/include/flang/Common/real.h
index b47ba46581db6..785cde3236bf4 100644
--- a/flang/include/flang/Common/real.h
+++ b/flang/include/flang/Common/real.h
@@ -13,7 +13,6 @@
 // The various representations are distinguished by their binary precisions
 // (number of explicit significand bits and any implicit MSB in the fraction).
 
-#include "api-attrs.h"
 #include <cinttypes>
 
 namespace Fortran::common {
diff --git a/flang/include/flang/Common/type-kinds.h b/flang/include/flang/Common/type-kinds.h
new file mode 100644
index 0000000000000..4e5c4f69fcc67
--- /dev/null
+++ b/flang/include/flang/Common/type-kinds.h
@@ -0,0 +1,63 @@
+//===-- include/flang/Common/type-kinds.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_COMMON_TYPE_KINDS_H_
+#define FORTRAN_COMMON_TYPE_KINDS_H_
+
+#include "Fortran-consts.h"
+#include "real.h"
+#include <cinttypes>
+
+namespace Fortran::common {
+
+static constexpr int maxKind{16};
+
+// A predicate that is true when a kind value is a kind that could possibly
+// be supported for an intrinsic type category on some target instruction
+// set architecture.
+static constexpr bool IsValidKindOfIntrinsicType(
+    TypeCategory category, std::int64_t kind) {
+  switch (category) {
+  case TypeCategory::Integer:
+  case TypeCategory::Unsigned:
+    return kind == 1 || kind == 2 || kind == 4 || kind == 8 || kind == 16;
+  case TypeCategory::Real:
+  case TypeCategory::Complex:
+    return kind == 2 || kind == 3 || kind == 4 || kind == 8 || kind == 10 ||
+        kind == 16;
+  case TypeCategory::Character:
+    return kind == 1 || kind == 2 || kind == 4;
+  case TypeCategory::Logical:
+    return kind == 1 || kind == 2 || kind == 4 || kind == 8;
+  default:
+    return false;
+  }
+}
+
+static constexpr int TypeSizeInBytes(TypeCategory category, std::int64_t kind) {
+  if (IsValidKindOfIntrinsicType(category, kind)) {
+    if (category == TypeCategory::Real || category == TypeCategory::Complex) {
+      int precision{PrecisionOfRealKind(kind)};
+      int bits{BitsForBinaryPrecision(precision)};
+      if (bits == 80) { // x87 is stored in 16-byte containers
+        bits = 128;
+      }
+      if (category == TypeCategory::Complex) {
+        bits *= 2;
+      }
+      return bits >> 3;
+    } else {
+      return kind;
+    }
+  } else {
+    return -1;
+  }
+}
+
+} // namespace Fortran::common
+#endif // FORTRAN_COMMON_TYPE_KINDS_H_
diff --git a/flang/include/flang/Evaluate/target.h b/flang/include/flang/Evaluate/target.h
index 7b1593ca270db..cc6172b492b3c 100644
--- a/flang/include/flang/Evaluate/target.h
+++ b/flang/include/flang/Evaluate/target.h
@@ -15,6 +15,7 @@
 #include "flang/Common/enum-class.h"
 #include "flang/Common/enum-set.h"
 #include "flang/Common/target-rounding.h"
+#include "flang/Common/type-kinds.h"
 #include "flang/Evaluate/common.h"
 #include "flang/Support/Fortran.h"
 #include <cstdint>
@@ -131,7 +132,7 @@ class TargetCharacteristics {
   const IeeeFeatures &ieeeFeatures() const { return ieeeFeatures_; }
 
 private:
-  static constexpr int maxKind{16};
+  static constexpr int maxKind{common::maxKind};
   std::uint8_t byteSize_[common::TypeCategory_enumSize][maxKind + 1]{};
   std::uint8_t align_[common::TypeCategory_enumSize][maxKind + 1]{};
   bool isBigEndian_{false};
diff --git a/flang/include/flang/Evaluate/type.h b/flang/include/flang/Evaluate/type.h
index cfb162f040e8a..f3bba7790e1a2 100644
--- a/flang/include/flang/Evaluate/type.h
+++ b/flang/include/flang/Evaluate/type.h
@@ -25,6 +25,7 @@
 #include "flang/Common/idioms.h"
 #include "flang/Common/real.h"
 #include "flang/Common/template.h"
+#include "flang/Common/type-kinds.h"
 #include "flang/Support/Fortran-features.h"
 #include "flang/Support/Fortran.h"
 #include <cinttypes>
@@ -62,28 +63,6 @@ using LogicalResult = Type<TypeCategory::Logical, 4>;
 using LargestReal = Type<TypeCategory::Real, 16>;
 using Ascii = Type<TypeCategory::Character, 1>;
 
-// A predicate that is true when a kind value is a kind that could possibly
-// be supported for an intrinsic type category on some target instruction
-// set architecture.
-static constexpr bool IsValidKindOfIntrinsicType(
-    TypeCategory category, std::int64_t kind) {
-  switch (category) {
-  case TypeCategory::Integer:
-  case TypeCategory::Unsigned:
-    return kind == 1 || kind == 2 || kind == 4 || kind == 8 || kind == 16;
-  case TypeCategory::Real:
-  case TypeCategory::Complex:
-    return kind == 2 || kind == 3 || kind == 4 || kind == 8 || kind == 10 ||
-        kind == 16;
-  case TypeCategory::Character:
-    return kind == 1 || kind == 2 || kind == 4;
-  case TypeCategory::Logical:
-    return kind == 1 || kind == 2 || kind == 4 || kind == 8;
-  default:
-    return false;
-  }
-}
-
 // DynamicType is meant to be suitable for use as the result type for
 // GetType() functions and member functions; consequently, it must be
 // capable of being used in a constexpr context.  So it does *not*
@@ -95,7 +74,7 @@ static constexpr bool IsValidKindOfIntrinsicType(
 class DynamicType {
 public:
   constexpr DynamicType(TypeCategory cat, int k) : category_{cat}, kind_{k} {
-    CHECK(IsValidKindOfIntrinsicType(category_, kind_));
+    CHECK(common::IsValidKindOfIntrinsicType(category_, kind_));
   }
   DynamicType(int charKind, const semantics::ParamValue &len);
   // When a known length is presented, resolve it to its effective
@@ -103,7 +82,7 @@ class DynamicType {
   constexpr DynamicType(int k, std::int64_t len)
       : category_{TypeCategory::Character}, kind_{k}, knownLength_{
                                                           len >= 0 ? len : 0} {
-    CHECK(IsValidKindOfIntrinsicType(category_, kind_));
+    CHECK(common::IsValidKindOfIntrinsicType(category_, kind_));
   }
   explicit constexpr DynamicType(
       const semantics::DerivedTypeSpec &dt, bool poly = false)
@@ -360,7 +339,7 @@ using IndirectSubscriptIntegerExpr =
 // category that could possibly be supported on any target.
 template <TypeCategory CATEGORY, int KIND>
 using CategoryKindTuple =
-    std::conditional_t<IsValidKindOfIntrinsicType(CATEGORY, KIND),
+    std::conditional_t<common::IsValidKindOfIntrinsicType(CATEGORY, KIND),
         std::tuple<Type<CATEGORY, KIND>>, std::tuple<>>;
 
 template <TypeCategory CATEGORY, int... KINDS>
diff --git a/flang/lib/Evaluate/target.cpp b/flang/lib/Evaluate/target.cpp
index ba768f38c0ba4..c443278148304 100644
--- a/flang/lib/Evaluate/target.cpp
+++ b/flang/lib/Evaluate/target.cpp
@@ -8,6 +8,7 @@
 
 #include "flang/Evaluate/target.h"
 #include "flang/Common/template.h"
+#include "flang/Common/type-kinds.h"
 #include "flang/Evaluate/common.h"
 #include "flang/Evaluate/type.h"
 
@@ -19,21 +20,11 @@ TargetCharacteristics::TargetCharacteristics() {
   auto enableCategoryKinds{[this](TypeCategory category) {
     for (int kind{1}; kind <= maxKind; ++kind) {
       if (CanSupportType(category, kind)) {
-        auto byteSize{static_cast<std::size_t>(kind)};
-        if (category == TypeCategory::Real ||
-            category == TypeCategory::Complex) {
-          if (kind == 3) {
-            // non-IEEE 16-bit format (truncated 32-bit)
-            byteSize = 2;
-          } else if (kind == 10) {
-            // x87 floating-point
-            // Follow gcc precedent for "long double"
-            byteSize = 16;
-          }
-        }
+        auto byteSize{
+            static_cast<std::size_t>(common::TypeSizeInBytes(category, kind))};
         std::size_t align{byteSize};
         if (category == TypeCategory::Complex) {
-          byteSize = 2 * byteSize;
+          align /= 2;
         }
         EnableType(category, kind, byteSize, align);
       }
@@ -53,7 +44,7 @@ TargetCharacteristics::TargetCharacteristics() {
 
 bool TargetCharacteristics::CanSupportType(
     TypeCategory category, std::int64_t kind) {
-  return IsValidKindOfIntrinsicType(category, kind);
+  return common::IsValidKindOfIntrinsicType(category, kind);
 }
 
 bool TargetCharacteristics::EnableType(common::TypeCategory category,
diff --git a/flang/lib/Evaluate/tools.cpp b/flang/lib/Evaluate/tools.cpp
index fcd6860917247..702711e3cff53 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -8,6 +8,7 @@
 
 #include "flang/Evaluate/tools.h"
 #include "flang/Common/idioms.h"
+#include "flang/Common/type-kinds.h"
 #include "flang/Evaluate/characteristics.h"
 #include "flang/Evaluate/traverse.h"
 #include "flang/Parser/message.h"
@@ -1349,7 +1350,7 @@ template <TypeCategory TO, TypeCategory FROM>
 static std::optional<Expr<SomeType>> DataConstantConversionHelper(
     FoldingContext &context, const DynamicType &toType,
     const Expr<SomeType> &expr) {
-  if (!IsValidKindOfIntrinsicType(FROM, toType.kind())) {
+  if (!common::IsValidKindOfIntrinsicType(FROM, toType.kind())) {
     return std::nullopt;
   }
   DynamicType sizedType{FROM, toType.kind()};
diff --git a/flang/lib/Evaluate/type.cpp b/flang/lib/Evaluate/type.cpp
index c8f75f91ed9c6..5b5f3c2cd0cf0 100644
--- a/flang/lib/Evaluate/type.cpp
+++ b/flang/lib/Evaluate/type.cpp
@@ -8,6 +8,7 @@
 
 #include "flang/Evaluate/type.h"
 #include "flang/Common/idioms.h"
+#include "flang/Common/type-kinds.h"
 #include "flang/Evaluate/expression.h"
 #include "flang/Evaluate/fold.h"
 #include "flang/Evaluate/target.h"
@@ -118,7 +119,7 @@ namespace Fortran::evaluate {
 
 DynamicType::DynamicType(int k, const semantics::ParamValue &pv)
     : category_{TypeCategory::Character}, kind_{k} {
-  CHECK(IsValidKindOfIntrinsicType(category_, kind_));
+  CHECK(common::IsValidKindOfIntrinsicType(category_, kind_));
   if (auto n{ToInt64(pv.GetExplicit())}) {
     knownLength_ = *n > 0 ? *n : 0;
   } else {
@@ -660,7 +661,7 @@ std::optional<DynamicType> DynamicType::From(
   if (const auto *intrinsic{type.AsIntrinsic()}) {
     if (auto kind{ToInt64(intrinsic->kind())}) {
       TypeCategory category{intrinsic->category()};
-      if (IsValidKindOfIntrinsicType(category, *kind)) {
+      if (common::IsValidKindOfIntrinsicType(category, *kind)) {
         if (category == TypeCategory::Character) {
           const auto &charType{type.characterTypeSpec()};
           return DynamicType{static_cast<int>(*kind), charType.length()};
diff --git a/flang/lib/Lower/ConvertType.cpp b/flang/lib/Lower/ConvertType.cpp
index 2fab520e6c475..d45f9e7c0bf1b 100644
--- a/flang/lib/Lower/ConvertType.cpp
+++ b/flang/lib/Lower/ConvertType.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "flang/Lower/ConvertType.h"
+#include "flang/Common/type-kinds.h"
 #include "flang/Lower/AbstractConverter.h"
 #include "flang/Lower/CallInterface.h"
 #include "flang/Lower/ConvertVariable.h"
@@ -32,7 +33,7 @@ using Fortran::common::VectorElementCategory;
 //===--------------------------------------------------------------------===//
 
 static mlir::Type genRealType(mlir::MLIRContext *context, int kind) {
-  if (Fortran::evaluate::IsValidKindOfIntrinsicType(
+  if (Fortran::common::IsValidKindOfIntrinsicType(
           Fortran::common::TypeCategory::Real, kind)) {
     switch (kind) {
     case 2:
@@ -59,7 +60,7 @@ int getIntegerBits() {
 }
 static mlir::Type genIntegerType(mlir::MLIRContext *context, int kind,
                                  bool isUnsigned = false) {
-  if (Fortran::evaluate::IsValidKindOfIntrinsicType(
+  if (Fortran::common::IsValidKindOfIntrinsicType(
           Fortran::common::TypeCategory::Integer, kind)) {
     mlir::IntegerType::SignednessSemantics signedness =
         (isUnsigned ? mlir::IntegerType::SignednessSemantics::Unsigned
@@ -82,7 +83,7 @@ static mlir::Type genIntegerType(mlir::MLIRContext *context, int kind,
 }
 
 static mlir::Type genLogicalType(mlir::MLIRContext *context, int KIND) {
-  if (Fortran::evaluate::IsValidKindOfIntrinsicType(
+  if (Fortran::common::IsValidKindOfIntrinsicType(
           Fortran::common::TypeCategory::Logical, KIND))
     return fir::LogicalType::get(context, KIND);
   return {};
@@ -91,7 +92,7 @@ static mlir::Type genLogicalType(mlir::MLIRContext *context, int KIND) {
 static mlir::Type genCharacterType(
     mlir::MLIRContext *context, int KIND,
     Fortran::lower::LenParameterTy len = fir::CharacterType::unknownLen()) {
-  if (Fortran::evaluate::IsValidKindOfIntrinsicType(
+  if (Fortran::common::IsValidKindOfIntrinsicType(
           Fortran::common::TypeCategory::Character, KIND))
     return fir::CharacterType::get(context, KIND, len);
   return {};
diff --git a/flang/lib/Semantics/expression.cpp b/flang/lib/Semantics/expression.cpp
index f2b9702d7c5a0..e139bda7e4950 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -12,6 +12,7 @@
 #include "resolve-names-utils.h"
 #include "resolve-names.h"
 #include "flang/Common/idioms.h"
+#include "flang/Common/type-kinds.h"
 #include "flang/Evaluate/common.h"
 #include "flang/Evaluate/fold.h"
 #include "flang/Evaluate/tools.h"
@@ -1058,7 +1059,8 @@ MaybeExpr ExpressionAnalyzer::Analyze(const parser::Name &n) {
           if (const semantics::IntrinsicTypeSpec *
               intrinType{typeSpec->AsIntrinsic()}) {
             if (auto k{ToInt64(Fold(semantics::KindExpr{intrinType->kind()}))};
-                k && IsValidKindOfIntrinsicType(TypeCategory::Integer, *k)) {
+                k &&
+                common::IsValidKindOfIntrinsicType(TypeCategory::Integer, *k)) {
               kind = *k;
             }
           }
diff --git a/flang/lib/Semantics/type.cpp b/flang/lib/Semantics/type.cpp
index c5a75c4d619c5..964a37e1c822b 100644
--- a/flang/lib/Semantics/type.cpp
+++ b/flang/lib/Semantics/type.cpp
@@ -9,6 +9,7 @@
 #include "flang/Semantics/type.h"
 #include "check-declarations.h"
 #include "compute-offsets.h"
+#include "flang/Common/type-kinds.h"
 #include "flang/Evaluate/fold.h"
 #include "flang/Evaluate/tools.h"
 #include "flang/Evaluate/type.h"
@@ -125,7 +126,7 @@ void DerivedTypeSpec::EvaluateParameters(SemanticsContext &context) {
         auto restorer{foldingContext.WithPDTInstance(*this)};
         auto folded{Fold(foldingContext, KindExpr{intrinType->kind()})};
         if (auto k{evaluate::ToInt64(folded)}; k &&
-            evaluate::IsValidKindOfIntrinsicType(TypeCategory::Integer, *k)) {
+            common::IsValidKindOfIntrinsicType(TypeCategory::Integer, *k)) {
           parameterKind = static_cast<int>(*k);
         } else {
           messages.Say(

Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@klausler klausler merged commit 262b3f7 into llvm:main Apr 4, 2025
15 checks passed
@klausler klausler deleted the bug442 branch April 4, 2025 15:42
vzakhari added a commit to vzakhari/llvm-project that referenced this pull request Apr 4, 2025
I am having problems building Fortran runtime for CUDA
after llvm#134164. I need more time to investigate it,
but in the meantime including variant.h (or any header
that eventually includes a libcudacxx header) resolves
the issue.
vzakhari added a commit that referenced this pull request Apr 4, 2025
I am having problems building Fortran runtime for CUDA
after #134164. I need more time to investigate it,
but in the meantime including variant.h (or any header
that eventually includes a libcudacxx header) resolves
the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants