On Wed, 20 May 2026 08:38:46 GMT, Aleksey Shipilev <[email protected]> wrote:
>> Volodymyr Paprotski has updated the pull request incrementally with one >> additional commit since the last revision: >> >> second round of review from Sandhya > > src/hotspot/cpu/x86/stubGenerator_x86_64_sha3.cpp line 273: > >> 271: __ movq(T1, Address(state4, 24 * 8)); >> 272: __ vshufpd(T0, T0, T1, 0b00, Assembler::AVX_128bit); >> 273: __ evinserti64x2(A24, A24, T0, 0b01, Assembler::AVX_256bit); > > Should this also be replaced with `vinserti128`? This is still AVX-512? You are right, thanks, missed it in my last round. > src/hotspot/share/opto/library_call.cpp line 8377: > >> 8375: } >> 8376: >> 8377: if (!stubAddr) return false; > > For this -- what I assume is a release-build safety return -- to work, > `stubAddress` should be initialized to `nullptr`? This sent me down quite the rabbit hole, because I do indeed remember having to set stubs to `nullptr` by default then override in stubGenerator.. Its all been cleaned up with macros.. In `src/hotspot/cpu/x86/stubRoutines_x86.cpp`: ```C++ #define DEFINE_ARCH_ENTRY(arch, blob_name, stub_name, field_name, getter_name) \ address StubRoutines:: arch :: STUB_FIELD_NAME(field_name) = nullptr; Comment in `src/hotspot/share/runtime/stubDeclarations.hpp` says as much, but I wanted to find the above expansion as proof..: ```C++ // do_entry is used to declare or define a static field of class // StubRoutines with type address that stores a specific entry point // for a given stub. n.b. the number of entries associated with a stub // is often one but it can be more than one and, in a few special // cases, it is zero. do_entry is also used to declare and define an // associated getter method for the field. do_entry is used to declare // fields that should be initialized to nullptr. ``` ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31125#discussion_r3277838372 PR Review Comment: https://git.openjdk.org/jdk/pull/31125#discussion_r3277807843
