RFR (S): 8151002: Make Assembler methods vextract and vinsert match actual instructions
mikael.vidstedt at oracle.com
Wed Mar 2 23:02:43 UTC 2016
After discussing with Vladimir off-list we agreed that changing the type
of the immediate (imm8) argument to uint8_t is both clearer, has the
potential to catch incorrect uses of the functions, and also makes the
asserts more straightforward. In addition to that Vladimir noted that I
had accidentally included newline in the assert messages.
Incremental from webrev.01:
On 2016-03-02 13:12, Mikael Vidstedt wrote:
> Updated webrev:
> Incremental from webrev.00:
> Comments below...
> On 2016-03-01 23:36, Vladimir Ivanov wrote:
>> Nice cleanup, Mikael!
>> Outdated comments:
>> // Copy low 128bit into high 128bit of YMM registers.
>> // Load/store high 128bit of YMM registers which does not destroy
>> other half.
>> // Copy low 256bit into high 256bit of ZMM registers.
> Updated, thanks for catching!
>> ! emit_int8(imm8 & 0x01);
>> Maybe additionally assert valid imm8 range?
> Good idea, I had added asserts earlier but removed them. I added them
> back again!
>> Maybe keep vinsert*h variants and move them to MacroAssembler? They
>> look clearer in some contextes:
>> - __ vextractf128h(Address(rsp, base_addr+n*16),
>> + __ vextractf128(Address(rsp, base_addr+n*16),
>> as_XMMRegister(n), 1);
> Can I suggest that we try to live without them for a while and see how
> much we miss them? I think having it there may actually be more
> confusing in many cases :)
>> Otherwise, looks good.
>> Best regards,
>> Vladimir Ivanov
>> On 3/2/16 3:25 AM, Mikael Vidstedt wrote:
>>> Please review the following change which updates the various vextract*
>>> and vinsert* methods in assembler_x86 & macroAssembler_x86 to better
>>> match the real HW instructions, which also has the benefit of providing
>>> the full functionality/flexibility of the instructions where earlier
>>> only some specific modes were supported. Put differently, with this
>>> change it's much easier to correlate the methods to the Intel manual
>>> understand what they actually do.
>>> Specifically, the vinsert* family of instructions take three registers
>>> and an immediate which decide how the bits should be shuffled around,
>>> but without this change the method only allowed two of the registers to
>>> be specified, and the immediate was hard-coded to 0x01.
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8151002
>>> Special thanks to Mike Berg for helping discuss, co-develop, and test
>>> the change!
More information about the hotspot-compiler-dev