RFR: 8255438: [Vector API] More instructs in x86.ad should use legacy mode for code-gen [v2]

[Azeem Jiva]

On Wed, 28 Oct 2020 09:24:30 GMT, Jie Fu <jiefu at openjdk.org> wrote:

>> Hi all,
>> 
>> Just as @jatin-bhateja pointed out [1], there are more instructs in x86.ad which should use legacy mode.
>> 
>> It would be better to fix the following cases:
>> ------------------------
>> 1. instruct mul2L_reg
>>    The code-gen logic uses phaddd [2], which requires legacy mode here [3].
>>    This bug might be reproduced on AVX512 machines without avx512dq.
>> 
>> 2. instruct vmul4L_reg_avx
>>    The code-gen logic uses vphaddd [4], which requires legacy mode here [5].
>>    This bug might be reproduced on AVX512 machines without avx512dq.
>> 
>> 3. instruct reductionL
>>    For MulReductionVL, the code-gen chain can be:  reduceL --> reduce4L --> reduce_operation_128 --> vpmullq [6]
>>    vpmullq require legacy mode [7] if avx512dq isn't supported.
>>    This bug might be reproduced on AVX512 machines without avx512dq.
>> 
>> 4. instruct reductionB
>>    For MinReductionV, the code-gen chain can be:  reduceB --> reduce32B --> reduce_operation_128 --> pminsb [8]
>>    pminsb require legacy mode [9] if avx512bw isn't supported.
>>    This bug might be reproduced on AVX512 machines without avx512bw.
>> ------------------------
>> Bugs in mul2L_reg/vmul4L_reg_avx/reductionL can be only reproduced on AVX512 machines without avx512dq.
>> And bug in reductionB can be only reproduced on AVX512 machines without avx512bw.
>> 
>> Unfortunately, it's impossible for us to create reproducers since our AVX512 platforms support both avx512dq and avx512bw.
>> However, it do make sense to fix these unexposed bugs since vector api code will be sure to run on various paltforms (e.g., AVX512 machines without avx512dq/bw) in the future.
>> 
>> The fix just changes vec to legVec, which is quite safe in theory.
>> 
>> As for the reduction patterns of Float and Double, I don't see any reason that they should use legacy mode (maybe I've missed something).
>> 
>> Testing:
>>   - jdk/incubator/vector on both AVX512 and AVX256 machines
>> 
>> Any comments?
>> 
>> Thanks a lot.
>> Best regards,
>> Jie
>> 
>> [1] https://github.com/openjdk/jdk/pull/791#commitcomment-43473733
>> [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86.ad#L5472
>> [3] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/assembler_x86.cpp#L6217
>> [4] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86.ad#L5497
>> [5] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/assembler_x86.cpp#L6165
>> [6] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp#L1521
>> [7] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/assembler_x86.cpp#L6428
>> [8] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp#L1482
>> [9] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/assembler_x86.cpp#L6475
>
> Jie Fu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add reductionL_avx512dq and reductionB_avx512bw

Marked as reviewed by azeemj (Author).

-------------

PR: https://git.openjdk.java.net/jdk/pull/874