RFR: 8222029: Optimize Math.floorMod

[Claes Redestad]

I found a few missing corner cases in the existing test (all
permutations of [MIN|MAX]/[MIN|MAX]), so I added them and verified
the test pass both with and without my patch:

http://cr.openjdk.java.net/~redestad/8222029/open.01/

/Claes

On 2019-04-09 21:06, Claes Redestad wrote:
> In my cursory analysis of the test the interesting corner cases are
> covered and I have no reason to believe we'd see spurious errors
> elsewhere. I ran an adhoc semi-exhaustive test comparing results
> of the new version with the old and got an all-pass.
> 
> /Claes
> 
> On 2019-04-09 18:02, Joe Darcy wrote:
>> Basically I'm inquiring about whether the existing tests provide at 
>> least as good code coverage on the new implementation as the old one. 
>> As it is a relatively simple method, perhaps it there is full coverage 
>> before and after. However, at times changing the implementation 
>> requires updates to the tests to includes different cases to check and 
>> I wanted to make sure that was looked at here.
>>
>> Thanks,
>>
>> -Joe
>>
>> On 4/9/2019 5:32 AM, Claes Redestad wrote:
>>> I think those tests cover all interesting corner cases, so the only way
>>> I see it can be improved is to make it more exhaustive (say generate a
>>> large random sample of tests every run). Do you feel that is needed?
>>>
>>> /Claes
>>>
>>> On 2019-04-09 01:35, Joseph D. Darcy wrote:
>>>> Should any additional cases be added to 
>>>> test/jdk/java/lang/Math/DivModTests.java to cover the new 
>>>> implementation?
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>> On 4/5/2019 10:21 AM, Claes Redestad wrote:
>>>>>
>>>>>
>>>>> On 2019-04-05 17:41, Andrew Haley wrote:
>>>>>> On 4/5/19 2:44 PM, Claes Redestad wrote:
>>>>>>> Testing: tier1-2, all Math tests run locally, -prof perfasm 
>>>>>>> verification
>>>>>>> on the provided microbenchmark.
>>>>>>
>>>>>> Looks good.
>>>>>
>>>>> Thanks!
>>>>>
>>>>>>
>>>>>> I've kicked the tyres on AArch64, and it looks like a useful 
>>>>>> optimization. The
>>>>>> gains when the divisor is constant (a common case) are modest but 
>>>>>> worthwhile.
>>>>>>
>>>>>
>>>>> Thanks for trying it out and glad to hear it helps on AArch64 as well.
>>>>>
>>>>> /Claes
>>>>