On Fri, 8 May 2026 09:49:55 GMT, Paul Hübner <[email protected]> wrote:
>> Coleen Phillimore has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - include another file.
>> - Some forward declarations might do the trick.
>
> src/hotspot/cpu/ppc/continuationEntry_ppc.inline.hpp line 30:
>
>> 28: #include "runtime/continuationEntry.hpp"
>> 29:
>> 30: #include "code/codeCache.hpp"
>
> This doesn't seem very Valhalla-related at a glance. What happened?
It might have been transitively included and now it's not.
> src/hotspot/cpu/ppc/frame_ppc.hpp line 411:
>
>> 409: // size, in words, of maximum shift in frame position due to
>> alignment
>> 410: align_wiggle = 1,
>> 411: // This is wrong and unimplemented
>
> A little bit more context would be nice, "this is wrong" doesn't really say
> much. Then again, I'm not familiar with PPC frame code!
A ppc porter can fix it. I don't know what the sender_sp_offset should be.
> src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp line 1100:
>
>> 1098: // rest through register value=Z_R12.
>> 1099: BLOCK_COMMENT("Shuffle arguments");
>> 1100:
>
> Extra whitespace?
Since I changed this code, there's no need to maintain existing whitespace.
> src/hotspot/cpu/riscv/interp_masm_riscv.cpp line 1596:
>
>> 1594: // CallTypeData/VirtualCallTypeData to reach its end. Non null
>> 1595: // if there's a return to profile.
>> 1596: assert(SingleTypeEntry::static_cell_count() <
>> TypeStackSlotEntries::per_arg_count(), "can't move past ret type");
>
> Nit: "single type" in the assert message.
All the platforms say "ret type" in this assert, so I should leave it to be the
same.
> src/hotspot/share/asm/macroAssembler_common.hpp line 53:
>
>> 51: VMRegPair* regs_from, int from_index,
>> int regs_from_count, RegState* reg_state);
>> 52:
>> 53: int store_inline_type_fields_to_buf(ciInlineKlass* vk, bool
>> from_interpreter = true);
>
> Oh I see this was moved around to become common code.
Yes. The declaration are common.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2396#discussion_r3208957413
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2396#discussion_r3208960840
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2396#discussion_r3208965934
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2396#discussion_r3208975439
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2396#discussion_r3208984518
