[flang-commits] [flang] [llvm] [Flang] KIND De-templatization (PR #216960)

[Michael Kruse via flang-commits]

================
@@ -52,15 +53,29 @@ using common::TypeCategory;
 class TargetCharacteristics;
 
 // Specific intrinsic types are represented by specializations of
-// this class template Type<CATEGORY, KIND>.
-template <TypeCategory CATEGORY, int KIND = 0> class Type;
+// this class template Type<CATEGORY>.
+// This used to be Type<CATEGORY,KIND>, but now KIND is passed at runtime.
+// TODO: Since they are functionally the same, replace all occurances of Type
+// with TypeCategory.
+template <TypeCategory CATEGORY> class Type;
 
-using SubscriptInteger = Type<TypeCategory::Integer, 8>;
-using CInteger = Type<TypeCategory::Integer, 4>;
-using LargestInt = Type<TypeCategory::Integer, 16>;
-using LogicalResult = Type<TypeCategory::Logical, 4>;
-using LargestReal = Type<TypeCategory::Real, 16>;
-using Ascii = Type<TypeCategory::Character, 1>;
+using SubscriptInteger = Type<TypeCategory::Integer>;
+inline constexpr int SubscriptIntegerKind{8};
+
+using CInteger = Type<TypeCategory::Integer>;
+inline constexpr int CIntegerKind{4};
+
+using LargestInt = Type<TypeCategory::Integer>;
+inline constexpr int LargestIntKind{16};
+
+using LogicalResult = Type<TypeCategory::Logical>;
----------------
Meinersbur wrote:

There are two `GetScalarConstantValue`:
https://github.com/llvm/llvm-project/blob/8d7cb66599198a0ae123aac4bfc95f4f4aca8042/flang/include/flang/Evaluate/fold.h#L78-L89

This one checks for a specific kind. It depends on the caller whether they already know what the KIND is. Together with `std::get_if` and `Fortran::evaluate::UnwrapExpr` the one without kind argument will match any kind. I experimented to remove the overloads without kind but that caused so many changes, and in the majority of cases it can only be a specific kind by design that I did not consider it worth it. This is described in the summary. If here is such a deviation in behavior, my approach is to use the kind argument overload there. If you find more of such occurances, I would like to have them the same behaviour. The simplification could be part of a followup.

For `openmp-utils.cpp` I am going to use the kind-argument version of GetScalarConstantValue and add a test case of the OpenMP metadirective.

I interpret your request that naming these `Ascii`, `LogicalResult` etc. is misleading since it also includes multibyte strings[^1]/ any Logical. I will add a comment.

The description includes the PR stack as it was meant to be. GitHub did not allow me to add a new PR into the middle of the stack. So I dissolved the stack and created a new one. But GitHub does not allow adding already closed PRs to a stack, which is why #216958 is not part of it. 

[^1]: This was misleading before as well; 1-byte characters are not necessarily ascii.


https://github.com/llvm/llvm-project/pull/216960