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

Kim Barrett kbarrett at openjdk.org
Thu Feb 2 17:38:26 UTC 2023


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

Changes requested by kbarrett (Reviewer).

src/hotspot/share/utilities/globalDefinitions.hpp line 28:

> 26: #define SHARE_UTILITIES_GLOBALDEFINITIONS_HPP
> 27: 
> 28: #include "metaprogramming/primitiveConversions.hpp"

I think this should not be done, for the same reasons as not including the suggested bit_cast
(https://git.openjdk.org/jdk/pull/11865).  There is substantial overlap between this proposed
change and that one, though I think the bit_cast proposal is more problematic.

Rather than adding a new dependency here, separate out the code that uses the conversions
into new files and update the (much smaller number than use globalDefinitions.hpp) users
of those functions to include the new headers.

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

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


More information about the hotspot-dev mailing list