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

[David Holmes]

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.

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
>>>