RFR: 8333343: [REDO] AArch64: optimize integer remainder [v4]

[Andrew Haley]

On Fri, 31 May 2024 00:45:33 GMT, Jin Guojie <duke at openjdk.org> wrote:

>> On some Arm processors, a separate multiply/subtract is actually faster than the combined instruction.
>> 
>> (1) The following test has passed, which shows performance improvement.
>> 
>> make test TEST="micro:java.lang.IntegerDivMod"
>> make test TEST="micro:java.lang.LongDivMod"
>> 
>> * IntegerDivMod.testDivideRemainderUnsigned baseline(ns/ops) 2223 with this pacth(ns/ops) 1885 improvement(%) 17.93%
>> 
>> * IntegerDivMod.testRemainderUnsigned baseline(ns/ops) 2225 with this pacth(ns/ops) 1885 improvement(%) 18.03%
>> 
>> * LongDivMod.testDivideRemainderUnsigned baseline(ns/ops) 2231 with this pacth(ns/ops) 1894 improvement(%) 17.79%
>> 
>> * LongDivMod.testRemainderUnsigned baseline(ns/ops) 2232 with this pacth(ns/ops) 1891 improvement(%) 18.03%
>> 
>> (2) jtreg test has passed
>> 
>> make run-test  TEST=tier1
>
> Jin Guojie has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into dev0530
>  - Merge branch 'dev0530' of https://github.com/jinguojie-alibaba/jdk into dev0530
>  - Merge branch 'openjdk:master' into dev0530
>  - MacroAssembler::msub() takes a scratch register as an argument
>  - 8331558: AArch64: optimize integer remainder
>    
>    On some Arm processors, a separate multiply/subtract is actually faster than the combined instruction.
>    
>    (1) The following test has passed, which shows performance improvement.
>    
>    make test TEST="micro:java.lang.IntegerDivMod"
>    make test TEST="micro:java.lang.LongDivMod"
>    
>    * IntegerDivMod.testDivideRemainderUnsigned
>      baseline(ns/ops) 2223
>      with this pacth(ns/ops) 1885
>      improvement(%) 17.93%
>    
>    * IntegerDivMod.testRemainderUnsigned
>      baseline(ns/ops) 2225
>      with this pacth(ns/ops) 1885
>      improvement(%) 18.03%
>    
>    * LongDivMod.testDivideRemainderUnsigned
>      baseline(ns/ops) 2231
>      with this pacth(ns/ops) 1894
>      improvement(%) 17.79%
>    
>    * LongDivMod.testRemainderUnsigned
>      baseline(ns/ops) 2232
>      with this pacth(ns/ops) 1891
>      improvement(%) 18.03%
>    
>    (2) jtreg test has passed
>    
>    make run-test  TEST=tier1

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 2298:

> 2296:   if (VM_Version::supports_a53mac() && Ra != zr)
> 2297:     nop();
> 2298:   if (VM_Version::is_neoverse()) {

Maybe a more descriptive name?
Suggestion:

  if (VM_Version::split_msub()) {


Is there a good reason why we're restricting this to msub and not madd?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19471#discussion_r1629015638