RFR: 8297539: Consolidate the uses of the int<->float union conversion trick

[John R Rose]

On Thu, 2 Feb 2023 10:17:21 GMT, Afshin Zafari <duke at openjdk.org> wrote:

> ### Discussion
> 
> There are different ways of casting/converting integral types to floating point types and vice versa in the code. 
> These conversions are changed to invocation of a centralised `cast<To>(From)` method in `primitiveConversions.hpp`. To make this changes build and run, some new `cast` methods are introduced and header dependencies changed. 
> 
> ### Patch
> 
> - All instances of using `union` technique that are used for int<->float and long<->double type conversions are replaced by `PrimitiveConversions::cast<To>(From)` method.
>   - Instances of, for example, converting `int`<->`intptr_t` are not changed.
> - After this change, `JavaValue` also uses the `PrimitiveConversions::cast<To>(From)` templates. Therefore, all the `union` conversions for `int`<-> `float` and `long` <-> `double` in hotspot are now centralised in one location.
> - Templates in `PrimitiveConversions` class are numbered from 1-9 and all lines of code that use `PrimitivEConversions::cast<To>(From)` have comments saying which templated `cast<To>(From)` matches. Issue number 8297539 is also in the comments to make the search/reviews more easier.
> - The new 'cast<To>(From)' templates are numbered 6-9 for some instances where existing templates (1-5) did not match.
> - `globalDefinitions.hpp` now includes `primitiveConversions.hpp`.
> - `primitiveConversions.hpp` does NOT include `globalDefinitions.hpp` any more. Some typedef's are added to it to compensate the missing types.
> - The followings are not changed:
>   - `jni.hpp`
>   - `ciConstant.hpp` is not changed, since uinon is not used for converting `int` <->`float` or `long` <-> `double`. There are some asserts that allow setter/getter used only for the same types.
>   - `bytecodeInterpreter_zero.hpp` is not changed. Writing to and reading from the `VMJavaVal64` are always with the same type and no conversion is made.
>   - union with bit fields are not changed.
>   - `json.hpp` and `json.cpp` in src/hotspot/share/utilities.
> 
> ### Test
> mach5 tiers 1-5 passed, tiers 6-8 in progress

I think the term "cast" in this API means too many different things, including things that a C programmer would not think of as simple casting.  To be clear, this is a problem I have with the existing API, not just this PR.

I request we start to make a clearer distinction between conversions which preserve values (perhaps with truncation or loss of precision) and reinterpretations which discard values but keep the underlying bits.  Both may be called casts, but it is asking for bugs to use the name for both kinds of requests.

(Using the word cast for something that preserves at least some values is how the JLS handles conversion terminology, and the JLS makes reinterpret casts available only via special methods.  I know C++ has its own lexicon of terms, but I claim some privilege on this project for Java terms as well.)

I know this work here is changing from names like "jint_cast" which perform reinterpretations, but I always viewed those names as suspicious, and had to learn by rote that they refer to reinterpretations and not true C (or Java) casts.  Naming it just "cast" with a tricky type parameter is a move even farther in the wrong direction.

Having the function named "cast" in a class named PrimitiveConversions (not PrimitiveReinterpretations) makes it dangerously easy to miss that the functions in that API are (apparently?) all reinterpretations, and none are what a Java programmer would call a cast.  I suggest either renaming of the class (PrimitiveReinterpretations) or better changing the name from "cast" to something like "reinterpret".  Then casual readers of code will be less likely to make errors of… reinterpretation.

-------------

PR: https://git.openjdk.org/jdk/pull/12384