RFR: 8302191: Performance degradation for float/double modulo on Linux [v2]

[David Holmes]

On Tue, 21 Feb 2023 13:51:10 GMT, Jan Kratochvil <duke at openjdk.org> wrote:

>> I have OCA already processed/approved. I am not Author but my Author request is being processed these days (sent to Rob McKenna).
>> I did regression test x86_64 OpenJDK-8. I will leave other regression testing on GHA.
>> The patch (and former GCC performance regression) affects only x86_64+i686.
>
> Jan Kratochvil 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 one additional commit since the last revision:
> 
>   8302191: Performance degradation for float/double modulo on Linux

Thanks for the updates. A few minor comments. I need to look into effective testing and benchmarking for this change.

src/hotspot/cpu/x86/sharedRuntime_x86.cpp line 87:

> 85: #endif //COMPILER1
> 86: 
> 87: JRT_LEAF(jfloat, SharedRuntime::frem(jfloat  x, jfloat  y))

Nit: extra space before x and y.

src/hotspot/share/runtime/sharedRuntime.cpp line 236:

> 234: const julong double_infinity  = CONST64(0x7FF0000000000000);
> 235: 
> 236: #ifndef X86

I wonder if the WIN64 workaround is actually needed/valid for non-X86 windows?

test/micro/org/openjdk/bench/vm/floatingpoint/DremFrem.java line 2:

> 1: /*
> 2:  * Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved.

Oracle did not write this code so it should have your own copyright on it, unless you copied it from other OpenJDK code in which case it could have dual copyright.

test/micro/org/openjdk/bench/vm/floatingpoint/DremFrem.java line 9:

> 7:  * published by the Free Software Foundation.  Oracle designates this
> 8:  * particular file as subject to the "Classpath" exception as provided
> 9:  * by Oracle in the LICENSE file that accompanied this code.

I don't think the Classpath exception is relevant to this code.

test/micro/org/openjdk/bench/vm/floatingpoint/DremFrem.java line 44:

> 42:  * Java bytecode drem is defined as C fmod (not C drem==remainder).
> 43:  * GCC since 93ba85fdd253b4b9cf2b9e54e8e5969b1a3db098 has slow fmod().
> 44:  * Testcase is based on: https://stackoverflow.com/a/55673220/2995591

That could be a problem. What is the license for code shown on StackOverflow?

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

PR: https://git.openjdk.org/jdk/pull/12508