RFR: 8286302: Port JEP 425 to PPC64 [v5]

[Richard Reingruber]

On Thu, 17 Nov 2022 17:48:12 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> Richard Reingruber has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Cleanup BasicExp.java
>
> src/hotspot/cpu/aarch64/frame_aarch64.hpp line 110:
> 
>> 108:     // between a callee frame and its stack arguments, where it is part
>> 109:     // of the caller/callee overlap
>> 110:     metadata_words_at_top                            = 0,
> 
> I like the `_at_bottom` and `_at_top` versions. The old version could possibly get replaced completely in a separate RFE (this change is already large enough).

Thanks. I don't think though that these can replace metadata_words completely because for at least one platform you would replace it with 0.

> src/hotspot/cpu/ppc/continuationFreezeThaw_ppc.inline.hpp line 498:
> 
>> 496:     // align fp
>> 497:     int padding  = fp - align_down(fp, frame::frame_alignment);
>> 498:     fp  -= padding;
> 
> Additional whitespaces should better get removed.

Done

> src/hotspot/cpu/ppc/continuationFreezeThaw_ppc.inline.hpp line 520:
> 
>> 518: 
>> 519:     if ((bottom && argsize > 0) || caller.is_interpreted_frame()) {
>> 520:       frame_sp   -= argsize + frame::metadata_words_at_top;
> 
> whitespace

Done

> src/hotspot/cpu/ppc/frame_ppc.hpp line 370:
> 
>> 368: 
>> 369:   union {
>> 370:     intptr_t*  _fp; // frame pointer
> 
> whitespace

Done.

> src/hotspot/cpu/ppc/frame_ppc.inline.hpp line 113:
> 
>> 111:   // In thaw, non-heap frames use this constructor to pass oop_map.  I don't know why.
>> 112:   assert(_on_heap || _cb != nullptr, "these frames are always heap frames");
>> 113:   if (cb != NULL) {
> 
> Better use `nullptr`.

Done

> src/hotspot/cpu/ppc/nativeInst_ppc.cpp line 448:
> 
>> 446: }
>> 447: 
>> 448: // Inserts an undefined instruction at a given pc
> 
> More precisely: An instruction which is specified to cause a SIGILL.

Done

> src/hotspot/cpu/ppc/ppc.ad line 14386:
> 
>> 14384: 
>> 14385:   format %{ "CALL,static $meth \t// ==> " %}
>> 14386:   size(8);
> 
> Please make the sizes precise. They are only 8 Byte when continuations are enabled.

Done (note it's not done on other platforms).

> src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp line 1803:
> 
>> 1801: 
>> 1802:     // Read interpreter arguments into registers (this is an ad-hoc i2c adapter)
>> 1803:     __ ld(reg_cont_obj,   Interpreter::stackElementSize*3, R15_esp);
> 
> Would you like to align the indentation?

Done

> src/hotspot/cpu/ppc/stackChunkFrameStream_ppc.inline.hpp line 194:
> 
>> 192:         + 1 // for the mirror oop
>> 193:         + ((intptr_t*)f.interpreter_frame_monitor_begin()
>> 194:             - (intptr_t*)f.interpreter_frame_monitor_end())/BasicObjectLock::size();
> 
> whitespace

Ok now?

> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 488:
> 
>> 486:   assert(!Interpreter::contains(_cont.entryPC()), "");
>> 487:   static const int doYield_stub_frame_size = NOT_PPC64(frame::metadata_words)
>> 488:                                              PPC64_ONLY(frame::abi_reg_args_size >> LogBytesPerWord);
> 
> Unfortunate that we still need to distinguish, here. But, ok. I don't have a better idea atm.

A pd constant could be introduced. Not sure if it's worth it.

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

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