On Mon, 26 Feb 2024 09:39:01 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> Jatin Bhateja has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments resolutions.
>
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 1584:
>
>> 1582: if (elem_bt == T_SHORT) {
>> 1583: Label case0, case1, case2, case3;
>> 1584: Label* larr[] = {&case0, &case1, &case2, &case3};
>
> Not sure if I asked this already: why define them all here, rather than
> locally in the loop?
To avoid invariant initializations to happen within the loop, compiler will
unroll this small loop and will forward the initializations, if it does not
then we can save redundant allocation within loop.
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 1674:
>
>> 1672: int vector_len, int vlen_enc) {
>> 1673: assert(is_subword_type(elem_ty), "");
>> 1674: Label GATHER8_LOOP;
>
> Why not define it righ before the first use?
Its not going to affect generated code, by convention declaration should
anyways be at the beginning of block scope.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16354#discussion_r1502587701
PR Review Comment: https://git.openjdk.org/jdk/pull/16354#discussion_r1502587580
