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