RFR (S): 8151002: Make Assembler methods vextract and vinsert match actual instructions

[Mikael Vidstedt]

Updated webrev:

http://cr.openjdk.java.net/~mikael/webrevs/8151002/webrev.01/webrev/

Incremental from webrev.00:

http://cr.openjdk.java.net/~mikael/webrevs/8151002/webrev.01.incr/webrev/

Comments below...

On 2016-03-01 23:36, Vladimir Ivanov wrote:
> Nice cleanup, Mikael!
>
> src/cpu/x86/vm/assembler_x86.hpp:
>
> 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!

> src/cpu/x86/vm/assembler_x86.cpp:
>
> !   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), as_XMMRegister(n));
> +      __ 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 :)

Cheers,
Mikael

>
> 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 and
>> 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
>> Webrev:
>> http://cr.openjdk.java.net/~mikael/webrevs/8151002/webrev.00/webrev/
>>
>> Special thanks to Mike Berg for helping discuss, co-develop, and test
>> the change!
>>
>> Cheers,
>> Mikael
>>