RFR: 8307423: [s390x] Represent Registers as values [v3]

[Lutz Schmidt]

On Fri, 5 May 2023 12:53:27 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:

>> The recent Register implementation uses wild pointer (including null pointer) dereferences which exhibit undefined behavior. We should migrate away from pointer-based representation of Register values as it was done for x86 ([JDK-8292153](https://bugs.openjdk.org/browse/JDK-8292153)). Problems exist when trying to build with GCC 11.3.0 ([JDK-8307093](https://bugs.openjdk.org/browse/JDK-8307093)).
>> 
>> Tested `fastdebug, slowdebug, optimized, release build` , `tier1-test` on fastdebug build and build with GCC-9.5.0 as well.
>
> Amit Kumar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   suggestion from @RealLucy

Even more requests - sorry.

src/hotspot/cpu/s390/register_s390.hpp line 95:

> 93: 
> 94: inline constexpr Register as_Register(int encoding) {
> 95:   assert(encoding >= NOREG_ENCODING && encoding < Register::number_of_registers, "bad register encoding");

How about coding the assert condition as 
`encoding == NOREG_ENCODING || (0 <= encoding && encoding < Register::number_of_registers)`
That decouples NOREG_ENCODING from the is_valid() range.

src/hotspot/cpu/s390/register_s390.hpp line 152:

> 150: 
> 151: inline constexpr ConditionRegister as_ConditionRegister(int encoding) {
> 152:   assert(encoding >= 0 && encoding < ConditionRegister::number_of_registers, "bad condition register encoding");

Same as for Register

src/hotspot/cpu/s390/register_s390.hpp line 196:

> 194: 
> 195: inline constexpr FloatRegister as_FloatRegister(int encoding) {
> 196:   assert(encoding >= NOREG_ENCODING && encoding < FloatRegister::number_of_registers, "bad float register encoding");

Same as for Register

src/hotspot/cpu/s390/register_s390.hpp line 300:

> 298:   // accessors
> 299:   constexpr int  encoding()  const { assert(is_valid(), "invalid register"); return _encoding; }
> 300:   VectorRegister successor() const { return VectorRegister(encoding() + 1); }

Please add wrap-around logic as done for class Register

src/hotspot/cpu/s390/register_s390.hpp line 336:

> 334: 
> 335: inline constexpr VectorRegister as_VectorRegister(int encoding) {
> 336:   assert(encoding >= NOREG_ENCODING && encoding < VectorRegister::number_of_registers, "bad vector register encoding");

Same as for Register

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

Changes requested by lucy (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13805#pullrequestreview-1415374466
PR Review Comment: https://git.openjdk.org/jdk/pull/13805#discussion_r1186453369
PR Review Comment: https://git.openjdk.org/jdk/pull/13805#discussion_r1186453877
PR Review Comment: https://git.openjdk.org/jdk/pull/13805#discussion_r1186454144
PR Review Comment: https://git.openjdk.org/jdk/pull/13805#discussion_r1186457328
PR Review Comment: https://git.openjdk.org/jdk/pull/13805#discussion_r1186456042