[vector] Infrastructure for calling math routines in .s files

[Lupusoru, Razvan A]

Nice catches - I will address the issues you found in subsequent patch.

So the Linux .s files will be upstreamed today/tomorrow as soon I merge the infrastructure patch. The Windows one are not yet ready because I have not yet resolved build issues - namely it seems that build system does not properly set VS assembler and my attempts to address it have not yet prevailed. That said, you were right to point out an issue with patch below since it guards the linking with ifdef __VECTOR_API_MATH_INTRINSICS_COMMON but actually it should be ifdef __VECTOR_API_MATH_INTRINSICS_LINUX for now until we get the Windows changes in.

Thanks,
Razvan

-----Original Message-----
From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com] 
Sent: Tuesday, May 29, 2018 4:12 AM
To: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>; panama-dev at openjdk.java.net
Subject: Re: [vector] Infrastructure for calling math routines in .s files


>> Please take a look at the updated version:
>> http://cr.openjdk.java.net/~rlupusoru/panama/webrev_callsvml_11/
> 
> Thanks, it looks really nice!
> 
> Some minor comments:

One more:

src/hotspot/share/utilities/globalDefinitions.hpp:

+  T_VECTOR64    = 20,
+  T_VECTOR128   = 21,
+  T_VECTOR256   = 22,
+  T_VECTOR512   = 23,

Those aren't used anymore.

Best regards,
Vladimir Ivanov

> 
> src/hotspot/share/opto/callnode.cpp:
> 
> +void CallLeafVectorNode::calling_convention( BasicType* sig_bt,
> VMRegPair *parm_regs, uint argcnt ) const {
> +  uint num_bits =
> tf()->range()->field_at(TypeFunc::Parms)->is_vect()->length_in_bytes() 
> * BitsPerByte;
> +#ifndef PRODUCT
> +  const TypeTuple* d = tf()->domain();
> +  for (uint i = TypeFunc::Parms; i < d->cnt(); i++) {
> +    Node* arg = in(i);
> +    assert(arg->bottom_type()->is_vect()->length_in_bytes() *
> BitsPerByte == num_bits, "vector arguments must match");
> +  }
> +#endif
> 
> For assertion purposes it's preferred to use "#ifdef ASSERT".
> 
> "#ifndef PRODUCT" is used primarily for "optimized" build configuration.
> 
> Also, what do you think about putting num_bits value into 
> CallLeafVectorNode?
> 
> 
> 
> What's the story for Windows x64 support? I see some build changes, 
> but it seems assembly is tailored specifically for Linux.
> 
> src/hotspot/os_cpu/linux_x86/svml_s_expf4_core_l9ha.s:
> 
> +#ifdef __VECTOR_API_MATH_INTRINSICS_LINUX
> ...
> +__svml_expf4_ha_l9:
> 
> 
> src/hotspot/cpu/x86/stubGenerator_x86_64.cpp:
> 
> +#ifdef __VECTOR_API_MATH_INTRINSICS_COMMON
> +    if (UseVectorApiIntrinsics) {
> +      if (UseAVX >= 2) {
> +        StubRoutines::_vector_float64_exp = CAST_FROM_FN_PTR(address,
> __svml_expf4_ha_l9);
> +        StubRoutines::_vector_float128_exp = 
> +CAST_FROM_FN_PTR(address,
> __svml_expf4_ha_l9);
> 
> 
> src/hotspot/share/utilities/globalDefinitions_vecApi.hpp:
> 
> +#if defined(_WIN64) && (defined(_MSC_VER) && (_MSC_VER >= 1600)) 
> +__VECTOR_API_MATH_INTRINSICS_COMMON TEXTEQU <> #define 
> +__VECTOR_API_MATH_INTRINSICS_COMMON
> +__VECTOR_API_MATH_INTRINSICS_WINDOWS TEXTEQU <> #define 
> +__VECTOR_API_MATH_INTRINSICS_WINDOWS
> +#endif
> 
> 
>> Also, I separated the tests this time and you can review here. I can 
>> merge this patch separately:
>> http://cr.openjdk.java.net/~rlupusoru/panama/webrev_mathtests_00/
> 
> Looks good.
> 
> I second Paul - kudos to you and the team at Intel! That's an 
> impressive and very valuable contribution. Thanks a lot for pushing 
> the limits of Java platform on that front!
> 
> Best regards,
> Vladimir Ivanov
> 
>>
>> Thanks and have a great weekend!
>>
>> --Razvan
>>
>> -----Original Message-----
>> From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
>> Sent: Friday, May 18, 2018 7:25 PM
>> To: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>; 
>> panama-dev at openjdk.java.net
>> Subject: Re: [vector] Infrastructure for calling math routines in .s 
>> files
>>
>>> So I am sending out the initial patch which adds infrastructure 
>>> (plus one implementation - exp) in order to be able to call these 
>>> assembly routines from JIT compiled code. In a subsequent patches, 
>>> we will contribute the other implementations in form of assembly files.
>>> http://cr.openjdk.java.net/~rlupusoru/panama/webrev_callsvml_09/index.
>>> html
>>
>> Wow, vector calls! Nice! :-)
>>
>> My main concern is amount of changes in shared code needed. I'd like 
>> to see the logic somehow confined just to those vector stub calls.
>>
>> Some suggestions:
>>
>>     (1) introduce new node for calling vector stubs - CallLeafVector?
>>          - no need in CallLeafNode::has_vector_arguments_or_return()
>>
>>     (2) Implement custom calling convention instead of extending 
>> c_calling_convention. It seems there is a need for a limited set of 
>> shapes (unary, binary, ?) and even having those cases hard-coded 
>> should be fine. It can simplify the patch in the following way:
>>          - have the logic factored out into platform-specific
>> vector_calling_convention() / vector_return_value();
>>          - get rid of T_VECTOR* basic types
>>          - no need to extend return_value in x86_64.ad for vector 
>> types
>>
>>> Please take a look at the patch and let me know if you have any 
>>> questions. I would like to point out that among changes there are 
>>> also a few which make sure that compiler does not tag with "NO_FP"
>>> any calls that may end up calling methods that are using FP/vector 
>>> code. This is quite important for Vector API since unlike 
>>> auto-vectorizer, there may be live vector registers at any point in 
>>> the code.
>>
>> Doesn't it cause problems for floats/doubles in xmm registers then? I 
>> have some doubts all those cases are problematic, but, anyway, I 
>> suggest to handle it separately and focus on vector stub calls alone 
>> here.
>>
>> Best regards,
>> Vladimir Ivanov
>>