Re: RFR: 8276217: Harmonize StrictMath intrinsics handling [v3]
On Tue, 2 Nov 2021 06:25:33 GMT, Aleksey Shipilev wrote: >> This blocks JDK-8276215: `StrictMath` intrinsics are handled peculiarly by >> giving failing intrinsics a second chance to match against the similar >> `Math` intrinsics. This has interesting consequence for matchers: we can >> match the native `StrictMath.sqrt` to non-native intrinsic for `Math.sqrt`. >> Interpreter would then have to disambiguate the two. It could be made >> simpler and more consistent. >> >> For `min`/`max` methods, `StrictMath` already delegates to `Math` methods, >> so we can just drop the intrinsics for them. `sqrt` is harder to delegate, >> because it is `native` and a part of public API, so we can instead do the >> proper special intrinsic for it. >> >> There seem to be no performance regressions with this patch at least on >> Linux x86_64: >> >> >> $ CONF=linux-x86_64-server-release make test TEST="micro:StrictMathBench" >> >> Benchmark Mode Cnt Score Error Units >> >> ### Before >> >> StrictMathBench.minDouble thrpt4 230921.558 ± 234.238 ops/ms >> StrictMathBench.minFloat thrpt4 230932.303 ± 126.721 ops/ms >> StrictMathBench.minInt thrpt4 230917.256 ± 73.008 ops/ms >> StrictMathBench.minLongthrpt4 194460.828 ± 178.079 ops/ms >> >> >> StrictMathBench.maxDouble thrpt4 230983.180 ± 161.211 ops/ms >> StrictMathBench.maxFloat thrpt4 230969.290 ± 277.500 ops/ms >> StrictMathBench.maxInt thrpt4 231033.581 ± 200.015 ops/ms >> StrictMathBench.maxLongthrpt4 194590.744 ± 114.295 ops/ms >> >> >> StrictMathBench.sqrtDouble thrpt4 230722.037 ± .080 ops/ms >> >> ### After >> >> StrictMathBench.minDouble thrpt4 230976.625 ± 67.338 ops/ms >> StrictMathBench.minFloat thrpt4 230896.021 ± 270.434 ops/ms >> StrictMathBench.minInt thrpt4 230859.741 ± 403.147 ops/ms >> StrictMathBench.minLongthrpt4 194456.673 ± 111.557 ops/ms >> >> StrictMathBench.maxDouble thrpt4 230890.776 ± 89.924 ops/ms >> StrictMathBench.maxFloat thrpt4 230918.334 ± 63.160 ops/ms >> StrictMathBench.maxInt thrpt4 231059.128 ± 51.224 ops/ms >> StrictMathBench.maxLongthrpt4 194488.210 ± 495.224 ops/ms >> >> StrictMathBench.sqrtDouble thrpt4 231023.703 ± 247.330 ops/ms >> >> >> Additional testing: >> - [x] `StrictMath` benchmarks >> - [x] Linux x86_64 fastdebug `java/lang/StrictMath`, `java/lang/Math` >> - [x] Linux x86_64 fastdebug `tier1` > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Keep intrinsics on StrictMath Thanks! I am going to push this tomorrow morning, if no other comments show up. - PR: https://git.openjdk.java.net/jdk/pull/6184
Re: RFR: 8276217: Harmonize StrictMath intrinsics handling [v3]
On Tue, 2 Nov 2021 06:25:33 GMT, Aleksey Shipilev wrote: >> This blocks JDK-8276215: `StrictMath` intrinsics are handled peculiarly by >> giving failing intrinsics a second chance to match against the similar >> `Math` intrinsics. This has interesting consequence for matchers: we can >> match the native `StrictMath.sqrt` to non-native intrinsic for `Math.sqrt`. >> Interpreter would then have to disambiguate the two. It could be made >> simpler and more consistent. >> >> For `min`/`max` methods, `StrictMath` already delegates to `Math` methods, >> so we can just drop the intrinsics for them. `sqrt` is harder to delegate, >> because it is `native` and a part of public API, so we can instead do the >> proper special intrinsic for it. >> >> There seem to be no performance regressions with this patch at least on >> Linux x86_64: >> >> >> $ CONF=linux-x86_64-server-release make test TEST="micro:StrictMathBench" >> >> Benchmark Mode Cnt Score Error Units >> >> ### Before >> >> StrictMathBench.minDouble thrpt4 230921.558 ± 234.238 ops/ms >> StrictMathBench.minFloat thrpt4 230932.303 ± 126.721 ops/ms >> StrictMathBench.minInt thrpt4 230917.256 ± 73.008 ops/ms >> StrictMathBench.minLongthrpt4 194460.828 ± 178.079 ops/ms >> >> >> StrictMathBench.maxDouble thrpt4 230983.180 ± 161.211 ops/ms >> StrictMathBench.maxFloat thrpt4 230969.290 ± 277.500 ops/ms >> StrictMathBench.maxInt thrpt4 231033.581 ± 200.015 ops/ms >> StrictMathBench.maxLongthrpt4 194590.744 ± 114.295 ops/ms >> >> >> StrictMathBench.sqrtDouble thrpt4 230722.037 ± .080 ops/ms >> >> ### After >> >> StrictMathBench.minDouble thrpt4 230976.625 ± 67.338 ops/ms >> StrictMathBench.minFloat thrpt4 230896.021 ± 270.434 ops/ms >> StrictMathBench.minInt thrpt4 230859.741 ± 403.147 ops/ms >> StrictMathBench.minLongthrpt4 194456.673 ± 111.557 ops/ms >> >> StrictMathBench.maxDouble thrpt4 230890.776 ± 89.924 ops/ms >> StrictMathBench.maxFloat thrpt4 230918.334 ± 63.160 ops/ms >> StrictMathBench.maxInt thrpt4 231059.128 ± 51.224 ops/ms >> StrictMathBench.maxLongthrpt4 194488.210 ± 495.224 ops/ms >> >> StrictMathBench.sqrtDouble thrpt4 231023.703 ± 247.330 ops/ms >> >> >> Additional testing: >> - [x] `StrictMath` benchmarks >> - [x] Linux x86_64 fastdebug `java/lang/StrictMath`, `java/lang/Math` >> - [x] Linux x86_64 fastdebug `tier1` > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Keep intrinsics on StrictMath Marked as reviewed by aph (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6184
Re: RFR: 8276217: Harmonize StrictMath intrinsics handling [v3]
On Tue, 2 Nov 2021 06:25:33 GMT, Aleksey Shipilev wrote: >> This blocks JDK-8276215: `StrictMath` intrinsics are handled peculiarly by >> giving failing intrinsics a second chance to match against the similar >> `Math` intrinsics. This has interesting consequence for matchers: we can >> match the native `StrictMath.sqrt` to non-native intrinsic for `Math.sqrt`. >> Interpreter would then have to disambiguate the two. It could be made >> simpler and more consistent. >> >> For `min`/`max` methods, `StrictMath` already delegates to `Math` methods, >> so we can just drop the intrinsics for them. `sqrt` is harder to delegate, >> because it is `native` and a part of public API, so we can instead do the >> proper special intrinsic for it. >> >> There seem to be no performance regressions with this patch at least on >> Linux x86_64: >> >> >> $ CONF=linux-x86_64-server-release make test TEST="micro:StrictMathBench" >> >> Benchmark Mode Cnt Score Error Units >> >> ### Before >> >> StrictMathBench.minDouble thrpt4 230921.558 ± 234.238 ops/ms >> StrictMathBench.minFloat thrpt4 230932.303 ± 126.721 ops/ms >> StrictMathBench.minInt thrpt4 230917.256 ± 73.008 ops/ms >> StrictMathBench.minLongthrpt4 194460.828 ± 178.079 ops/ms >> >> >> StrictMathBench.maxDouble thrpt4 230983.180 ± 161.211 ops/ms >> StrictMathBench.maxFloat thrpt4 230969.290 ± 277.500 ops/ms >> StrictMathBench.maxInt thrpt4 231033.581 ± 200.015 ops/ms >> StrictMathBench.maxLongthrpt4 194590.744 ± 114.295 ops/ms >> >> >> StrictMathBench.sqrtDouble thrpt4 230722.037 ± .080 ops/ms >> >> ### After >> >> StrictMathBench.minDouble thrpt4 230976.625 ± 67.338 ops/ms >> StrictMathBench.minFloat thrpt4 230896.021 ± 270.434 ops/ms >> StrictMathBench.minInt thrpt4 230859.741 ± 403.147 ops/ms >> StrictMathBench.minLongthrpt4 194456.673 ± 111.557 ops/ms >> >> StrictMathBench.maxDouble thrpt4 230890.776 ± 89.924 ops/ms >> StrictMathBench.maxFloat thrpt4 230918.334 ± 63.160 ops/ms >> StrictMathBench.maxInt thrpt4 231059.128 ± 51.224 ops/ms >> StrictMathBench.maxLongthrpt4 194488.210 ± 495.224 ops/ms >> >> StrictMathBench.sqrtDouble thrpt4 231023.703 ± 247.330 ops/ms >> >> >> Additional testing: >> - [x] `StrictMath` benchmarks >> - [x] Linux x86_64 fastdebug `java/lang/StrictMath`, `java/lang/Math` >> - [x] Linux x86_64 fastdebug `tier1` > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Keep intrinsics on StrictMath Thanks! I re-ran the tests, they seem to be fine. I need a second (R)eviewer for this. - PR: https://git.openjdk.java.net/jdk/pull/6184
Re: RFR: 8276217: Harmonize StrictMath intrinsics handling [v3]
On Tue, 2 Nov 2021 06:25:33 GMT, Aleksey Shipilev wrote: >> This blocks JDK-8276215: `StrictMath` intrinsics are handled peculiarly by >> giving failing intrinsics a second chance to match against the similar >> `Math` intrinsics. This has interesting consequence for matchers: we can >> match the native `StrictMath.sqrt` to non-native intrinsic for `Math.sqrt`. >> Interpreter would then have to disambiguate the two. It could be made >> simpler and more consistent. >> >> For `min`/`max` methods, `StrictMath` already delegates to `Math` methods, >> so we can just drop the intrinsics for them. `sqrt` is harder to delegate, >> because it is `native` and a part of public API, so we can instead do the >> proper special intrinsic for it. >> >> There seem to be no performance regressions with this patch at least on >> Linux x86_64: >> >> >> $ CONF=linux-x86_64-server-release make test TEST="micro:StrictMathBench" >> >> Benchmark Mode Cnt Score Error Units >> >> ### Before >> >> StrictMathBench.minDouble thrpt4 230921.558 ± 234.238 ops/ms >> StrictMathBench.minFloat thrpt4 230932.303 ± 126.721 ops/ms >> StrictMathBench.minInt thrpt4 230917.256 ± 73.008 ops/ms >> StrictMathBench.minLongthrpt4 194460.828 ± 178.079 ops/ms >> >> >> StrictMathBench.maxDouble thrpt4 230983.180 ± 161.211 ops/ms >> StrictMathBench.maxFloat thrpt4 230969.290 ± 277.500 ops/ms >> StrictMathBench.maxInt thrpt4 231033.581 ± 200.015 ops/ms >> StrictMathBench.maxLongthrpt4 194590.744 ± 114.295 ops/ms >> >> >> StrictMathBench.sqrtDouble thrpt4 230722.037 ± .080 ops/ms >> >> ### After >> >> StrictMathBench.minDouble thrpt4 230976.625 ± 67.338 ops/ms >> StrictMathBench.minFloat thrpt4 230896.021 ± 270.434 ops/ms >> StrictMathBench.minInt thrpt4 230859.741 ± 403.147 ops/ms >> StrictMathBench.minLongthrpt4 194456.673 ± 111.557 ops/ms >> >> StrictMathBench.maxDouble thrpt4 230890.776 ± 89.924 ops/ms >> StrictMathBench.maxFloat thrpt4 230918.334 ± 63.160 ops/ms >> StrictMathBench.maxInt thrpt4 231059.128 ± 51.224 ops/ms >> StrictMathBench.maxLongthrpt4 194488.210 ± 495.224 ops/ms >> >> StrictMathBench.sqrtDouble thrpt4 231023.703 ± 247.330 ops/ms >> >> >> Additional testing: >> - [x] `StrictMath` benchmarks >> - [x] Linux x86_64 fastdebug `tier1` > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Keep intrinsics on StrictMath Good. Thank you for fixing it. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6184
Re: RFR: 8276217: Harmonize StrictMath intrinsics handling [v3]
> This blocks JDK-8276215: `StrictMath` intrinsics are handled peculiarly by > giving failing intrinsics a second chance to match against the similar `Math` > intrinsics. This has interesting consequence for matchers: we can match the > native `StrictMath.sqrt` to non-native intrinsic for `Math.sqrt`. Interpreter > would then have to disambiguate the two. It could be made simpler and more > consistent. > > For `min`/`max` methods, `StrictMath` already delegates to `Math` methods, so > we can just drop the intrinsics for them. `sqrt` is harder to delegate, > because it is `native` and a part of public API, so we can instead do the > proper special intrinsic for it. > > There seem to be no performance regressions with this patch at least on Linux > x86_64: > > > $ CONF=linux-x86_64-server-release make test TEST="micro:StrictMathBench" > > Benchmark Mode Cnt Score Error Units > > ### Before > > StrictMathBench.minDouble thrpt4 230921.558 ± 234.238 ops/ms > StrictMathBench.minFloat thrpt4 230932.303 ± 126.721 ops/ms > StrictMathBench.minInt thrpt4 230917.256 ± 73.008 ops/ms > StrictMathBench.minLongthrpt4 194460.828 ± 178.079 ops/ms > > > StrictMathBench.maxDouble thrpt4 230983.180 ± 161.211 ops/ms > StrictMathBench.maxFloat thrpt4 230969.290 ± 277.500 ops/ms > StrictMathBench.maxInt thrpt4 231033.581 ± 200.015 ops/ms > StrictMathBench.maxLongthrpt4 194590.744 ± 114.295 ops/ms > > > StrictMathBench.sqrtDouble thrpt4 230722.037 ± .080 ops/ms > > ### After > > StrictMathBench.minDouble thrpt4 230976.625 ± 67.338 ops/ms > StrictMathBench.minFloat thrpt4 230896.021 ± 270.434 ops/ms > StrictMathBench.minInt thrpt4 230859.741 ± 403.147 ops/ms > StrictMathBench.minLongthrpt4 194456.673 ± 111.557 ops/ms > > StrictMathBench.maxDouble thrpt4 230890.776 ± 89.924 ops/ms > StrictMathBench.maxFloat thrpt4 230918.334 ± 63.160 ops/ms > StrictMathBench.maxInt thrpt4 231059.128 ± 51.224 ops/ms > StrictMathBench.maxLongthrpt4 194488.210 ± 495.224 ops/ms > > StrictMathBench.sqrtDouble thrpt4 231023.703 ± 247.330 ops/ms > > > Additional testing: > - [x] `StrictMath` benchmarks > - [x] Linux x86_64 fastdebug `tier1` Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision: Keep intrinsics on StrictMath - Changes: - all: https://git.openjdk.java.net/jdk/pull/6184/files - new: https://git.openjdk.java.net/jdk/pull/6184/files/27202fa4..005cace6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6184=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6184=01-02 Stats: 67 lines in 5 files changed: 55 ins; 5 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/6184.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6184/head:pull/6184 PR: https://git.openjdk.java.net/jdk/pull/6184
