[vectorIntrinsics] RFR: 8283709: Add x86 back-end implementation for bit BIT_COUNT operation [v9]

[Jatin Bhateja]

On Mon, 11 Apr 2022 21:42:35 GMT, Sandhya Viswanathan <sviswanathan at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8283709: Review comments resolved.
>
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4541:
> 
>> 4539:                                                       KRegister mask, bool merge, int vec_enc) {
>> 4540:   assert(VM_Version::supports_avx512vl() || vec_enc == Assembler::AVX_512bit, "");
>> 4541:   assert(UsePopCountInstruction, "");
> 
> Please remove this assert on UsePopCountInstruction.

Again, its counter intuitive to enter into this control flow with UsePopcountInstruction being false.

> src/hotspot/cpu/x86/x86.ad line 8660:
> 
>> 8658:               VM_Version::supports_avx512_bitalg()) ||
>> 8659:              (is_non_subword_integral_type(Matcher::vector_element_basic_type(n->in(1))) &&
>> 8660:               VM_Version::supports_avx512_vpopcntdq())));
> 
> Could be replaced by:
> predicate(is_pop_count_instr_target(Matcher::vector_element_basic_type(n->in(1))));
> Also no need to check for UsePopCountInstruction any where as it is only meant for scalar popcount.

ADLC does not include code written in %source { }  clause .ad file in to generate DFA.cpp
And adding a prediction logic into a new matcher routine or some other files included by dfa.cpp looks like an overkill since its appearing on only two instruction patterns.

> src/hotspot/cpu/x86/x86.ad line 8691:
> 
>> 8689: 
>> 8690: instruct vpopcountL_avx_reg(vec dst, vec src, vec xtmp1, vec xtmp2, vec xtmp3, rRegP rtmp) %{
>> 8691:   predicate(!UsePopCountInstruction || !VM_Version::supports_avx512_vpopcntdq());
> 
> Please remove the reference to UsePopCountInstruction here.

There is still a matcher path which can be chosen even if user turns off UsePopCountInstruction,  relating this flag to an operation may not be correct.  But I agree that to avoid any confusion we can remove its usage from pattern for the time being.

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

PR: https://git.openjdk.java.net/panama-vector/pull/185