Reduced MaxVectorSize and vector type initialization

[Vladimir Kozlov]

Hi Nils,

"On a platform that supports AVX2 but have MaxVectorSize limited to 16, the VM will fail when the TypeVect::VECTY/mreg2type[Op_VecY] is uninitialized. We will also hit asserts in a few places like"

MaxVectorSize was designed to limit vector size for testing purpose. I just run compiler/codegen jtreg tests, which includes vector tests, on avx2 Intel machine with -XX:MaxVectorSize=16 and did not 
hit any problems.

I looked and did not find what mismatch you are talking about:
"In the ad-files feature flags (UseAVX etc.) are used to control what rules should be matched if it has effects on specific vector registers. Here we have a mismatch."

C2 should not generate vector with size > MaxVectorSize so they should not be any instructions in .ad file which conflict with it.

Can you show output of -Xlog:os+cpu on your machine?

vector_size_supported() takes into account MaxVectorSize:

   static const bool vector_size_supported(const BasicType bt, int size) {
     return (Matcher::max_vector_size(bt) >= size &&
             Matcher::min_vector_size(bt) <= size);
   }

const int Matcher::max_vector_size(const BasicType bt) {
   return vector_width_in_bytes(bt)/type2aelembytes(bt);
}

const int Matcher::vector_width_in_bytes(BasicType bt) {
...
   // Use flag to limit vector size.
   size = MIN2(size,(int)MaxVectorSize);


Thanks,
Vladimir

On 10/25/17 6:13 AM, Nils Eliasson wrote:
> Deans suggestion with making the TypeVect initialization unconditional also removes all platform dependencies on type.cpp:
> 
> http://cr.openjdk.java.net/~neliasso/maxvectorsize/webrev.02/
> 
> Regards,
> Nils
> 
> On 2017-10-25 00:02, Vladimir Kozlov wrote:
>> We can't use platform specific UseAVX flag in shared code in type.cpp.
>>
>> I would say we should not support AVX (set UseAVX to 0) on AMD < 17h.
>> And we need to ask AMD which AMD cpus are supporting AVX, AVX2, AVX3 and corresponding vectors 32 and 64 bytes.
>> If AMD's Instructions Set before 17h does not support whole 32 bytes vectors we can't call it AVX.
>>
>> Thanks,
>> Vladimir
>>
>> On 10/18/17 10:01 AM, dean.long at oracle.com wrote:
>>> How about initializing TypeVect::VECTY and friends unconditionally? I am nervous about exchanging one guarding condition for another.
>>>
>>> dl
>>>
>>>
>>> On 10/18/17 1:03 AM, Nils Eliasson wrote:
>>>>
>>>> HI,
>>>>
>>>> I ran into a problem with the interaction between MaxVectorSize and the UseAVX. For some AMD CPUs we limit the vector size to 16 because it gives the best performance.
>>>>
>>>>> +    if (cpu_family() < 0x17 && MaxVectorSize > 16) {
>>>>> +      // Limit vectors size to 16 bytes on AMD cpus < 17h.
>>>>>        FLAG_SET_DEFAULT(MaxVectorSize, 16);
>>>>>      }
>>>>
>>>> Whenf MaxVecorSize is set to 16 it has the sideeffect that the TypeVect::VECTY and mreg2type[Op_VecY] won't be initalized even though the platform has the capability.
>>>>
>>>> Type.cpp:~660
>>>>
>>>> [...]
>>>> >   if (Matcher::vector_size_supported(T_FLOAT,8)) {
>>>> >     TypeVect::VECTY = TypeVect::make(T_FLOAT,8);
>>>> >   }
>>>> [...]
>>>> >   mreg2type[Op_VecY] = TypeVect::VECTY;
>>>>
>>>>
>>>> In the ad-files feature flags (UseAVX etc.) are used to control what rules should be matched if it has effects on specific vector registers. Here we have a mismatch.
>>>>
>>>> On a platform that supports AVX2 but have MaxVectorSize limited to 16, the VM will fail when the TypeVect::VECTY/mreg2type[Op_VecY] is uninitialized. We will also hit asserts in a few places like: 
>>>> assert(Matcher::vector_size_supported(T_FLOAT,RegMask::SlotsPerVecY), "sanity");
>>>>
>>>> Shouldn't the type initialization in type.cpp be dependent on feature flag (UseAVX etc.) instead of MaxVectorSize? (The type for the vector registers are initialized if the platform supports them, 
>>>> but they might not be used if MaxVectorSize is limited.)
>>>>
>>>> This is a patch that solves the problem, but I have not convinced myself that it is the right way:
>>>> http://cr.openjdk.java.net/~neliasso/maxvectorsize/webrev/
>>>>
>>>> Feedback appreciated,
>>>>
>>>> Regards,
>>>> Nils Eliasson
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~neliasso/maxvectorsize/webrev/
>>>>
>>>
>