RFR (S) 8220656 : ARM32: -XX:MaxVectorSize=16 makes SIGILL

[Boris Ulasevich]

Hi David,

On 15.05.2019 15:39, David Holmes wrote:
> Hi Boris,
> 
> On 15/05/2019 9:02 pm, Boris Ulasevich wrote:
>> Hi David,
>>
>> Thanks for review! I added a warning and fixed the drop back value:
>> http://cr.openjdk.java.net/~bulasevich/8220656/webrev.02
> 
> With that change this code is pointless:
> 
>   274   if (FLAG_IS_DEFAULT(MaxVectorSize)) {
>   275     // FLAG_SET_DEFAULT(MaxVectorSize, has_simd() ? 16 : 8);
>   276     // SIMD/NEON can use 16, but default is 8 because currently
>   277     // larger than 8 will disable instruction scheduling
>   278     FLAG_SET_DEFAULT(MaxVectorSize, 8);
>   279   }
> 
> as it will be handled by your new code.

This code sets MaxVectorSize=8 when no -XX:MaxVectorSize option is 
specified while the new code sets 8/16 when the specified size is too 
big. I think these two blocks of code live together quite well :)

> Thanks,
> David
> 
>> regards,
>> Boris
>>
>> On 15.05.2019 4:16, David Holmes wrote:
>>> On 15/05/2019 6:48 am, dean.long at oracle.com wrote:
>>>> Looks fine, but you might want to print a warning if you override 
>>>> the user's choice.
>>>
>>> I would normally agree, but in this case the following code will 
>>> already silently overwrite the user's choice in some cases:
>>>
>>>   281   if (MaxVectorSize > 16) {
>>>   282     FLAG_SET_DEFAULT(MaxVectorSize, 8);
>>>   283   }
>>>
>>> and I find it odd that it drops it back to 8 rather than taking the 
>>> upper-limit of 16.
>>>
>>> To me the first problem here is that MaxVectorSize should be a 
>>> pd_global so each platform can just set the right default to begin 
>>> with, and set a correct allowed range for that platform. In fact with 
>>> a constraint function it could even incorporate the has_simd() check. 
>>> But that's getting a bit beyond the current fix.
>>>
>>> I can't see any non-convoluted way to print a warning under all the 
>>> right circumstances, so I'll leave it to Boris to decide what to do 
>>> here. I don't think a warning is essential.
>>>
>>> Cheers,
>>> David
>>>
>>>> dl
>>>>
>>>> On 5/14/19 8:39 AM, Boris Ulasevich wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review a simple change to prevent VLD1/VST1 instructions 
>>>>> generation on ARM32 processor without SIMD support. The change 
>>>>> overrides MaxVectorSize option set up by user to a value acceptable 
>>>>> by the hardware.
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8220656
>>>>> http://cr.openjdk.java.net/~bulasevich/8220656/webrev.01/
>>>>>
>>>>> thanks,
>>>>> Boris
>>>>