RFR: 8328998: Encoding support for Intel APX extended general-purpose registers [v27]
Vladimir Kozlov
kvn at openjdk.org
Wed May 22 17:04:10 UTC 2024
On Wed, 22 May 2024 16:19:04 GMT, Steve Dohrmann <duke at openjdk.org> wrote:
>> src/hotspot/cpu/x86/assembler_x86.cpp line 13030:
>>
>>> 13028: }
>>> 13029: }
>>> 13030: if (is_map1) emit_int8(0x0F);
>>
>> - First. What `is_map1` means? There is no explanation for this name. May be add comment somewhere in `assembler_x86.hpp` file or use more meaningful name.
>>
>> - Second. You added one more byte `0x0F` for instructions even when extended registers are not used and APX is not enabled. Why? You added it in several `prefix()` and `prefixq()` methods. It can lead to regression since code size will increase.
>
> The is_map1 bool indicates an x86 map1 instruction which, when
> legacy encoded, uses a 0x0F opcode prefix. By specification, the
> opcode prefix is omitted when using rex2 encoding in support
> of APX extended GPRs.
>
> I added this comment before the relevant prefix functions in assembler_x86.hpp. Is this sufficient?
Comment is good.
Actually you should have point me to code which shows that code generation did not change, we generated `0x0F` before:
- prefix(src);
- emit_int16(0x0F, 0x18);
+ prefix(src, true /* is_map1 */);
+ emit_int8(0x18);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1610346227
More information about the hotspot-compiler-dev
mailing list