RFR: 8307423: [s390x] Represent Registers as values
Martin Doerr
mdoerr at openjdk.org
Thu May 4 19:56:18 UTC 2023
On Thu, 4 May 2023 15:08:57 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.
LGTM. Please consider my minor suggestions.
src/hotspot/cpu/s390/assembler_s390.hpp line 196:
> 194: _index(index),
> 195: _disp(disp) {}
> 196:
I can live with the removal, but I guess it may be useful at some point of time. s390 supports specifying both, index and disp (unlike PPC64).
src/hotspot/cpu/s390/register_s390.hpp line 186:
> 184:
> 185: // tester
> 186: constexpr bool is_valid() const { return 0 <= _encoding && _encoding < number_of_registers; }
Indentation is different than for the other register types. I suggest to adapt it, here.
src/hotspot/cpu/s390/register_s390.hpp line 433:
> 431:
> 432: // Temporary registers to be used within frame manager. We can use
> 433: // the nonvolatile because the call stub has saved them.
Better: "nonvolatile ones"
-------------
Marked as reviewed by mdoerr (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/13805#pullrequestreview-1413798190
PR Review Comment: https://git.openjdk.org/jdk/pull/13805#discussion_r1185448158
PR Review Comment: https://git.openjdk.org/jdk/pull/13805#discussion_r1185450644
PR Review Comment: https://git.openjdk.org/jdk/pull/13805#discussion_r1185443064
More information about the hotspot-dev
mailing list