On Tue, 20 Feb 2024 08:04:27 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 1584:
>>
>>> 1582: Label *larr[] = {&case0, &case1, &case2, &case3};
>>> 1583: for (int i = 0; i < 4; i++) {
>>> 1584: // dst[i] = mask ? src[index[i]] : 0
>>
>> I like these comments a lot!
>> They would be even better if they had a more clear relation to the register
>> names.
>>
>> `dst[i] = mask[i+midx] ? src[idx_base[i]] : 0`
>> It would then be helpful to know the types of these arrays.
>> It seems that `idx_base` is an array with type int, right?
>
> Ah, and can we use `base_index`? Otherwise we have an inconsistency with
> `index` vs `idx`.
We are accessing two arrays here, source arrays and index arrays, registers
holding their addresses are 'base' and 'idx_base', naming looks appropriate in
this context. Comments adjusted to reflect actual register names.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16354#discussion_r1501753568
