[Flang][NFC] Split common headers to reduce dependencies. #110244

Meinersbur · 2024-09-27T11:17:30Z

Fortran.h and target.h are defining symbols where some are used by both, the Fortran runtime (Flang-RT) and Fortran compiler (Flang), and others are used by Flang only. With the upcoming refactoring of the Fortran runtime into its own subproject (#110217), move the declarations that are used by both into new headers to minimize the amount of code that will need to be shared by Flang-RT and Flang.

Details:

Fortran.h: Flang-RT only uses some enum definitions out of this file, but not AsFortran which is defined in Fortran.cpp. Moving the enums into Fortran-consts.h allows keeping Fortran.cpp within Flang.
target.h: Contains some floating-point definitions that is used by the non-GTest unittests in fp-testing.h. Flang-RT also uses some non-GTest as well. Moving those definitions avoids the dependence on the entire FortranEvaluate library.

github-actions · 2024-09-27T11:21:30Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvmbot · 2024-10-07T20:48:23Z

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-runtime

Author: Michael Kruse (Meinersbur)

Changes

Fortran.h and target.h are defining symbols where some are used by both, the Fortran runtime and Flang, and others are used by Flang only. With the upcoming refactoring of the Fortran runtime into its own subproject (#110217), move the declarations that are used by both into new headers to minimize the amount of code that will need to be shared by FortranRuntime and Flang.

Details:

Fortran.h: FortranRuntime only uses some enum definitions out of this file, but not AsFortran which is defined in Fortran.cpp. Moving the enums into Fortran-consts.h allows keeping Fortran.cpp within Flang.
target.h: Contains some floating-point definitions that is used by the non-GTest unittests in fp-testing.h. FortranRuntime also uses some non-GTest as well. Moving those definitions avoids the dependence on the entire FortranEvaluate library.

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

14 Files Affected:

(added) flang/include/flang/Common/Fortran-consts.h (+44)
(modified) flang/include/flang/Common/Fortran.h (+1-25)
(modified) flang/include/flang/Common/format.h (+1-1)
(added) flang/include/flang/Common/target-rounding.h (+37)
(modified) flang/include/flang/Evaluate/common.h (+3-5)
(modified) flang/include/flang/Evaluate/target.h (+2-13)
(modified) flang/include/flang/Runtime/cpp-type.h (+1-1)
(modified) flang/include/flang/Runtime/type-code.h (+1-1)
(modified) flang/runtime/format.h (+1-1)
(modified) flang/runtime/non-tbp-dio.h (+2-1)
(modified) flang/runtime/type-info.h (+1-1)
(modified) flang/unittests/Evaluate/fp-testing.cpp (+1-1)
(modified) flang/unittests/Evaluate/fp-testing.h (+3-3)
(modified) flang/unittests/Runtime/Complex.cpp (+1-1)

diff --git a/flang/include/flang/Common/Fortran-consts.h b/flang/include/flang/Common/Fortran-consts.h
new file mode 100644
index 00000000000000..eedcdae335c400
--- /dev/null
+++ b/flang/include/flang/Common/Fortran-consts.h
@@ -0,0 +1,44 @@
+//===-- include/flang/Common/Fortran-consts.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_FORTRAN_CONSTS_H_
+#define FORTRAN_COMMON_FORTRAN_CONSTS_H_
+
+#include "flang/Common/enum-class.h"
+#include <cstdint>
+
+namespace Fortran::common {
+
+// Fortran has five kinds of intrinsic data types, plus the derived types.
+ENUM_CLASS(TypeCategory, Integer, Real, Complex, Character, Logical, Derived)
+ENUM_CLASS(VectorElementCategory, Integer, Unsigned, Real)
+
+ENUM_CLASS(IoStmtKind, None, Backspace, Close, Endfile, Flush, Inquire, Open,
+    Print, Read, Rewind, Wait, Write)
+
+// Defined I/O variants
+ENUM_CLASS(
+    DefinedIo, ReadFormatted, ReadUnformatted, WriteFormatted, WriteUnformatted)
+
+// Fortran arrays may have up to 15 dimensions (See Fortran 2018 section 5.4.6).
+static constexpr int maxRank{15};
+
+// Floating-point rounding modes; these are packed into a byte to save
+// room in the runtime's format processing context structure.  These
+// enumerators are defined with the corresponding values returned from
+// llvm.get.rounding.
+enum class RoundingMode : std::uint8_t {
+  ToZero, // ROUND=ZERO, RZ - truncation
+  TiesToEven, // ROUND=NEAREST, RN - default IEEE rounding
+  Up, // ROUND=UP, RU
+  Down, // ROUND=DOWN, RD
+  TiesAwayFromZero, // ROUND=COMPATIBLE, RC - ties round away from zero
+};
+
+} // namespace Fortran::common
+#endif /* FORTRAN_COMMON_FORTRAN_CONSTS_H_ */
diff --git a/flang/include/flang/Common/Fortran.h b/flang/include/flang/Common/Fortran.h
index 5b2ed43a8f99c0..623607d4fad267 100644
--- a/flang/include/flang/Common/Fortran.h
+++ b/flang/include/flang/Common/Fortran.h
@@ -14,6 +14,7 @@
 
 #include "enum-set.h"
 #include "idioms.h"
+#include "flang/Common/Fortran-consts.h"
 #include <cinttypes>
 #include <optional>
 #include <string>
@@ -21,10 +22,6 @@
 namespace Fortran::common {
 class LanguageFeatureControl;
 
-// Fortran has five kinds of intrinsic data types, plus the derived types.
-ENUM_CLASS(TypeCategory, Integer, Real, Complex, Character, Logical, Derived)
-ENUM_CLASS(VectorElementCategory, Integer, Unsigned, Real)
-
 constexpr bool IsNumericTypeCategory(TypeCategory category) {
   return category == TypeCategory::Integer || category == TypeCategory::Real ||
       category == TypeCategory::Complex;
@@ -47,9 +44,6 @@ const char *AsFortran(RelationalOperator);
 
 ENUM_CLASS(Intent, Default, In, Out, InOut)
 
-ENUM_CLASS(IoStmtKind, None, Backspace, Close, Endfile, Flush, Inquire, Open,
-    Print, Read, Rewind, Wait, Write)
-
 // Union of specifiers for all I/O statements.
 ENUM_CLASS(IoSpecKind, Access, Action, Advance, Asynchronous, Blank, Decimal,
     Delim, Direct, Encoding, End, Eor, Err, Exist, File, Fmt, Form, Formatted,
@@ -61,29 +55,11 @@ ENUM_CLASS(IoSpecKind, Access, Action, Advance, Asynchronous, Blank, Decimal,
     Dispose, // nonstandard
 )
 
-// Defined I/O variants
-ENUM_CLASS(
-    DefinedIo, ReadFormatted, ReadUnformatted, WriteFormatted, WriteUnformatted)
 const char *AsFortran(DefinedIo);
 
-// Floating-point rounding modes; these are packed into a byte to save
-// room in the runtime's format processing context structure.  These
-// enumerators are defined with the corresponding values returned from
-// llvm.get.rounding.
-enum class RoundingMode : std::uint8_t {
-  ToZero, // ROUND=ZERO, RZ - truncation
-  TiesToEven, // ROUND=NEAREST, RN - default IEEE rounding
-  Up, // ROUND=UP, RU
-  Down, // ROUND=DOWN, RD
-  TiesAwayFromZero, // ROUND=COMPATIBLE, RC - ties round away from zero
-};
-
 // Fortran label. Must be in [1..99999].
 using Label = std::uint64_t;
 
-// Fortran arrays may have up to 15 dimensions (See Fortran 2018 section 5.4.6).
-static constexpr int maxRank{15};
-
 // CUDA subprogram attribute combinations
 ENUM_CLASS(CUDASubprogramAttrs, Host, Device, HostDevice, Global, Grid_Global)
 
diff --git a/flang/include/flang/Common/format.h b/flang/include/flang/Common/format.h
index 2374ff6983cf41..138e84b72b733d 100644
--- a/flang/include/flang/Common/format.h
+++ b/flang/include/flang/Common/format.h
@@ -10,7 +10,7 @@
 #define FORTRAN_COMMON_FORMAT_H_
 
 #include "enum-set.h"
-#include "flang/Common/Fortran.h"
+#include "flang/Common/Fortran-consts.h"
 #include <cstring>
 
 // Define a FormatValidator class template to validate a format expression
diff --git a/flang/include/flang/Common/target-rounding.h b/flang/include/flang/Common/target-rounding.h
new file mode 100644
index 00000000000000..c0c9f6c49b26a2
--- /dev/null
+++ b/flang/include/flang/Common/target-rounding.h
@@ -0,0 +1,37 @@
+//===-- include/flang/Common/target-rounding.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_TARGET_ROUNDING_H_
+#define FORTRAN_COMMON_TARGET_ROUNDING_H_
+
+#include "flang/Common/Fortran-consts.h"
+#include "flang/Common/enum-set.h"
+
+namespace Fortran::common {
+
+// Floating-point rounding control
+struct Rounding {
+  common::RoundingMode mode{common::RoundingMode::TiesToEven};
+  // When set, emulate status flag behavior peculiar to x86
+  // (viz., fail to set the Underflow flag when an inexact product of a
+  // multiplication is rounded up to a normal number from a subnormal
+  // in some rounding modes)
+#if __x86_64__ || __riscv || __loongarch__
+  bool x86CompatibleBehavior{true};
+#else
+  bool x86CompatibleBehavior{false};
+#endif
+};
+
+// These are ordered like the bits in a common fenv.h header file.
+ENUM_CLASS(RealFlag, InvalidArgument, Denorm, DivideByZero, Overflow, Underflow,
+    Inexact)
+using RealFlags = common::EnumSet<RealFlag, RealFlag_enumSize>;
+
+} // namespace Fortran::common
+#endif /* FORTRAN_COMMON_TARGET_ROUNDING_H_ */
diff --git a/flang/include/flang/Evaluate/common.h b/flang/include/flang/Evaluate/common.h
index d493e5fe044173..915e95169c7f81 100644
--- a/flang/include/flang/Evaluate/common.h
+++ b/flang/include/flang/Evaluate/common.h
@@ -16,6 +16,7 @@
 #include "flang/Common/idioms.h"
 #include "flang/Common/indirection.h"
 #include "flang/Common/restorer.h"
+#include "flang/Common/target-rounding.h"
 #include "flang/Parser/char-block.h"
 #include "flang/Parser/message.h"
 #include <cinttypes>
@@ -32,6 +33,8 @@ class IntrinsicProcTable;
 class TargetCharacteristics;
 
 using common::ConstantSubscript;
+using common::RealFlag;
+using common::RealFlags;
 using common::RelationalOperator;
 
 // Integers are always ordered; reals may not be.
@@ -128,11 +131,6 @@ static constexpr bool Satisfies(RelationalOperator op, Relation relation) {
   return false; // silence g++ warning
 }
 
-// These are ordered like the bits in a common fenv.h header file.
-ENUM_CLASS(RealFlag, InvalidArgument, Denorm, DivideByZero, Overflow, Underflow,
-    Inexact)
-using RealFlags = common::EnumSet<RealFlag, RealFlag_enumSize>;
-
 template <typename A> struct ValueWithRealFlags {
   A AccumulateFlags(RealFlags &f) {
     f |= flags;
diff --git a/flang/include/flang/Evaluate/target.h b/flang/include/flang/Evaluate/target.h
index d076fcbf083078..1950a4cb6bfc76 100644
--- a/flang/include/flang/Evaluate/target.h
+++ b/flang/include/flang/Evaluate/target.h
@@ -15,24 +15,13 @@
 #include "flang/Common/Fortran.h"
 #include "flang/Common/enum-class.h"
 #include "flang/Common/enum-set.h"
+#include "flang/Common/target-rounding.h"
 #include "flang/Evaluate/common.h"
 #include <cstdint>
 
 namespace Fortran::evaluate {
 
-// Floating-point rounding control
-struct Rounding {
-  common::RoundingMode mode{common::RoundingMode::TiesToEven};
-  // When set, emulate status flag behavior peculiar to x86
-  // (viz., fail to set the Underflow flag when an inexact product of a
-  // multiplication is rounded up to a normal number from a subnormal
-  // in some rounding modes)
-#if __x86_64__ || __riscv || __loongarch__
-  bool x86CompatibleBehavior{true};
-#else
-  bool x86CompatibleBehavior{false};
-#endif
-};
+using common::Rounding;
 
 ENUM_CLASS(IeeeFeature, Denormal, Divide, Flags, Halting, Inf, Io, NaN,
     Rounding, Sqrt, Standard, Subnormal, UnderflowControl)
diff --git a/flang/include/flang/Runtime/cpp-type.h b/flang/include/flang/Runtime/cpp-type.h
index fe21dd544cf7d8..27b43de7d0f0f6 100644
--- a/flang/include/flang/Runtime/cpp-type.h
+++ b/flang/include/flang/Runtime/cpp-type.h
@@ -11,7 +11,7 @@
 #ifndef FORTRAN_RUNTIME_CPP_TYPE_H_
 #define FORTRAN_RUNTIME_CPP_TYPE_H_
 
-#include "flang/Common/Fortran.h"
+#include "flang/Common/Fortran-consts.h"
 #include "flang/Common/float128.h"
 #include "flang/Common/uint128.h"
 #include <complex>
diff --git a/flang/include/flang/Runtime/type-code.h b/flang/include/flang/Runtime/type-code.h
index 8e7314e0af1efc..dd3a9f2690ee74 100644
--- a/flang/include/flang/Runtime/type-code.h
+++ b/flang/include/flang/Runtime/type-code.h
@@ -9,7 +9,7 @@
 #ifndef FORTRAN_RUNTIME_TYPE_CODE_H_
 #define FORTRAN_RUNTIME_TYPE_CODE_H_
 
-#include "flang/Common/Fortran.h"
+#include "flang/Common/Fortran-consts.h"
 #include "flang/Common/optional.h"
 #include "flang/ISO_Fortran_binding_wrapper.h"
 #include <utility>
diff --git a/flang/runtime/format.h b/flang/runtime/format.h
index 5329f2482d3e46..815bf70685e647 100644
--- a/flang/runtime/format.h
+++ b/flang/runtime/format.h
@@ -13,7 +13,7 @@
 
 #include "environment.h"
 #include "io-error.h"
-#include "flang/Common/Fortran.h"
+#include "flang/Common/Fortran-consts.h"
 #include "flang/Common/optional.h"
 #include "flang/Decimal/decimal.h"
 #include "flang/Runtime/freestanding-tools.h"
diff --git a/flang/runtime/non-tbp-dio.h b/flang/runtime/non-tbp-dio.h
index 05038a264ed992..8429d790fea57a 100644
--- a/flang/runtime/non-tbp-dio.h
+++ b/flang/runtime/non-tbp-dio.h
@@ -22,7 +22,8 @@
 #ifndef FORTRAN_RUNTIME_NON_TBP_DIO_H_
 #define FORTRAN_RUNTIME_NON_TBP_DIO_H_
 
-#include "flang/Common/Fortran.h"
+#include "flang/Common/Fortran-consts.h"
+#include "flang/Common/api-attrs.h"
 #include <cstddef>
 
 namespace Fortran::runtime::typeInfo {
diff --git a/flang/runtime/type-info.h b/flang/runtime/type-info.h
index c3f3595e32ef28..32403b1db5169e 100644
--- a/flang/runtime/type-info.h
+++ b/flang/runtime/type-info.h
@@ -13,7 +13,7 @@
 // flang/module/__fortran_type_info.f90.
 
 #include "terminator.h"
-#include "flang/Common/Fortran.h"
+#include "flang/Common/Fortran-consts.h"
 #include "flang/Common/bit-population-count.h"
 #include "flang/Common/optional.h"
 #include "flang/Runtime/descriptor.h"
diff --git a/flang/unittests/Evaluate/fp-testing.cpp b/flang/unittests/Evaluate/fp-testing.cpp
index 94d8d5086d000b..1a1d7425d58249 100644
--- a/flang/unittests/Evaluate/fp-testing.cpp
+++ b/flang/unittests/Evaluate/fp-testing.cpp
@@ -7,8 +7,8 @@
 #include <xmmintrin.h>
 #endif
 
+using Fortran::common::RealFlag;
 using Fortran::common::RoundingMode;
-using Fortran::evaluate::RealFlag;
 
 ScopedHostFloatingPointEnvironment::ScopedHostFloatingPointEnvironment(
 #if __x86_64__
diff --git a/flang/unittests/Evaluate/fp-testing.h b/flang/unittests/Evaluate/fp-testing.h
index 22dfa2d7d80c60..9091963a99b32d 100644
--- a/flang/unittests/Evaluate/fp-testing.h
+++ b/flang/unittests/Evaluate/fp-testing.h
@@ -1,12 +1,12 @@
 #ifndef FORTRAN_TEST_EVALUATE_FP_TESTING_H_
 #define FORTRAN_TEST_EVALUATE_FP_TESTING_H_
 
-#include "flang/Evaluate/target.h"
+#include "flang/Common/target-rounding.h"
 #include <fenv.h>
 
+using Fortran::common::RealFlags;
+using Fortran::common::Rounding;
 using Fortran::common::RoundingMode;
-using Fortran::evaluate::RealFlags;
-using Fortran::evaluate::Rounding;
 
 class ScopedHostFloatingPointEnvironment {
 public:
diff --git a/flang/unittests/Runtime/Complex.cpp b/flang/unittests/Runtime/Complex.cpp
index 46f3ad2f2712b2..d714da24dc4e58 100644
--- a/flang/unittests/Runtime/Complex.cpp
+++ b/flang/unittests/Runtime/Complex.cpp
@@ -13,7 +13,7 @@
 #pragma clang diagnostic ignored "-Wc99-extensions"
 #endif
 
-#include "flang/Common/Fortran.h"
+#include "flang/Common/Fortran-consts.h"
 #include "flang/Runtime/cpp-type.h"
 #include "flang/Runtime/entry-names.h"

DavidTruby

LGTM. Can you add [NFC] in the commit message?
Thanks for working on this!

jeanPerier

Please hold off on merging the pieces until there is more approval on the top level patch/approach.

The overall churn is quite big and takes time to review and assess for the runtime authors.

While the goal makes sense to me, the solution has a big footprint on the code/headers, so I think it is important to give some time to assess the solution and arguments made in the RFC. @sscalpone who is named as a code owner in the new flang-rt project is not available this week and needs to be involved.

Meinersbur · 2024-11-07T16:27:28Z

@jeanPerier I think the patches up until #110298 are good ideas independent of flang-rt. These reduce dependence of unrelated components.

The high number of changes files comes mostly from file renaming and the consequential path changes in inside the files (#include, copyright header, header guard). These nearly completely mechanical changes (I wrote a python script to apply them) are isolated in #110298.

@sscalpone is mentioned as code owner in flang-rt/ since he is already code owner of the same runtime sources in flang/.

The RFC started in August. #110217 ended draft status a month ago. There hasn't been any sign of
activity from Nvidia in the RFC or patch series for 3 weeks now. Please understand that eventually I will assume that Nvidia has no more concerns.

jeanPerier · 2024-11-07T18:08:56Z

Please understand that eventually I will assume that Nvidia has no more concerns.

Thank you for your understanding @Meinersbur, and for the work your doing to make builds of the runtime independent of the compiler itself. We are aware of the effort you put in the RFC and these patches, please rest assured that we are actively reviewing, testing, and discussing it since you called for review again on #110298 earlier this week.

…einersbur/flang_runtime_split-headers

…inersbur/flang_runtime_split-headers

…einersbur/flang_runtime_split-headers

…inersbur/flang_runtime_split-headers

mjklemm

The PR LGTM. I did not approve. since devs from NVIDIA are still reviewing.

jeanPerier

Thanks for letting me the time to review and test the whole solution. Given the progress, I am ok with this patch to be merged now to make it easier to iterate on the last two core patches.

[Flang] Split common headers

70ef83a

Meinersbur added 2 commits September 27, 2024 13:26

clang-format

6071ac6

Normlize header guards

98c71d8

Meinersbur mentioned this pull request Sep 27, 2024

[Flang] LLVM_ENABLE_RUNTIMES=flang-rt #110217

Merged

Meinersbur marked this pull request as ready for review October 7, 2024 20:47

llvmbot added flang:runtime flang Flang issues not falling into any other category flang:semantics labels Oct 7, 2024

Meinersbur requested review from clementval, tblah, mjklemm, jplehr, klausler, vzakhari, skatrak, jeanPerier, DavidTruby and kkwli October 30, 2024 13:23

DavidTruby approved these changes Nov 6, 2024

View reviewed changes

Meinersbur changed the title ~~[Flang] Split common headers~~ [Flang][NFC] Split common headers. Nov 6, 2024

jeanPerier reviewed Nov 6, 2024

View reviewed changes

Meinersbur requested a review from sscalpone November 7, 2024 15:49

Meinersbur changed the title ~~[Flang][NFC] Split common headers.~~ [Flang][NFC] Split common headers to reduce dependencies. Nov 11, 2024

Meinersbur added 3 commits November 15, 2024 11:57

Merge commit 'fa627d98e87504b6f6d621a7dab5d140340ed760^' into users/m…

7a08438

…einersbur/flang_runtime_split-headers

Merge commit 'fa627d98e87504b6f6d621a7dab5d140340ed760' into users/me…

a62b33e

…inersbur/flang_runtime_split-headers

Merge commit '06eb10dadfaeaadc5d0d95d38bea4bfb5253e077^' into users/m…

49b22d8

…einersbur/flang_runtime_split-headers

Meinersbur added 2 commits November 15, 2024 23:05

Merge commit '06eb10dadfaeaadc5d0d95d38bea4bfb5253e077' into users/me…

4dc1b14

…inersbur/flang_runtime_split-headers

Merge branch 'main' into users/meinersbur/flang_runtime_split-headers

2a97e4a

mjklemm reviewed Nov 16, 2024

View reviewed changes

jeanPerier approved these changes Nov 27, 2024

View reviewed changes

Meinersbur merged commit 0cda970 into main Dec 5, 2024
8 checks passed

Meinersbur deleted the users/meinersbur/flang_runtime_split-headers branch December 5, 2024 10:29

Meinersbur mentioned this pull request Jan 11, 2025

[Flang] Introduce FortranSupport #122069

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Flang][NFC] Split common headers to reduce dependencies. #110244

[Flang][NFC] Split common headers to reduce dependencies. #110244

Meinersbur commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading

llvmbot commented Oct 7, 2024 •

edited

Loading

DavidTruby left a comment

jeanPerier left a comment

Meinersbur commented Nov 7, 2024

jeanPerier commented Nov 7, 2024

mjklemm left a comment

jeanPerier left a comment

[Flang][NFC] Split common headers to reduce dependencies. #110244

[Flang][NFC] Split common headers to reduce dependencies. #110244

Conversation

Meinersbur commented Sep 27, 2024 • edited Loading

github-actions bot commented Sep 27, 2024 • edited Loading

llvmbot commented Oct 7, 2024 • edited Loading

DavidTruby left a comment

Choose a reason for hiding this comment

jeanPerier left a comment

Choose a reason for hiding this comment

Meinersbur commented Nov 7, 2024

jeanPerier commented Nov 7, 2024

mjklemm left a comment

Choose a reason for hiding this comment

jeanPerier left a comment

Choose a reason for hiding this comment

Meinersbur commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading

llvmbot commented Oct 7, 2024 •

edited

Loading