Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v4]
On Wed, 23 Mar 2022 02:03:26 GMT, Fei Yang wrote:
>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the
>> https://openjdk.java.net/projects/riscv-port branch.
>>
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are
>> also carried out regularly. So it should be good enough to run most Java
>> programs.
>>
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix copyright header
Update looks good.
Testing results are also good.
-
Marked as reviewed by kvn (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Tue, 22 Mar 2022 11:50:13 GMT, Fei Yang wrote:
>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the
>> https://openjdk.java.net/projects/riscv-port branch.
>>
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are
>> also carried out regularly. So it should be good enough to run most Java
>> programs.
>>
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Address review comments
src/hotspot/cpu/riscv/disassembler_riscv.hpp line 18:
> 16: *
> 17: * You should have received a copy of the GNU General Public License
> version
> 18: * 2 along with this work; if not, write to the Free Software Foundation,
> * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
These 2 lines merged into 1 accidentally causing failure in copyright headers
verification.
-
PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Tue, 22 Mar 2022 11:50:13 GMT, Fei Yang wrote:
>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the
>> https://openjdk.java.net/projects/riscv-port branch.
>>
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are
>> also carried out regularly. So it should be good enough to run most Java
>> programs.
>>
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Address review comments
I looked on C1/C2 changes and compiler tests. Seems reasonable.
But before approval I would need to run changes through our testing.
-
PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8280182: HotSpot Style Guide has stale link to chromium style guide
On Tue, 18 Jan 2022 23:09:12 GMT, Liam Miller-Cushon wrote: > Update links to the chromium style guide in the HotSpot Style Guide. I agree that for small changes like this the process should be simplified. - PR: https://git.openjdk.java.net/jdk/pull/7138
Re: RFR: 8276025: Hotspot's libsvml.so may conflict with user dependency [v2]
On Thu, 4 Nov 2021 18:15:08 GMT, Sandhya Viswanathan wrote: >> Looks good. I will run tests. > > Thanks a lot @vnkozlov. @sviswa7 testing passed you can integrate. - PR: https://git.openjdk.java.net/jdk/pull/6265
Re: RFR: 8276025: Hotspot's libsvml.so may conflict with user dependency [v2]
On Thu, 4 Nov 2021 19:49:08 GMT, Sandhya Viswanathan wrote: >> This patch removes conflicts with libsvml.so distributed with Intel's MKL >> library: >> Renames exported symbols from __svml to __jsvml. >> Renames library from libsvml.so to libjsvml.so. >> Updates the stubGenerator_x86_64.cpp accordingly to load libjsvml.so and >> the renamed symbols. >> Updates tests to look for the new library. >> >> Please review. >> >> Best Regards, >> Sandhya > > Sandhya Viswanathan has updated the pull request incrementally with one > additional commit since the last revision: > > change filename to jsvml* There is also need to update one closed test we have before integration. - PR: https://git.openjdk.java.net/jdk/pull/6265
Re: RFR: 8276025: Hotspot's libsvml.so may conflict with user dependency [v2]
On Thu, 4 Nov 2021 19:49:08 GMT, Sandhya Viswanathan wrote: >> This patch removes conflicts with libsvml.so distributed with Intel's MKL >> library: >> Renames exported symbols from __svml to __jsvml. >> Renames library from libsvml.so to libjsvml.so. >> Updates the stubGenerator_x86_64.cpp accordingly to load libjsvml.so and >> the renamed symbols. >> Updates tests to look for the new library. >> >> Please review. >> >> Best Regards, >> Sandhya > > Sandhya Viswanathan has updated the pull request incrementally with one > additional commit since the last revision: > > change filename to jsvml* Good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6265
Re: RFR: 8276025: Hotspot's libsvml.so may conflict with user dependency
On Thu, 4 Nov 2021 17:48:56 GMT, Sandhya Viswanathan wrote: > This patch removes conflicts with libsvml.so distributed with Intel's MKL > library: > Renames exported symbols from __svml to __jsvml. > Renames library from libsvml.so to libjsvml.so. > Updates the stubGenerator_x86_64.cpp accordingly to load libjsvml.so and > the renamed symbols. > Updates tests to look for the new library. > > Please review. > > Best Regards, > Sandhya For completeness may be rename files too: jsvml_*.S and jsvml_*.S.inc - PR: https://git.openjdk.java.net/jdk/pull/6265
Re: RFR: 8276025: Hotspot's libsvml.so may conflict with user dependency
On Thu, 4 Nov 2021 17:48:56 GMT, Sandhya Viswanathan wrote: > This patch removes conflicts with libsvml.so distributed with Intel's MKL > library: > Renames exported symbols from __svml to __jsvml. > Renames library from libsvml.so to libjsvml.so. > Updates the stubGenerator_x86_64.cpp accordingly to load libjsvml.so and > the renamed symbols. > Updates tests to look for the new library. > > Please review. > > Best Regards, > Sandhya Looks good. I will run tests. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6265
Re: RFR: 8274329: Fix non-portable HotSpot code in MethodMatcher::parse_method_pattern [v3]
On Wed, 6 Oct 2021 05:17:28 GMT, Jie Fu wrote:
>> Hi all,
>>
>> I tried to build OpenJDK on Cygwin (Windows 2016 + VS2019).
>> However, I failed with C4474 and C4778 warnings as below:
>>
>> Compiling 100 properties into resource bundles for java.desktop
>> Compiling 3038 files for java.base
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): error
>> C2220: the following warning is treated as an error
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning
>> C4778: 'sscanf' : unterminated format string
>> '%255[*\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e\x97%n'
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning
>> C4474: 'sscanf' : too many arguments passed for format string
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): note:
>> placeholders and their parameters expect 1 variadic arguments, but 3 were
>> provided
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning
>> C4778: 'sscanf' : unterminated format string
>> '%1022[[);/\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e%n'
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning
>> C4474: 'sscanf' : too many arguments passed for format string
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): note:
>> placeholders and their parameters expect 0 variadic arguments, but 2 were
>> provided
>>
>>
>> The failure is caused by non-ASCII chars in the format string of sscanf
>> [1][2], which is non-portable on our Windows platform.
>> In fact, these non-ASCII coding also triggers C4819 warning, which had been
>> disabled in JDK-8216154 [3].
>> And I also found an article showing that sscanf may fail with non-ASCII in
>> the format string [4].
>>
>> So it would be nice to remove these non-ASCII chars (`\x80 ~ \xef`).
>> And I think it's safe to do so.
>>
>> This is because:
>> 1) There are actually no non-ASCII chars for package/class/method/signature
>> names.
>> 2) I don't think there is a use case, in which people will input non-ASCII
>> for `CompileCommand`.
>>
>> You may argue that the non-ASCII may be used by the parser itself.
>> But I didn't find that usage at all. (Please let me know if I miss
>> something.)
>>
>> So I suggest to remove these non-ASCII code to make HotSpot to be more
>> portable.
>> And if we do so, we can also remove the only one
>> `PRAGMA_DISABLE_MSVC_WARNING(4819)` [5].
>>
>> Testing:
>> - Build tests on Windows
>> - tier1~3 on Linux/x64
>>
>> Thanks.
>> Best regards,
>> Jie
>>
>> [1]
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L269
>> [2]
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L319
>> [3]
>> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-January/032014.html
>> [4] https://jeffpar.github.io/kbarchive/kb/047/Q47369/
>> [5]
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L246
>
> Jie Fu has updated the pull request incrementally with one additional commit
> since the last revision:
>
> Split with RANGEBASE_ASCII and RANGEBASE_NON_ASCII
Passed my tier1-3 testing
-
Marked as reviewed by kvn (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5704
Re: RFR: 8274329: Fix non-portable HotSpot code in MethodMatcher::parse_method_pattern [v3]
On Wed, 6 Oct 2021 05:17:28 GMT, Jie Fu wrote:
>> Hi all,
>>
>> I tried to build OpenJDK on Cygwin (Windows 2016 + VS2019).
>> However, I failed with C4474 and C4778 warnings as below:
>>
>> Compiling 100 properties into resource bundles for java.desktop
>> Compiling 3038 files for java.base
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): error
>> C2220: the following warning is treated as an error
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning
>> C4778: 'sscanf' : unterminated format string
>> '%255[*\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e\x97%n'
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning
>> C4474: 'sscanf' : too many arguments passed for format string
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): note:
>> placeholders and their parameters expect 1 variadic arguments, but 3 were
>> provided
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning
>> C4778: 'sscanf' : unterminated format string
>> '%1022[[);/\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e%n'
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning
>> C4474: 'sscanf' : too many arguments passed for format string
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): note:
>> placeholders and their parameters expect 0 variadic arguments, but 2 were
>> provided
>>
>>
>> The failure is caused by non-ASCII chars in the format string of sscanf
>> [1][2], which is non-portable on our Windows platform.
>> In fact, these non-ASCII coding also triggers C4819 warning, which had been
>> disabled in JDK-8216154 [3].
>> And I also found an article showing that sscanf may fail with non-ASCII in
>> the format string [4].
>>
>> So it would be nice to remove these non-ASCII chars (`\x80 ~ \xef`).
>> And I think it's safe to do so.
>>
>> This is because:
>> 1) There are actually no non-ASCII chars for package/class/method/signature
>> names.
>> 2) I don't think there is a use case, in which people will input non-ASCII
>> for `CompileCommand`.
>>
>> You may argue that the non-ASCII may be used by the parser itself.
>> But I didn't find that usage at all. (Please let me know if I miss
>> something.)
>>
>> So I suggest to remove these non-ASCII code to make HotSpot to be more
>> portable.
>> And if we do so, we can also remove the only one
>> `PRAGMA_DISABLE_MSVC_WARNING(4819)` [5].
>>
>> Testing:
>> - Build tests on Windows
>> - tier1~3 on Linux/x64
>>
>> Thanks.
>> Best regards,
>> Jie
>>
>> [1]
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L269
>> [2]
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L319
>> [3]
>> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-January/032014.html
>> [4] https://jeffpar.github.io/kbarchive/kb/047/Q47369/
>> [5]
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L246
>
> Jie Fu has updated the pull request incrementally with one additional commit
> since the last revision:
>
> Split with RANGEBASE_ASCII and RANGEBASE_NON_ASCII
Looks good to me. Let me test it before approval.
-
PR: https://git.openjdk.java.net/jdk/pull/5704
Re: RFR: 8274329: Fix non-portable HotSpot code in MethodMatcher::parse_method_pattern
On Sun, 26 Sep 2021 09:55:00 GMT, Jie Fu wrote:
> Hi all,
>
> I tried to build OpenJDK on Cygwin (Windows 2016 + VS2019).
> However, I failed with C4474 and C4778 warnings as below:
>
> Compiling 100 properties into resource bundles for java.desktop
> Compiling 3038 files for java.base
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): error
> C2220: the following warning is treated as an error
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning
> C4778: 'sscanf' : unterminated format string
> '%255[*\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e\x97%n'
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning
> C4474: 'sscanf' : too many arguments passed for format string
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): note:
> placeholders and their parameters expect 1 variadic arguments, but 3 were
> provided
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning
> C4778: 'sscanf' : unterminated format string
> '%1022[[);/\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e%n'
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning
> C4474: 'sscanf' : too many arguments passed for format string
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): note:
> placeholders and their parameters expect 0 variadic arguments, but 2 were
> provided
>
>
> The failure is caused by non-ASCII chars in the format string of sscanf
> [1][2], which is non-portable on our Windows platform.
> In fact, these non-ASCII coding also triggers C4819 warning, which had been
> disabled in JDK-8216154 [3].
> And I also found an article showing that sscanf may fail with non-ASCII in
> the format string [4].
>
> So it would be nice to remove these non-ASCII chars (`\x80 ~ \xef`).
> And I think it's safe to do so.
>
> This is because:
> 1) There are actually no non-ASCII chars for package/class/method/signature
> names.
> 2) I don't think there is a use case, in which people will input non-ASCII
> for `CompileCommand`.
>
> You may argue that the non-ASCII may be used by the parser itself.
> But I didn't find that usage at all. (Please let me know if I miss something.)
>
> So I suggest to remove these non-ASCII code to make HotSpot to be more
> portable.
> And if we do so, we can also remove the only one
> `PRAGMA_DISABLE_MSVC_WARNING(4819)` [5].
>
> Testing:
> - Build tests on Windows
> - tier1~3 on Linux/x64
>
> Thanks.
> Best regards,
> Jie
>
> [1]
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L269
> [2]
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L319
> [3]
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-January/032014.html
> [4] https://jeffpar.github.io/kbarchive/kb/047/Q47369/
> [5]
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L246
`RANGEBASE` was added by
[JDK-6500501](https://bugs.openjdk.java.net/browse/JDK-6500501) and later was
modified by [JDK-8027829](https://bugs.openjdk.java.net/browse/JDK-8027829)
Note the original comment from 6500501:
// The characters allowed in a class or method name. All characters > 0x7f
// are allowed in order to handle obfuscated class files (e.g. Volano)
-
PR: https://git.openjdk.java.net/jdk/pull/5704
Re: RFR: 8274329: Fix non-portable HotSpot code in MethodMatcher::parse_method_pattern
On Sun, 26 Sep 2021 09:55:00 GMT, Jie Fu wrote:
> Hi all,
>
> I tried to build OpenJDK on Cygwin (Windows 2016 + VS2019).
> However, I failed with C4474 and C4778 warnings as below:
>
> Compiling 100 properties into resource bundles for java.desktop
> Compiling 3038 files for java.base
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): error
> C2220: the following warning is treated as an error
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning
> C4778: 'sscanf' : unterminated format string
> '%255[*\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e\x97%n'
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning
> C4474: 'sscanf' : too many arguments passed for format string
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): note:
> placeholders and their parameters expect 1 variadic arguments, but 3 were
> provided
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning
> C4778: 'sscanf' : unterminated format string
> '%1022[[);/\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e%n'
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning
> C4474: 'sscanf' : too many arguments passed for format string
> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): note:
> placeholders and their parameters expect 0 variadic arguments, but 2 were
> provided
>
>
> The failure is caused by non-ASCII chars in the format string of sscanf
> [1][2], which is non-portable on our Windows platform.
> In fact, these non-ASCII coding also triggers C4819 warning, which had been
> disabled in JDK-8216154 [3].
> And I also found an article showing that sscanf may fail with non-ASCII in
> the format string [4].
>
> So it would be nice to remove these non-ASCII chars (`\x80 ~ \xef`).
> And I think it's safe to do so.
>
> This is because:
> 1) There are actually no non-ASCII chars for package/class/method/signature
> names.
> 2) I don't think there is a use case, in which people will input non-ASCII
> for `CompileCommand`.
>
> You may argue that the non-ASCII may be used by the parser itself.
> But I didn't find that usage at all. (Please let me know if I miss something.)
>
> So I suggest to remove these non-ASCII code to make HotSpot to be more
> portable.
> And if we do so, we can also remove the only one
> `PRAGMA_DISABLE_MSVC_WARNING(4819)` [5].
>
> Testing:
> - Build tests on Windows
> - tier1~3 on Linux/x64
>
> Thanks.
> Best regards,
> Jie
>
> [1]
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L269
> [2]
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L319
> [3]
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-January/032014.html
> [4] https://jeffpar.github.io/kbarchive/kb/047/Q47369/
> [5]
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L246
This should be discussed with `build` group and may be `runtime` to get more
comments.
-
PR: https://git.openjdk.java.net/jdk/pull/5704
Re: RFR: 8273459: Update code segment alignment to 64 bytes [v4]
On Tue, 28 Sep 2021 17:31:24 GMT, Scott Gibbons wrote: >> Change the default code entry alignment to 64 bytes from 32 bytes. This >> allows for maintaining proper 64-byte alignment of data within a code >> segment, which is required by several AVX-512 instructions. >> >> I ran into this while implementing Base64 encoding and decoding. Code >> segments which were allocated with the address mod 32 == 0 but with the >> address mod 64 != 0 would cause the align() macro to misalign. This is >> because the align macro aligns to the size of the code segment and not the >> offset of the PC. So align(64) would align the PC to a multiple of 64 bytes >> from the start of the segment, and not to a pure 64-byte boundary as >> requested. Changing the alignment of the segment to 64 bytes fixes the >> issue. >> >> I have not seen any measurable difference in either performance or memory >> usage with the tests I have run. >> >> See [this >> ](https://mail.openjdk.java.net/pipermail/hotspot-dev/2021-August/054180.html) >> article for the discussion thread. > > Scott Gibbons has updated the pull request incrementally with two additional > commits since the last revision: > > - Merge branch 'asgibbons-align-fix' of https://github.com/asgibbons/jdk > into asgibbons-align-fix > - Revert .gitignore. Add comments and assert in align(). My testing passed. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5547
Re: RFR: 8273459: Update code segment alignment to 64 bytes [v4]
On Tue, 28 Sep 2021 17:31:24 GMT, Scott Gibbons wrote: >> Change the default code entry alignment to 64 bytes from 32 bytes. This >> allows for maintaining proper 64-byte alignment of data within a code >> segment, which is required by several AVX-512 instructions. >> >> I ran into this while implementing Base64 encoding and decoding. Code >> segments which were allocated with the address mod 32 == 0 but with the >> address mod 64 != 0 would cause the align() macro to misalign. This is >> because the align macro aligns to the size of the code segment and not the >> offset of the PC. So align(64) would align the PC to a multiple of 64 bytes >> from the start of the segment, and not to a pure 64-byte boundary as >> requested. Changing the alignment of the segment to 64 bytes fixes the >> issue. >> >> I have not seen any measurable difference in either performance or memory >> usage with the tests I have run. >> >> See [this >> ](https://mail.openjdk.java.net/pipermail/hotspot-dev/2021-August/054180.html) >> article for the discussion thread. > > Scott Gibbons has updated the pull request incrementally with two additional > commits since the last revision: > > - Merge branch 'asgibbons-align-fix' of https://github.com/asgibbons/jdk > into asgibbons-align-fix > - Revert .gitignore. Add comments and assert in align(). Good. Let me test it before you push. - PR: https://git.openjdk.java.net/jdk/pull/5547
Re: RFR: 8273459: Update code segment alignment to 64 bytes
On Tue, 28 Sep 2021 01:10:53 GMT, Scott Gibbons wrote: >> Change the default code entry alignment to 64 bytes from 32 bytes. This >> allows for maintaining proper 64-byte alignment of data within a code >> segment, which is required by several AVX-512 instructions. >> >> I ran into this while implementing Base64 encoding and decoding. Code >> segments which were allocated with the address mod 32 == 0 but with the >> address mod 64 != 0 would cause the align() macro to misalign. This is >> because the align macro aligns to the size of the code segment and not the >> offset of the PC. So align(64) would align the PC to a multiple of 64 bytes >> from the start of the segment, and not to a pure 64-byte boundary as >> requested. Changing the alignment of the segment to 64 bytes fixes the >> issue. >> >> I have not seen any measurable difference in either performance or memory >> usage with the tests I have run. >> >> See [this >> ](https://mail.openjdk.java.net/pipermail/hotspot-dev/2021-August/054180.html) >> article for the discussion thread. > > Reverted `CodeEntryAlignment` to 32 bytes. Added `align64()` function and > updated references to `align(64)`. > HI @asgibbons, Alignment of JIT sequence for loops work under influence of > -XX:OptoLoopAlignment flag. This is currently set to 16 may be we can change > this to 64 and study the effect on ICache as a separate patch as pointed by > @dean-long. I would be very careful changing loop code alignment. You will introduce a lot of NOP instruction into code to alight it which will be executed. I am fine with experimenting (running SPEC benchmarks) with different `OptoLoopAlignment` values - may be on modern CPUs 16 bytes may not be optimal. But there are a lot of code with small loops in Java which will regress if pre-loop code has a lot of NOPs. > > For stubs a new runtime constant StubCodeEntryAlignment can be added which > determines the start address alignment of stub BufferBlobs. This will limit > the impact of changes also will prevent any unwanted fragmentation issues. May be but what benefit you can get with different alignment for stubs code? > > Your current patch with new align64 macro looks good, it will also restrict > the scope of changes. - PR: https://git.openjdk.java.net/jdk/pull/5547
Re: RFR: 8273459: Update code segment alignment to 64 bytes [v2]
On Tue, 28 Sep 2021 01:15:30 GMT, Scott Gibbons wrote: >> Change the default code entry alignment to 64 bytes from 32 bytes. This >> allows for maintaining proper 64-byte alignment of data within a code >> segment, which is required by several AVX-512 instructions. >> >> I ran into this while implementing Base64 encoding and decoding. Code >> segments which were allocated with the address mod 32 == 0 but with the >> address mod 64 != 0 would cause the align() macro to misalign. This is >> because the align macro aligns to the size of the code segment and not the >> offset of the PC. So align(64) would align the PC to a multiple of 64 bytes >> from the start of the segment, and not to a pure 64-byte boundary as >> requested. Changing the alignment of the segment to 64 bytes fixes the >> issue. >> >> I have not seen any measurable difference in either performance or memory >> usage with the tests I have run. >> >> See [this >> ](https://mail.openjdk.java.net/pipermail/hotspot-dev/2021-August/054180.html) >> article for the discussion thread. > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Revert alignment of 64-bytes; Add align64() Current changes looks good. I have only 2 comments. src/hotspot/cpu/x86/macroAssembler_x86.cpp line 1173: > 1171: } > 1172: > 1173: // See 8273459. Function for ensuring 64-byte alignment, intended for > stubs only. I suggest to extend comment to explain why it will not work for nmethods - because they are copied when published. - Changes requested by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5547
Re: RFR: 8273459: Update code segment alignment to 64 bytes [v2]
On Thu, 16 Sep 2021 16:08:22 GMT, Erik Joelsson wrote: >> Scott Gibbons has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert alignment of 64-bytes; Add align64() > > .gitignore line 19: > >> 17: **/JTwork/** >> 18: /src/utils/LogCompilation/target/ >> 19: **.project** > > Changes to .gitignore should probably be made separately. Yes. This change should be removed from patch. - PR: https://git.openjdk.java.net/jdk/pull/5547
Re: RFR: 8273459: Update code segment alignment to 64 bytes
On Thu, 16 Sep 2021 13:52:24 GMT, Scott Gibbons wrote: > Change the default code entry alignment to 64 bytes from 32 bytes. This > allows for maintaining proper 64-byte alignment of data within a code > segment, which is required by several AVX-512 instructions. > > I ran into this while implementing Base64 encoding and decoding. Code > segments which were allocated with the address mod 32 == 0 but with the > address mod 64 != 0 would cause the align() macro to misalign. This is > because the align macro aligns to the size of the code segment and not the > offset of the PC. So align(64) would align the PC to a multiple of 64 bytes > from the start of the segment, and not to a pure 64-byte boundary as > requested. Changing the alignment of the segment to 64 bytes fixes the issue. > > I have not seen any measurable difference in either performance or memory > usage with the tests I have run. > > See [this > ](https://mail.openjdk.java.net/pipermail/hotspot-dev/2021-August/054180.html) > article for the discussion thread. I am back from vacation! I want to point out that when we generate code for these stubs we don't move them in CodeCache (in contrast to compiled methods): https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/stubRoutines.cpp#L268 `BufferBlob::create()` allocates specified space (of size `code_size2`, for example) directly in CodeCache in `NonNMethod` section: https://github.com/openjdk/jdk/blob/master/src/hotspot/share/code/codeBlob.cpp#L232 https://github.com/openjdk/jdk/blob/master/src/hotspot/share/code/codeBlob.cpp#L272 Based on that, using `pc()` and new `align64()` should be fine for these few cases. - PR: https://git.openjdk.java.net/jdk/pull/5547
Re: RFR: 8273459: Update code segment alignment to 64 bytes
On Thu, 16 Sep 2021 13:52:24 GMT, Scott Gibbons wrote: > Change the default code entry alignment to 64 bytes from 32 bytes. This > allows for maintaining proper 64-byte alignment of data within a code > segment, which is required by several AVX-512 instructions. > > I ran into this while implementing Base64 encoding and decoding. Code > segments which were allocated with the address mod 32 == 0 but with the > address mod 64 != 0 would cause the align() macro to misalign. This is > because the align macro aligns to the size of the code segment and not the > offset of the PC. So align(64) would align the PC to a multiple of 64 bytes > from the start of the segment, and not to a pure 64-byte boundary as > requested. Changing the alignment of the segment to 64 bytes fixes the issue. > > I have not seen any measurable difference in either performance or memory > usage with the tests I have run. > > See [this > ](https://mail.openjdk.java.net/pipermail/hotspot-dev/2021-August/054180.html) > article for the discussion thread. I share Dean's concern from the discussion before. `CodeEntryAlignment` is used in a lot of places and we have to be careful about changes to it. I found only 7 cases with `align(64)`: src/hotspot/cpu/x86//stubGenerator_x86_64.cpp:__ align(64); src/hotspot/cpu/x86//stubGenerator_x86_64.cpp:__ align(64); src/hotspot/cpu/x86//stubGenerator_x86_64.cpp:__ align(64); src/hotspot/cpu/x86//stubGenerator_x86_64.cpp:__ align(64); src/hotspot/cpu/x86//stubGenerator_x86_32.cpp:__ align(64); src/hotspot/cpu/x86//stubGenerator_x86_32.cpp:__ align(64); src/hotspot/cpu/x86//stubGenerator_x86_32.cpp:__ align(64); It does not justify such general changes. May suggestion would be to add new `align64()` method to call pc() as you suggested in original proposal. New function may also use `MaxVectorSize` as Jatin proposed if it helps. - Changes requested by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5547
Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v8]
On Thu, 24 Jun 2021 17:02:03 GMT, Scott Gibbons wrote: >> Add the Base64 Decode intrinsic for x86 to utilize AVX-512 for acceleration. >> Also allows for performance improvement for non-AVX-512 enabled platforms. >> Due to the nature of MIME-encoded inputs, modify the intrinsic signature to >> accept an additional parameter (isMIME) for fast-path MIME decoding. >> >> A change was made to the signature of DecodeBlock in Base64.java to provide >> the intrinsic information as to whether MIME decoding was being done. This >> allows for the intrinsic to bypass the expensive setup of zmm registers from >> AVX tables, knowing there may be invalid Base64 characters every 76 >> characters or so. A change was also made here removing the restriction that >> the intrinsic must return an even multiple of 3 bytes decoded. This >> implementation handles the pad characters at the end of the string and will >> return the actual number of characters decoded. >> >> The AVX portion of this code will decode in blocks of 256 bytes per loop >> iteration, then in chunks of 64 bytes, followed by end fixup decoding. The >> non-AVX code is an assembly-optimized version of the java DecodeBlock and >> behaves identically. >> >> Running the Base64Decode benchmark, this change increases decode performance >> by an average of 2.6x with a maximum 19.7x for buffers > ~20k. The numbers >> are given in the table below. >> >> **Base Score** is without intrinsic support, **Optimized Score** is using >> this intrinsic, and **Gain** is **Base** / **Optimized**. >> >> >> Benchmark Name | Base Score | Optimized Score | Gain >> -- | -- | -- | -- >> testBase64Decode size 1 | 15.36 | 15.32 | 1.00 >> testBase64Decode size 3 | 17.00 | 16.72 | 1.02 >> testBase64Decode size 7 | 20.60 | 18.82 | 1.09 >> testBase64Decode size 32 | 34.21 | 26.77 | 1.28 >> testBase64Decode size 64 | 54.43 | 38.35 | 1.42 >> testBase64Decode size 80 | 66.40 | 48.34 | 1.37 >> testBase64Decode size 96 | 73.16 | 52.90 | 1.38 >> testBase64Decode size 112 | 84.93 | 51.82 | 1.64 >> testBase64Decode size 512 | 288.81 | 32.04 | 9.01 >> testBase64Decode size 1000 | 560.48 | 40.79 | 13.74 >> testBase64Decode size 2 | 9530.28 | 483.37 | 19.72 >> testBase64Decode size 5 | 24552.24 | 1735.07 | 14.15 >> testBase64MIMEDecode size 1 | 22.87 | 21.36 | 1.07 >> testBase64MIMEDecode size 3 | 27.79 | 25.32 | 1.10 >> testBase64MIMEDecode size 7 | 44.74 | 43.81 | 1.02 >> testBase64MIMEDecode size 32 | 142.69 | 129.56 | 1.10 >> testBase64MIMEDecode size 64 | 256.90 | 243.80 | 1.05 >> testBase64MIMEDecode size 80 | 311.60 | 310.80 | 1.00 >> testBase64MIMEDecode size 96 | 364.00 | 346.66 | 1.05 >> testBase64MIMEDecode size 112 | 472.88 | 394.78 | 1.20 >> testBase64MIMEDecode size 512 | 1814.96 | 1671.28 | 1.09 >> testBase64MIMEDecode size 1000 | 3623.50 | 3227.61 | 1.12 >> testBase64MIMEDecode size 2 | 70484.09 | 64940.77 | 1.09 >> testBase64MIMEDecode size 5 | 191732.34 | 158158.95 | 1.21 >> testBase64WithErrorInputsDecode size 1 | 1531.02 | 1185.19 | 1.29 >> testBase64WithErrorInputsDecode size 3 | 1306.59 | 1170.99 | 1.12 >> testBase64WithErrorInputsDecode size 7 | 1238.11 | 1176.62 | 1.05 >> testBase64WithErrorInputsDecode size 32 | 1346.46 | 1138.47 | 1.18 >> testBase64WithErrorInputsDecode size 64 | 1195.28 | 1172.52 | 1.02 >> testBase64WithErrorInputsDecode size 80 | 1469.00 | 1180.94 | 1.24 >> testBase64WithErrorInputsDecode size 96 | 1434.48 | 1167.74 | 1.23 >> testBase64WithErrorInputsDecode size 112 | 1440.06 | 1162.56 | 1.24 >> testBase64WithErrorInputsDecode size 512 | 1362.79 | 1193.42 | 1.14 >> testBase64WithErrorInputsDecode size 1000 | 1426.07 | 1194.44 | 1.19 >> testBase64WithErrorInputsDecode size 2 | 1398.44 | 1138.17 | 1.23 >> testBase64WithErrorInputsDecode size 5 | 1409.41 | 1114.16 | 1.26 > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed Windows register stomping. Latest update fixed TestBase64.java test issue. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4368
Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v7]
On Wed, 23 Jun 2021 00:31:55 GMT, Scott Gibbons wrote: >> Add the Base64 Decode intrinsic for x86 to utilize AVX-512 for acceleration. >> Also allows for performance improvement for non-AVX-512 enabled platforms. >> Due to the nature of MIME-encoded inputs, modify the intrinsic signature to >> accept an additional parameter (isMIME) for fast-path MIME decoding. >> >> A change was made to the signature of DecodeBlock in Base64.java to provide >> the intrinsic information as to whether MIME decoding was being done. This >> allows for the intrinsic to bypass the expensive setup of zmm registers from >> AVX tables, knowing there may be invalid Base64 characters every 76 >> characters or so. A change was also made here removing the restriction that >> the intrinsic must return an even multiple of 3 bytes decoded. This >> implementation handles the pad characters at the end of the string and will >> return the actual number of characters decoded. >> >> The AVX portion of this code will decode in blocks of 256 bytes per loop >> iteration, then in chunks of 64 bytes, followed by end fixup decoding. The >> non-AVX code is an assembly-optimized version of the java DecodeBlock and >> behaves identically. >> >> Running the Base64Decode benchmark, this change increases decode performance >> by an average of 2.6x with a maximum 19.7x for buffers > ~20k. The numbers >> are given in the table below. >> >> **Base Score** is without intrinsic support, **Optimized Score** is using >> this intrinsic, and **Gain** is **Base** / **Optimized**. >> >> >> Benchmark Name | Base Score | Optimized Score | Gain >> -- | -- | -- | -- >> testBase64Decode size 1 | 15.36 | 15.32 | 1.00 >> testBase64Decode size 3 | 17.00 | 16.72 | 1.02 >> testBase64Decode size 7 | 20.60 | 18.82 | 1.09 >> testBase64Decode size 32 | 34.21 | 26.77 | 1.28 >> testBase64Decode size 64 | 54.43 | 38.35 | 1.42 >> testBase64Decode size 80 | 66.40 | 48.34 | 1.37 >> testBase64Decode size 96 | 73.16 | 52.90 | 1.38 >> testBase64Decode size 112 | 84.93 | 51.82 | 1.64 >> testBase64Decode size 512 | 288.81 | 32.04 | 9.01 >> testBase64Decode size 1000 | 560.48 | 40.79 | 13.74 >> testBase64Decode size 2 | 9530.28 | 483.37 | 19.72 >> testBase64Decode size 5 | 24552.24 | 1735.07 | 14.15 >> testBase64MIMEDecode size 1 | 22.87 | 21.36 | 1.07 >> testBase64MIMEDecode size 3 | 27.79 | 25.32 | 1.10 >> testBase64MIMEDecode size 7 | 44.74 | 43.81 | 1.02 >> testBase64MIMEDecode size 32 | 142.69 | 129.56 | 1.10 >> testBase64MIMEDecode size 64 | 256.90 | 243.80 | 1.05 >> testBase64MIMEDecode size 80 | 311.60 | 310.80 | 1.00 >> testBase64MIMEDecode size 96 | 364.00 | 346.66 | 1.05 >> testBase64MIMEDecode size 112 | 472.88 | 394.78 | 1.20 >> testBase64MIMEDecode size 512 | 1814.96 | 1671.28 | 1.09 >> testBase64MIMEDecode size 1000 | 3623.50 | 3227.61 | 1.12 >> testBase64MIMEDecode size 2 | 70484.09 | 64940.77 | 1.09 >> testBase64MIMEDecode size 5 | 191732.34 | 158158.95 | 1.21 >> testBase64WithErrorInputsDecode size 1 | 1531.02 | 1185.19 | 1.29 >> testBase64WithErrorInputsDecode size 3 | 1306.59 | 1170.99 | 1.12 >> testBase64WithErrorInputsDecode size 7 | 1238.11 | 1176.62 | 1.05 >> testBase64WithErrorInputsDecode size 32 | 1346.46 | 1138.47 | 1.18 >> testBase64WithErrorInputsDecode size 64 | 1195.28 | 1172.52 | 1.02 >> testBase64WithErrorInputsDecode size 80 | 1469.00 | 1180.94 | 1.24 >> testBase64WithErrorInputsDecode size 96 | 1434.48 | 1167.74 | 1.23 >> testBase64WithErrorInputsDecode size 112 | 1440.06 | 1162.56 | 1.24 >> testBase64WithErrorInputsDecode size 512 | 1362.79 | 1193.42 | 1.14 >> testBase64WithErrorInputsDecode size 1000 | 1426.07 | 1194.44 | 1.19 >> testBase64WithErrorInputsDecode size 2 | 1398.44 | 1138.17 | 1.23 >> testBase64WithErrorInputsDecode size 5 | 1409.41 | 1114.16 | 1.26 > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Fixing Windows build warnings The rest of testing hs-tier1-4 and xcomp is finished and clean. So this is the only failure. I attached hs_err file to RFE. - PR: https://git.openjdk.java.net/jdk/pull/4368
Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v7]
On Wed, 23 Jun 2021 00:31:55 GMT, Scott Gibbons wrote: >> Add the Base64 Decode intrinsic for x86 to utilize AVX-512 for acceleration. >> Also allows for performance improvement for non-AVX-512 enabled platforms. >> Due to the nature of MIME-encoded inputs, modify the intrinsic signature to >> accept an additional parameter (isMIME) for fast-path MIME decoding. >> >> A change was made to the signature of DecodeBlock in Base64.java to provide >> the intrinsic information as to whether MIME decoding was being done. This >> allows for the intrinsic to bypass the expensive setup of zmm registers from >> AVX tables, knowing there may be invalid Base64 characters every 76 >> characters or so. A change was also made here removing the restriction that >> the intrinsic must return an even multiple of 3 bytes decoded. This >> implementation handles the pad characters at the end of the string and will >> return the actual number of characters decoded. >> >> The AVX portion of this code will decode in blocks of 256 bytes per loop >> iteration, then in chunks of 64 bytes, followed by end fixup decoding. The >> non-AVX code is an assembly-optimized version of the java DecodeBlock and >> behaves identically. >> >> Running the Base64Decode benchmark, this change increases decode performance >> by an average of 2.6x with a maximum 19.7x for buffers > ~20k. The numbers >> are given in the table below. >> >> **Base Score** is without intrinsic support, **Optimized Score** is using >> this intrinsic, and **Gain** is **Base** / **Optimized**. >> >> >> Benchmark Name | Base Score | Optimized Score | Gain >> -- | -- | -- | -- >> testBase64Decode size 1 | 15.36 | 15.32 | 1.00 >> testBase64Decode size 3 | 17.00 | 16.72 | 1.02 >> testBase64Decode size 7 | 20.60 | 18.82 | 1.09 >> testBase64Decode size 32 | 34.21 | 26.77 | 1.28 >> testBase64Decode size 64 | 54.43 | 38.35 | 1.42 >> testBase64Decode size 80 | 66.40 | 48.34 | 1.37 >> testBase64Decode size 96 | 73.16 | 52.90 | 1.38 >> testBase64Decode size 112 | 84.93 | 51.82 | 1.64 >> testBase64Decode size 512 | 288.81 | 32.04 | 9.01 >> testBase64Decode size 1000 | 560.48 | 40.79 | 13.74 >> testBase64Decode size 2 | 9530.28 | 483.37 | 19.72 >> testBase64Decode size 5 | 24552.24 | 1735.07 | 14.15 >> testBase64MIMEDecode size 1 | 22.87 | 21.36 | 1.07 >> testBase64MIMEDecode size 3 | 27.79 | 25.32 | 1.10 >> testBase64MIMEDecode size 7 | 44.74 | 43.81 | 1.02 >> testBase64MIMEDecode size 32 | 142.69 | 129.56 | 1.10 >> testBase64MIMEDecode size 64 | 256.90 | 243.80 | 1.05 >> testBase64MIMEDecode size 80 | 311.60 | 310.80 | 1.00 >> testBase64MIMEDecode size 96 | 364.00 | 346.66 | 1.05 >> testBase64MIMEDecode size 112 | 472.88 | 394.78 | 1.20 >> testBase64MIMEDecode size 512 | 1814.96 | 1671.28 | 1.09 >> testBase64MIMEDecode size 1000 | 3623.50 | 3227.61 | 1.12 >> testBase64MIMEDecode size 2 | 70484.09 | 64940.77 | 1.09 >> testBase64MIMEDecode size 5 | 191732.34 | 158158.95 | 1.21 >> testBase64WithErrorInputsDecode size 1 | 1531.02 | 1185.19 | 1.29 >> testBase64WithErrorInputsDecode size 3 | 1306.59 | 1170.99 | 1.12 >> testBase64WithErrorInputsDecode size 7 | 1238.11 | 1176.62 | 1.05 >> testBase64WithErrorInputsDecode size 32 | 1346.46 | 1138.47 | 1.18 >> testBase64WithErrorInputsDecode size 64 | 1195.28 | 1172.52 | 1.02 >> testBase64WithErrorInputsDecode size 80 | 1469.00 | 1180.94 | 1.24 >> testBase64WithErrorInputsDecode size 96 | 1434.48 | 1167.74 | 1.23 >> testBase64WithErrorInputsDecode size 112 | 1440.06 | 1162.56 | 1.24 >> testBase64WithErrorInputsDecode size 512 | 1362.79 | 1193.42 | 1.14 >> testBase64WithErrorInputsDecode size 1000 | 1426.07 | 1194.44 | 1.19 >> testBase64WithErrorInputsDecode size 2 | 1398.44 | 1138.17 | 1.23 >> testBase64WithErrorInputsDecode size 5 | 1409.41 | 1114.16 | 1.26 > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Fixing Windows build warnings I hit strange failure in compiler/intrinsics/base64/TestBase64.java test on Windows machine which have Intel 8167M cpu (AVX512). # EXCEPTION_ACCESS_VIOLATION (0xc005) at pc=0x7ff92bcbd99e, pid=24628, tid=6804 # # Problematic frame: # V [jvm.dll+0xabd99e] ObjectMonitor::object_peek+0xe # Current thread (0x016c923de2c0): JavaThread "MainThread" [_thread_in_Java, id=6804, stack(0x0060df60,0x0060df70)] Stack: [0x0060df60,0x0060df70], sp=0x0060df6fcb50, free space=1010k Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code) V [jvm.dll+0xabd99e] ObjectMonitor::object_peek+0xe (objectMonitor.cpp:304) V [jvm.dll+0xc48d5b] ObjectSynchronizer::quick_enter+0x9b (synchronizer.cpp:331) V [jvm.dll+0xb9b6f6] SharedRuntime::monitor_enter_helper+0x36 (sharedRuntime.cpp:2112) V [jvm.dll+0x389894] Runtime1::monitorenter+0x94 (c1_Runtime1.cpp:748) C 0x016c99c4a757 Java frames: (J=compiled Java code,
Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v7]
On Wed, 23 Jun 2021 00:31:55 GMT, Scott Gibbons wrote: >> Add the Base64 Decode intrinsic for x86 to utilize AVX-512 for acceleration. >> Also allows for performance improvement for non-AVX-512 enabled platforms. >> Due to the nature of MIME-encoded inputs, modify the intrinsic signature to >> accept an additional parameter (isMIME) for fast-path MIME decoding. >> >> A change was made to the signature of DecodeBlock in Base64.java to provide >> the intrinsic information as to whether MIME decoding was being done. This >> allows for the intrinsic to bypass the expensive setup of zmm registers from >> AVX tables, knowing there may be invalid Base64 characters every 76 >> characters or so. A change was also made here removing the restriction that >> the intrinsic must return an even multiple of 3 bytes decoded. This >> implementation handles the pad characters at the end of the string and will >> return the actual number of characters decoded. >> >> The AVX portion of this code will decode in blocks of 256 bytes per loop >> iteration, then in chunks of 64 bytes, followed by end fixup decoding. The >> non-AVX code is an assembly-optimized version of the java DecodeBlock and >> behaves identically. >> >> Running the Base64Decode benchmark, this change increases decode performance >> by an average of 2.6x with a maximum 19.7x for buffers > ~20k. The numbers >> are given in the table below. >> >> **Base Score** is without intrinsic support, **Optimized Score** is using >> this intrinsic, and **Gain** is **Base** / **Optimized**. >> >> >> Benchmark Name | Base Score | Optimized Score | Gain >> -- | -- | -- | -- >> testBase64Decode size 1 | 15.36 | 15.32 | 1.00 >> testBase64Decode size 3 | 17.00 | 16.72 | 1.02 >> testBase64Decode size 7 | 20.60 | 18.82 | 1.09 >> testBase64Decode size 32 | 34.21 | 26.77 | 1.28 >> testBase64Decode size 64 | 54.43 | 38.35 | 1.42 >> testBase64Decode size 80 | 66.40 | 48.34 | 1.37 >> testBase64Decode size 96 | 73.16 | 52.90 | 1.38 >> testBase64Decode size 112 | 84.93 | 51.82 | 1.64 >> testBase64Decode size 512 | 288.81 | 32.04 | 9.01 >> testBase64Decode size 1000 | 560.48 | 40.79 | 13.74 >> testBase64Decode size 2 | 9530.28 | 483.37 | 19.72 >> testBase64Decode size 5 | 24552.24 | 1735.07 | 14.15 >> testBase64MIMEDecode size 1 | 22.87 | 21.36 | 1.07 >> testBase64MIMEDecode size 3 | 27.79 | 25.32 | 1.10 >> testBase64MIMEDecode size 7 | 44.74 | 43.81 | 1.02 >> testBase64MIMEDecode size 32 | 142.69 | 129.56 | 1.10 >> testBase64MIMEDecode size 64 | 256.90 | 243.80 | 1.05 >> testBase64MIMEDecode size 80 | 311.60 | 310.80 | 1.00 >> testBase64MIMEDecode size 96 | 364.00 | 346.66 | 1.05 >> testBase64MIMEDecode size 112 | 472.88 | 394.78 | 1.20 >> testBase64MIMEDecode size 512 | 1814.96 | 1671.28 | 1.09 >> testBase64MIMEDecode size 1000 | 3623.50 | 3227.61 | 1.12 >> testBase64MIMEDecode size 2 | 70484.09 | 64940.77 | 1.09 >> testBase64MIMEDecode size 5 | 191732.34 | 158158.95 | 1.21 >> testBase64WithErrorInputsDecode size 1 | 1531.02 | 1185.19 | 1.29 >> testBase64WithErrorInputsDecode size 3 | 1306.59 | 1170.99 | 1.12 >> testBase64WithErrorInputsDecode size 7 | 1238.11 | 1176.62 | 1.05 >> testBase64WithErrorInputsDecode size 32 | 1346.46 | 1138.47 | 1.18 >> testBase64WithErrorInputsDecode size 64 | 1195.28 | 1172.52 | 1.02 >> testBase64WithErrorInputsDecode size 80 | 1469.00 | 1180.94 | 1.24 >> testBase64WithErrorInputsDecode size 96 | 1434.48 | 1167.74 | 1.23 >> testBase64WithErrorInputsDecode size 112 | 1440.06 | 1162.56 | 1.24 >> testBase64WithErrorInputsDecode size 512 | 1362.79 | 1193.42 | 1.14 >> testBase64WithErrorInputsDecode size 1000 | 1426.07 | 1194.44 | 1.19 >> testBase64WithErrorInputsDecode size 2 | 1398.44 | 1138.17 | 1.23 >> testBase64WithErrorInputsDecode size 5 | 1409.41 | 1114.16 | 1.26 > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Fixing Windows build warnings I will run our internal testing before approving this. - PR: https://git.openjdk.java.net/jdk/pull/4368
Integrated: 8268272: Remove JDK-8264874 changes because Graal was removed.
On Fri, 4 Jun 2021 17:40:48 GMT, Vladimir Kozlov wrote: > JDK-8264806 removed Graal sources from JDK and INCLUDE_GRAAL is not defined > anymore. We can now remove code added by JDK-8264874: > https://github.com/openjdk/jdk/commit/951f277a > > Tested Tier1. This pull request has now been integrated. Changeset: 48dc72b7 Author:Vladimir Kozlov URL: https://git.openjdk.java.net/jdk/commit/48dc72b74d6b4b7b8fb605b62fc0057b5f4652e1 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod 8268272: Remove JDK-8264874 changes because Graal was removed. Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/4367
RFR: 8268272: Remove JDK-8264874 changes because Graal was removed.
JDK-8264806 removed Graal sources from JDK and INCLUDE_GRAAL is not defined anymore. We can now remove code added by JDK-8264874: https://github.com/openjdk/jdk/commit/951f277a Tested Tier1. - Commit messages: - 8268272: Remove JDK-8264874 changes because Graal was removed. Changes: https://git.openjdk.java.net/jdk/pull/4367/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4367=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268272 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4367.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4367/head:pull/4367 PR: https://git.openjdk.java.net/jdk/pull/4367
Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v8]
On Wed, 19 May 2021 00:58:15 GMT, Sandhya Viswanathan wrote: >> This PR contains Short Vector Math Library support related changes for >> [JEP-414 Vector API (Second Incubator)](https://openjdk.java.net/jeps/414), >> in preparation for when targeted. >> >> Intel Short Vector Math Library (SVML) based intrinsics in native x86 >> assembly provide optimized implementation for Vector API transcendental and >> trigonometric methods. >> These methods are built into a separate library instead of being part of >> libjvm.so or jvm.dll. >> >> The following changes are made: >>The source for these methods is placed in the jdk.incubator.vector module >> under src/jdk.incubator.vector/linux/native/libsvml and >> src/jdk.incubator.vector/windows/native/libsvml. >>The assembly source files are named as “*.S” and include files are named >> as “*.S.inc”. >>The corresponding build script is placed at >> make/modules/jdk.incubator.vector/Lib.gmk. >>Changes are made to build system to support dependency tracking for >> assembly files with includes. >>The built native libraries (libsvml.so/svml.dll) are placed in bin >> directory of JDK on Windows and lib directory of JDK on Linux. >>The C2 JIT uses the dll_load and dll_lookup to get the addresses of >> optimized methods from this library. >> >> Build system changes and module library build scripts are contributed by >> Magnus (magnus.ihse.bur...@oracle.com). >> >> Looking forward to your review and feedback. >> >> Performance: >> Micro benchmark Base Optimized Unit Gain(Optimized/Base) >> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90 >> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05 >> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94 >> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79 >> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55 >> Double128Vector.COS 49.94 245.89 ops/ms 4.92 >> Double128Vector.COSH 26.91 126.00 ops/ms 4.68 >> Double128Vector.EXP 71.64 379.65 ops/ms 5.30 >> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18 >> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44 >> Double128Vector.LOG 61.95 279.84 ops/ms 4.52 >> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03 >> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79 >> Double128Vector.SIN 49.36 240.79 ops/ms 4.88 >> Double128Vector.SINH 26.59 103.75 ops/ms 3.90 >> Double128Vector.TAN 41.05 152.39 ops/ms 3.71 >> Double128Vector.TANH 45.29 169.53 ops/ms 3.74 >> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96 >> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01 >> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78 >> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44 >> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04 >> Double256Vector.COS 58.26 389.77 ops/ms 6.69 >> Double256Vector.COSH 29.44 151.11 ops/ms 5.13 >> Double256Vector.EXP 86.67 564.68 ops/ms 6.52 >> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80 >> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62 >> Double256Vector.LOG 71.52 394.90 ops/ms 5.52 >> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54 >> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05 >> Double256Vector.SIN 57.06 380.98 ops/ms 6.68 >> Double256Vector.SINH 29.40 117.37 ops/ms 3.99 >> Double256Vector.TAN 44.90 279.90 ops/ms 6.23 >> Double256Vector.TANH 54.08 274.71 ops/ms 5.08 >> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35 >> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57 >> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04 >> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32 >> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69 >> Double512Vector.COS 59.88 837.04 ops/ms 13.98 >> Double512Vector.COSH 30.34 172.76 ops/ms 5.70 >> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14 >> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34 >> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34 >> Double512Vector.LOG 74.84 996.00 ops/ms 13.31 >> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72 >> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34 >> Double512Vector.POW 37.42 384.13 ops/ms 10.26 >> Double512Vector.SIN 59.74 728.45 ops/ms 12.19 >> Double512Vector.SINH 29.47 143.38 ops/ms 4.87 >> Double512Vector.TAN 46.20 587.21 ops/ms 12.71 >> Double512Vector.TANH 57.36 495.42 ops/ms 8.64 >> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06 >> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16 >> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44 >> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28 >> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53 >> Double64Vector.COS 23.42 152.01 ops/ms 6.49 >> Double64Vector.COSH 17.34 113.34 ops/ms 6.54 >> Double64Vector.EXP 27.08 203.53 ops/ms 7.52 >> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15 >> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59 >> Double64Vector.LOG 26.75 142.63 ops/ms 5.33 >> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40 >> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38 >> Double64Vector.SIN 23.28 146.91 ops/ms 6.31 >> Double64Vector.SINH 17.62 88.59 ops/ms 5.03 >> Double64Vector.TAN 21.00 86.43 ops/ms 4.12 >> Double64Vector.TANH 23.75 111.35 ops/ms 4.69 >> Float128Vector.ACOS 57.52 110.65
Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v7]
On Tue, 18 May 2021 23:59:28 GMT, Sandhya Viswanathan wrote: >> This PR contains Short Vector Math Library support related changes for >> [JEP-414 Vector API (Second Incubator)](https://openjdk.java.net/jeps/414), >> in preparation for when targeted. >> >> Intel Short Vector Math Library (SVML) based intrinsics in native x86 >> assembly provide optimized implementation for Vector API transcendental and >> trigonometric methods. >> These methods are built into a separate library instead of being part of >> libjvm.so or jvm.dll. >> >> The following changes are made: >>The source for these methods is placed in the jdk.incubator.vector module >> under src/jdk.incubator.vector/linux/native/libsvml and >> src/jdk.incubator.vector/windows/native/libsvml. >>The assembly source files are named as “*.S” and include files are named >> as “*.S.inc”. >>The corresponding build script is placed at >> make/modules/jdk.incubator.vector/Lib.gmk. >>Changes are made to build system to support dependency tracking for >> assembly files with includes. >>The built native libraries (libsvml.so/svml.dll) are placed in bin >> directory of JDK on Windows and lib directory of JDK on Linux. >>The C2 JIT uses the dll_load and dll_lookup to get the addresses of >> optimized methods from this library. >> >> Build system changes and module library build scripts are contributed by >> Magnus (magnus.ihse.bur...@oracle.com). >> >> Looking forward to your review and feedback. >> >> Performance: >> Micro benchmark Base Optimized Unit Gain(Optimized/Base) >> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90 >> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05 >> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94 >> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79 >> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55 >> Double128Vector.COS 49.94 245.89 ops/ms 4.92 >> Double128Vector.COSH 26.91 126.00 ops/ms 4.68 >> Double128Vector.EXP 71.64 379.65 ops/ms 5.30 >> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18 >> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44 >> Double128Vector.LOG 61.95 279.84 ops/ms 4.52 >> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03 >> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79 >> Double128Vector.SIN 49.36 240.79 ops/ms 4.88 >> Double128Vector.SINH 26.59 103.75 ops/ms 3.90 >> Double128Vector.TAN 41.05 152.39 ops/ms 3.71 >> Double128Vector.TANH 45.29 169.53 ops/ms 3.74 >> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96 >> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01 >> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78 >> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44 >> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04 >> Double256Vector.COS 58.26 389.77 ops/ms 6.69 >> Double256Vector.COSH 29.44 151.11 ops/ms 5.13 >> Double256Vector.EXP 86.67 564.68 ops/ms 6.52 >> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80 >> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62 >> Double256Vector.LOG 71.52 394.90 ops/ms 5.52 >> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54 >> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05 >> Double256Vector.SIN 57.06 380.98 ops/ms 6.68 >> Double256Vector.SINH 29.40 117.37 ops/ms 3.99 >> Double256Vector.TAN 44.90 279.90 ops/ms 6.23 >> Double256Vector.TANH 54.08 274.71 ops/ms 5.08 >> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35 >> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57 >> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04 >> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32 >> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69 >> Double512Vector.COS 59.88 837.04 ops/ms 13.98 >> Double512Vector.COSH 30.34 172.76 ops/ms 5.70 >> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14 >> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34 >> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34 >> Double512Vector.LOG 74.84 996.00 ops/ms 13.31 >> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72 >> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34 >> Double512Vector.POW 37.42 384.13 ops/ms 10.26 >> Double512Vector.SIN 59.74 728.45 ops/ms 12.19 >> Double512Vector.SINH 29.47 143.38 ops/ms 4.87 >> Double512Vector.TAN 46.20 587.21 ops/ms 12.71 >> Double512Vector.TANH 57.36 495.42 ops/ms 8.64 >> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06 >> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16 >> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44 >> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28 >> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53 >> Double64Vector.COS 23.42 152.01 ops/ms 6.49 >> Double64Vector.COSH 17.34 113.34 ops/ms 6.54 >> Double64Vector.EXP 27.08 203.53 ops/ms 7.52 >> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15 >> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59 >> Double64Vector.LOG 26.75 142.63 ops/ms 5.33 >> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40 >> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38 >> Double64Vector.SIN 23.28 146.91 ops/ms 6.31 >> Double64Vector.SINH 17.62 88.59 ops/ms 5.03 >> Double64Vector.TAN 21.00 86.43 ops/ms 4.12 >> Double64Vector.TANH 23.75 111.35 ops/ms 4.69 >> Float128Vector.ACOS 57.52 110.65
Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v4]
On Sat, 15 May 2021 02:06:29 GMT, Sandhya Viswanathan wrote: >> This PR contains Short Vector Math Library support related changes for >> [JEP-414 Vector API (Second Incubator)](https://openjdk.java.net/jeps/414), >> in preparation for when targeted. >> >> Intel Short Vector Math Library (SVML) based intrinsics in native x86 >> assembly provide optimized implementation for Vector API transcendental and >> trigonometric methods. >> These methods are built into a separate library instead of being part of >> libjvm.so or jvm.dll. >> >> The following changes are made: >>The source for these methods is placed in the jdk.incubator.vector module >> under src/jdk.incubator.vector/linux/native/libsvml and >> src/jdk.incubator.vector/windows/native/libsvml. >>The assembly source files are named as “*.S” and include files are named >> as “*.S.inc”. >>The corresponding build script is placed at >> make/modules/jdk.incubator.vector/Lib.gmk. >>Changes are made to build system to support dependency tracking for >> assembly files with includes. >>The built native libraries (libsvml.so/svml.dll) are placed in bin >> directory of JDK on Windows and lib directory of JDK on Linux. >>The C2 JIT uses the dll_load and dll_lookup to get the addresses of >> optimized methods from this library. >> >> Build system changes and module library build scripts are contributed by >> Magnus (magnus.ihse.bur...@oracle.com). >> >> Looking forward to your review and feedback. >> >> Performance: >> Micro benchmark Base Optimized Unit Gain(Optimized/Base) >> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90 >> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05 >> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94 >> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79 >> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55 >> Double128Vector.COS 49.94 245.89 ops/ms 4.92 >> Double128Vector.COSH 26.91 126.00 ops/ms 4.68 >> Double128Vector.EXP 71.64 379.65 ops/ms 5.30 >> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18 >> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44 >> Double128Vector.LOG 61.95 279.84 ops/ms 4.52 >> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03 >> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79 >> Double128Vector.SIN 49.36 240.79 ops/ms 4.88 >> Double128Vector.SINH 26.59 103.75 ops/ms 3.90 >> Double128Vector.TAN 41.05 152.39 ops/ms 3.71 >> Double128Vector.TANH 45.29 169.53 ops/ms 3.74 >> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96 >> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01 >> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78 >> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44 >> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04 >> Double256Vector.COS 58.26 389.77 ops/ms 6.69 >> Double256Vector.COSH 29.44 151.11 ops/ms 5.13 >> Double256Vector.EXP 86.67 564.68 ops/ms 6.52 >> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80 >> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62 >> Double256Vector.LOG 71.52 394.90 ops/ms 5.52 >> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54 >> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05 >> Double256Vector.SIN 57.06 380.98 ops/ms 6.68 >> Double256Vector.SINH 29.40 117.37 ops/ms 3.99 >> Double256Vector.TAN 44.90 279.90 ops/ms 6.23 >> Double256Vector.TANH 54.08 274.71 ops/ms 5.08 >> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35 >> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57 >> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04 >> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32 >> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69 >> Double512Vector.COS 59.88 837.04 ops/ms 13.98 >> Double512Vector.COSH 30.34 172.76 ops/ms 5.70 >> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14 >> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34 >> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34 >> Double512Vector.LOG 74.84 996.00 ops/ms 13.31 >> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72 >> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34 >> Double512Vector.POW 37.42 384.13 ops/ms 10.26 >> Double512Vector.SIN 59.74 728.45 ops/ms 12.19 >> Double512Vector.SINH 29.47 143.38 ops/ms 4.87 >> Double512Vector.TAN 46.20 587.21 ops/ms 12.71 >> Double512Vector.TANH 57.36 495.42 ops/ms 8.64 >> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06 >> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16 >> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44 >> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28 >> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53 >> Double64Vector.COS 23.42 152.01 ops/ms 6.49 >> Double64Vector.COSH 17.34 113.34 ops/ms 6.54 >> Double64Vector.EXP 27.08 203.53 ops/ms 7.52 >> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15 >> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59 >> Double64Vector.LOG 26.75 142.63 ops/ms 5.33 >> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40 >> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38 >> Double64Vector.SIN 23.28 146.91 ops/ms 6.31 >> Double64Vector.SINH 17.62 88.59 ops/ms 5.03 >> Double64Vector.TAN 21.00 86.43 ops/ms 4.12 >> Double64Vector.TANH 23.75 111.35 ops/ms 4.69 >> Float128Vector.ACOS 57.52 110.65
Integrated: 8267112: JVMCI compiler modules should be kept upgradable
On Thu, 13 May 2021 16:37:38 GMT, Vladimir Kozlov wrote: > [JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes > removed sources and also removed JVMCI compiler from list of upgradable > modules. JVMCI compiler modules should be upgradable in JDK to work with > GraalVM. > > Make these modules upgradable again and empty by leaving only reference to > JVMCI (jdk.internal.vm.ci) module. It does not restore sources - only > `module-info.java` files are kept. > > Note, we continue discussion about > [JDK-8265091](https://bugs.openjdk.java.net/browse/JDK-8265091): "Use Module > API to export JVMCI packages at runtime" to see if we can remove these > `module-info.java` files. > > Changes were proposed by @dougxc after testing > [JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes with > GraalVM. > I restored related code in some tests for them to pass. > > Testing: full tier1-tier3. This pull request has now been integrated. Changeset: 2effdd1b Author:Vladimir Kozlov URL: https://git.openjdk.java.net/jdk/commit/2effdd1b6799a15a766b2b2a6cba4806d92122f3 Stats: 83 lines in 9 files changed: 34 ins; 42 del; 7 mod 8267112: JVMCI compiler modules should be kept upgradable Reviewed-by: mchung, erikj, dnsimon - PR: https://git.openjdk.java.net/jdk/pull/4014
Re: RFR: 8267112: JVMCI compiler modules should be kept upgradable
On Thu, 13 May 2021 16:37:38 GMT, Vladimir Kozlov wrote: > [JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes > removed sources and also removed JVMCI compiler from list of upgradable > modules. JVMCI compiler modules should be upgradable in JDK to work with > GraalVM. > > Make these modules upgradable again and empty by leaving only reference to > JVMCI (jdk.internal.vm.ci) module. It does not restore sources - only > `module-info.java` files are kept. > > Note, we continue discussion about > [JDK-8265091](https://bugs.openjdk.java.net/browse/JDK-8265091): "Use Module > API to export JVMCI packages at runtime" to see if we can remove these > `module-info.java` files. > > Changes were proposed by @dougxc after testing > [JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes with > GraalVM. > I restored related code in some tests for them to pass. > > Testing: full tier1-tier3. Thank you, Erik. - PR: https://git.openjdk.java.net/jdk/pull/4014
Re: RFR: 8267112: JVMCI compiler modules should be kept upgradable
On Thu, 13 May 2021 16:37:38 GMT, Vladimir Kozlov wrote: > [JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes > removed sources and also removed JVMCI compiler from list of upgradable > modules. JVMCI compiler modules should be upgradable in JDK to work with > GraalVM. > > Make these modules upgradable again and empty by leaving only reference to > JVMCI (jdk.internal.vm.ci) module. It does not restore sources - only > `module-info.java` files are kept. > > Note, we continue discussion about > [JDK-8265091](https://bugs.openjdk.java.net/browse/JDK-8265091): "Use Module > API to export JVMCI packages at runtime" to see if we can remove these > `module-info.java` files. > > Changes were proposed by @dougxc after testing > [JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes with > GraalVM. > I restored related code in some tests for them to pass. > > Testing: full tier1-tier3. @erikj79, are you okay with these changes? - PR: https://git.openjdk.java.net/jdk/pull/4014
Re: RFR: 8267112: JVMCI compiler modules should be kept upgradable
On Thu, 13 May 2021 16:37:38 GMT, Vladimir Kozlov wrote: > [JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes > removed sources and also removed JVMCI compiler from list of upgradable > modules. JVMCI compiler modules should be upgradable in JDK to work with > GraalVM. > > Make these modules upgradable again and empty by leaving only reference to > JVMCI (jdk.internal.vm.ci) module. It does not restore sources - only > `module-info.java` files are kept. > > Note, we continue discussion about > [JDK-8265091](https://bugs.openjdk.java.net/browse/JDK-8265091): "Use Module > API to export JVMCI packages at runtime" to see if we can remove these > `module-info.java` files. > > Changes were proposed by @dougxc after testing > [JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes with > GraalVM. > I restored related code in some tests for them to pass. > > Testing: full tier1-tier3. Thank you, Mandy. - PR: https://git.openjdk.java.net/jdk/pull/4014
RFR: 8267112: JVMCI compiler modules should be kept upgradable
[JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes removed sources and also removed JVMCI compiler from list of upgradable modules. JVMCI compiler modules should be upgradable in JDK to work with GraalVM. Make these modules upgradable again and empty by leaving only reference to JVMCI (jdk.internal.vm.ci) module. It does not restore sources - only `module-info.java` files are kept. Note, we continue discussion about [JDK-8265091](https://bugs.openjdk.java.net/browse/JDK-8265091): "Use Module API to export JVMCI packages at runtime" to see if we can remove these `module-info.java` files. Changes were proposed by @dougxc after testing [JDK-8264806](https://bugs.openjdk.java.net/browse/JDK-8264806) changes with GraalVM. I restored related code in some tests for them to pass. Testing: full tier1-tier3. - Commit messages: - Fix tests - 8267112: Graal modules should be kept upgradable Changes: https://git.openjdk.java.net/jdk/pull/4014/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4014=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267112 Stats: 83 lines in 9 files changed: 34 ins; 42 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/4014.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4014/head:pull/4014 PR: https://git.openjdk.java.net/jdk/pull/4014
Integrated: 8264806: Remove the experimental JIT compiler
On Fri, 9 Apr 2021 17:35:11 GMT, Vladimir Kozlov wrote: > As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 This pull request has now been integrated. Changeset: 4785e112 Author:Vladimir Kozlov URL: https://git.openjdk.java.net/jdk/commit/4785e112 Stats: 441532 lines in 2884 files changed: 0 ins; 441518 del; 14 mod 8264806: Remove the experimental JIT compiler Reviewed-by: iignatyev, erikj - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v5]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - Merge branch 'master' into JDK-8264806 - Merge branch 'JDK-8264805' into JDK-8264806 - Merge branch 'master' into JDK-8264805 - Restore Compiler::isGraalEnabled() - Restore Graal Builder image makefile - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 - Address review comments - 8264806: Remove the experimental JIT compiler - Remove exports from Graal module to jdk.aot - Merge branch 'master' into JDK-8264805 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/694acedf...b1e9ba63 - Changes: https://git.openjdk.java.net/jdk/pull/3421/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3421=04 Stats: 441532 lines in 2884 files changed: 0 ins; 441518 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v4]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits: - Merge branch 'JDK-8264805' into JDK-8264806 - Merge branch 'master' into JDK-8264805 - Restore Compiler::isGraalEnabled() - Restore Graal Builder image makefile - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 - Address review comments - 8264806: Remove the experimental JIT compiler - Remove exports from Graal module to jdk.aot - Merge branch 'master' into JDK-8264805 - Merge branch 'master' into JDK-8264805 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/b524a81a...db7c9aaf - Changes: https://git.openjdk.java.net/jdk/pull/3421/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3421=03 Stats: 468493 lines in 3247 files changed: 2 ins; 468290 del; 201 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Integrated: 8264805: Remove the experimental Ahead-of-Time Compiler
On Thu, 8 Apr 2021 15:23:52 GMT, Vladimir Kozlov wrote: > As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Ahead-of-Time Compiler from JDK: > > - `jdk.aot` module — the `jaotc` tool > - `src/hotspot/share/aot` — loads AoT compiled code into VM for execution > - related HotSpot code guarded by `#if INCLUDE_AOT` > > Additionally, remove tests as well as code in makefiles related to AoT > compilation. > > Tested hs-tier1-4 This pull request has now been integrated. Changeset: 694acedf Author:Vladimir Kozlov URL: https://git.openjdk.java.net/jdk/commit/694acedf Stats: 26972 lines in 378 files changed: 2 ins; 26772 del; 198 mod 8264805: Remove the experimental Ahead-of-Time Compiler Reviewed-by: coleenp, erikj, stefank, iignatyev, dholmes, aph, shade, iklam, mchung, iveresov - PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v6]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Ahead-of-Time Compiler from JDK: > > - `jdk.aot` module — the `jaotc` tool > - `src/hotspot/share/aot` — loads AoT compiled code into VM for execution > - related HotSpot code guarded by `#if INCLUDE_AOT` > > Additionally, remove tests as well as code in makefiles related to AoT > compilation. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits: - Merge branch 'master' into JDK-8264805 - Address review comments - Remove exports from Graal module to jdk.aot - Merge branch 'master' into JDK-8264805 - Merge branch 'master' into JDK-8264805 - 8264805: Remove the experimental Ahead-of-Time Compiler - Changes: https://git.openjdk.java.net/jdk/pull/3398/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3398=05 Stats: 26972 lines in 378 files changed: 2 ins; 26772 del; 198 mod Patch: https://git.openjdk.java.net/jdk/pull/3398.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3398/head:pull/3398 PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8265403: consolidate definition of CPU features [v3]
On Mon, 19 Apr 2021 19:38:52 GMT, Doug Simon wrote:
>> src/hotspot/cpu/x86/vmStructs_x86.hpp line 45:
>>
>>> 43:
>>> 44: #define DECLARE_LONG_CPU_FEATURE_CONSTANT(id, name, bit)
>>> GENERATE_VM_LONG_CONSTANT_ENTRY(VM_Version::CPU_##id)
>>> 45: #define VM_LONG_CPU_FEATURE_CONSTANTS
>>> CPU_FEATURE_FLAGS(DECLARE_LONG_CPU_FEATURE_CONSTANT)
>>
>> Do we need to keep `VM_LONG_CONSTANTS_CPU` after you removed its body here
>> and in vmStructs_jvmci.cpp?
>>
>> What about `VM_INT_CONSTANTS_CPU` here? vmStructs_jvmci.cpp duplicates it.
>
> `vmStructs.cpp` and `vmStructs_jvmci.cpp` are disjoint. This file (i.e.
> `vmStructs_x86.hpp`) is only used by `vmStructs.cpp`.
> `vmStructs.cpp` expects all macros such as `VM_LONG_CONSTANTS_CPU` to be
> defined.
> `vmStructs_jvmci.cpp` will provide a dummy definition for missing macros.
Got it. Even so they are empty everywhere :(
>> src/hotspot/share/jvmci/vmStructs_jvmci.cpp line 753:
>>
>>> 751:
>>> 752: #define DECLARE_INT_CPU_FEATURE_CONSTANT(id, name, bit)
>>> GENERATE_VM_INT_CONSTANT_ENTRY(VM_Version::CPU_##id)
>>> 753: #define VM_INT_CPU_FEATURE_CONSTANTS
>>> CPU_FEATURE_FLAGS(DECLARE_INT_CPU_FEATURE_CONSTANT)
>>
>> Missing `#undef DECLARE_INT_CPU_FEATURE_CONSTANT`.
>
> No, it must stay defined up to the point `VM_INT_CPU_FEATURE_CONSTANTS` is
> used. Since this is a `.cpp` file, it's ok to leave it defined.
I see.
>> src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotJVMCIBackendFactory.java
>> line 66:
>>
>>> 64: long bitMask = e.getValue();
>>> 65: String key = e.getKey();
>>> 66: if (key.startsWith("VM_Version::CPU_")) {
>>
>> As I understand this code, it goes over `constants` values passed from VM
>> and Trying to map them to `enumType`. It catches cases when a value is
>> missing in `enumType`. What about case when `enumType` has `extra` value
>> which is not defined in `constants`?
>
> We could warn about that but cannot remove it without breaking backwards
> capability for JVMCI wrt Graal. Such a deleted capability will simply be seen
> as "not supported" by Graal.
Okay.
-
PR: https://git.openjdk.java.net/jdk/pull/3558
Re: RFR: 8265403: consolidate definition of CPU features [v4]
On Mon, 19 Apr 2021 19:56:45 GMT, Doug Simon wrote: >> While porting >> [JDK-8224974](https://bugs.openjdk.java.net/browse/JDK-8224974) to Graal, I >> noticed that new CPU features were defined for x86 and AArch64 without being >> exposed via JVMCI. To avoid this problem in future, this PR updates x86 and >> AArch64 to define CPU features with a single macro that is used to generate >> enum declarations as well as vmstructs entries. >> >> In addition, the JVMCI API is updated to exposes the new CPU feature >> constants and now has a check that ensure these constants are in sync with >> the underlying macro definition. > > Doug Simon has updated the pull request incrementally with two additional > commits since the last revision: > > - updated date in copyright > - added blank lines after macros You need review from Runtime group too. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3558
Re: RFR: 8265403: consolidate definition of CPU features [v3]
On Mon, 19 Apr 2021 09:46:17 GMT, Doug Simon wrote:
>> While porting
>> [JDK-8224974](https://bugs.openjdk.java.net/browse/JDK-8224974) to Graal, I
>> noticed that new CPU features were defined for x86 and AArch64 without being
>> exposed via JVMCI. To avoid this problem in future, this PR updates x86 and
>> AArch64 to define CPU features with a single macro that is used to generate
>> enum declarations as well as vmstructs entries.
>>
>> In addition, the JVMCI API is updated to exposes the new CPU feature
>> constants and now has a check that ensure these constants are in sync with
>> the underlying macro definition.
>
> Doug Simon has refreshed the contents of this pull request, and previous
> commits have been removed. The incremental views will show differences
> compared to the previous content of the PR. The pull request contains one new
> commit since the last revision:
>
> avoid use of a lambda in JVMCI initialization
Please, update copyright years in files you touched.
src/hotspot/cpu/aarch64/vm_version_aarch64.hpp line 118:
> 116: decl(STXR_PREFETCH, "stxr_prefetch", 29) \
> 117: decl(A53MAC,"a53mac",30)
> 118: #define DECLARE_CPU_FEATURE_FLAG(id, name, bit) CPU_##id = (1 << bit),
Add empty line before to separate `CPU_FEATURE_FLAGS` macro.
src/hotspot/cpu/x86/vmStructs_x86.hpp line 45:
> 43:
> 44: #define DECLARE_LONG_CPU_FEATURE_CONSTANT(id, name, bit)
> GENERATE_VM_LONG_CONSTANT_ENTRY(VM_Version::CPU_##id)
> 45: #define VM_LONG_CPU_FEATURE_CONSTANTS
> CPU_FEATURE_FLAGS(DECLARE_LONG_CPU_FEATURE_CONSTANT)
Do we need to keep `VM_LONG_CONSTANTS_CPU` after you removed its body here and
in vmStructs_jvmci.cpp?
What about `VM_INT_CONSTANTS_CPU` here? vmStructs_jvmci.cpp duplicates it.
src/hotspot/cpu/x86/vm_version_x86.hpp line 363:
> 361: decl(AVX512_VBMI, "avx512_vbmi", 45) /* Vector BMI
> instructions */ \
> 362: decl(HV,"hv",46) /* Hypervisor
> instructions */
> 363: #define DECLARE_CPU_FEATURE_FLAG(id, name, bit) CPU_##id = (1ULL << bit),
Add empty line before it to separate `CPU_FEATURE_FLAGS` macro more clear.
src/hotspot/share/jvmci/vmStructs_jvmci.cpp line 753:
> 751:
> 752: #define DECLARE_INT_CPU_FEATURE_CONSTANT(id, name, bit)
> GENERATE_VM_INT_CONSTANT_ENTRY(VM_Version::CPU_##id)
> 753: #define VM_INT_CPU_FEATURE_CONSTANTS
> CPU_FEATURE_FLAGS(DECLARE_INT_CPU_FEATURE_CONSTANT)
Missing `#undef DECLARE_INT_CPU_FEATURE_CONSTANT`.
src/hotspot/share/jvmci/vmStructs_jvmci.cpp line 769:
> 767:
> 768: #define DECLARE_LONG_CPU_FEATURE_CONSTANT(id, name, bit)
> GENERATE_VM_LONG_CONSTANT_ENTRY(VM_Version::CPU_##id)
> 769: #define VM_LONG_CPU_FEATURE_CONSTANTS
> CPU_FEATURE_FLAGS(DECLARE_LONG_CPU_FEATURE_CONSTANT)
Missing `#undef DECLARE_LONG_CPU_FEATURE_CONSTANT`.
src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotJVMCIBackendFactory.java
line 66:
> 64: long bitMask = e.getValue();
> 65: String key = e.getKey();
> 66: if (key.startsWith("VM_Version::CPU_")) {
As I understand this code, it goes over `constants` values passed from VM and
Trying to map them to `enumType`. It catches cases when a value is missing in
`enumType`. What about case when `enumType` has `extra` value which is not
defined in `constants`?
-
PR: https://git.openjdk.java.net/jdk/pull/3558
Re: RFR: 8252600: remove mx configuration [v2]
On Sun, 18 Apr 2021 20:17:14 GMT, Doug Simon wrote: >> This PR removes the mx configuration files in the JDK as they do not really >> belong here. Instead, I've updated and moved them to >> https://github.com/dougxc/mx_jdk. > > Doug Simon has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > 8252600: [JVMCI] remove mx configuration The PR is missing `[JVMCI] ` in title to match RFE title. Otherwise it is good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3559
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Tue, 13 Apr 2021 09:30:23 GMT, Doug Simon wrote: >> We would definitely like to be able to continue testing of GraalVM with the >> JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like >> it does today is important. > >> @dougxc I restored Compiler::isGraalEnabled(). > > Thanks. I guess this should really be named `isJVMCICompilerEnabled` now and > the `vm.graal.enabled` predicate renamed to `vm.jvmcicompiler.enabled` but > maybe that's too big a change (or can be done later). @dougxc Renaming should be done separately. May be use RFE I filed: https://bugs.openjdk.java.net/browse/JDK-8265032 Do you approve these Graal removal changes? - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sun, 11 Apr 2021 10:25:47 GMT, Doug Simon wrote: >> Vladimir Kozlov 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 three additional >> commits since the last revision: >> >> - Restore Graal Builder image makefile >> - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 >> - 8264806: Remove the experimental JIT compiler > > We would definitely like to be able to continue testing of GraalVM with the > JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like > it does today is important. @dougxc I restored Compiler::isGraalEnabled(). - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v3]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision: Restore Compiler::isGraalEnabled() - Changes: - all: https://git.openjdk.java.net/jdk/pull/3421/files - new: https://git.openjdk.java.net/jdk/pull/3421/files/a246aaa6..9d6bd42c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3421=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3421=01-02 Stats: 25 lines in 1 file changed: 23 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Mon, 12 Apr 2021 16:18:32 GMT, Erik Joelsson wrote: >> Vladimir Kozlov 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 three additional >> commits since the last revision: >> >> - Restore Graal Builder image makefile >> - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 >> - 8264806: Remove the experimental JIT compiler > > make/common/Modules.gmk line 68: > >> 66: >> 67: # Filter out Graal specific modules >> 68: MODULES_FILTER += jdk.internal.vm.compiler > > If we are unconditionally filtering out these modules, then why leave the > module-info.java files in at all? We filter out because we can't build Graal anymore. But we need these module-info.java files because JVMCI's module-info.java references them: https://github.com/openjdk/jdk/blob/master/src/jdk.internal.vm.ci/share/classes/module-info.java#L26 Otherwise we can't build JVMCI which we continue to support. I filed followup RFE to implement Alan's suggestion to use Module API which will allow to remove these files later: https://bugs.openjdk.java.net/browse/JDK-8265091 - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sat, 10 Apr 2021 16:47:45 GMT, Igor Ignatyev wrote: >> Vladimir Kozlov 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 three additional >> commits since the last revision: >> >> - Restore Graal Builder image makefile >> - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 >> - 8264806: Remove the experimental JIT compiler > > Marked as reviewed by iignatyev (Reviewer). Thank you, Igor. I filed https://bugs.openjdk.java.net/browse/JDK-8265032 - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sat, 10 Apr 2021 15:38:11 GMT, Igor Ignatyev wrote: >> Vladimir Kozlov 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 three additional >> commits since the last revision: >> >> - Restore Graal Builder image makefile >> - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 >> - 8264806: Remove the experimental JIT compiler > > should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and > update a few of its users accordingly? > what about `vm.graal.enabled` `@requires` property? @iignatev If you think that I should clean tests anyway I will file follow up RFE to do that. - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
On Sat, 10 Apr 2021 15:38:11 GMT, Igor Ignatyev wrote:
> should we remove `sun.hotspot.code.Compiler::isGraalEnabled` method and
> update a few of its users accordingly?
> what about `vm.graal.enabled` `@requires` property?
Thank you, @iignatev for looking on changes.
I forgot to mention that `Compiler::isGraalEnabled()` returns always false now.
Because 94 tests uses `@requires !vm.graal.enabled` I don't want to include
them in these changes which are already very big. I am not sure if I should
modify tests if GraalVM group wants to run all these tests.
Unfortunately changes in `Compiler.java` are listed the last on `Files changed`
tab and GitHub has trouble to load these big changes - it takes time to see
them. Here `Compiler.java` chnges for review:
diff --git a/test/lib/sun/hotspot/code/Compiler.java
b/test/lib/sun/hotspot/code/Compiler.java
index 99122bd93b8..71288ae4482 100644
--- a/test/lib/sun/hotspot/code/Compiler.java
+++ b/test/lib/sun/hotspot/code/Compiler.java
@@ -60,33 +60,10 @@ public class Compiler {
/**
* Check if Graal is used as JIT compiler.
*
- * Graal is enabled if following conditions are true:
- * - we are not in Interpreter mode
- * - UseJVMCICompiler flag is true
- * - jvmci.Compiler variable is equal to 'graal'
- * - TieredCompilation is not used or TieredStopAtLevel is greater than 3
- * No need to check client mode because it set UseJVMCICompiler to false.
- *
- * @return true if Graal is used as JIT compiler.
+ * @return false because Graal is removed from JDK.
*/
public static boolean isGraalEnabled() {
-Boolean useCompiler = WB.getBooleanVMFlag("UseCompiler");
-if (useCompiler == null || !useCompiler) {
-return false;
-}
-Boolean useJvmciComp = WB.getBooleanVMFlag("UseJVMCICompiler");
-if (useJvmciComp == null || !useJvmciComp) {
-return false;
-}
-
-Boolean tieredCompilation = WB.getBooleanVMFlag("TieredCompilation");
-Long compLevel = WB.getIntxVMFlag("TieredStopAtLevel");
-// if TieredCompilation is enabled and compilation level is <= 3 then
no Graal is used
-if (tieredCompilation != null && tieredCompilation &&
-compLevel != null && compLevel <= 3) {
-return false;
-}
-return true;
+return false;
}
```
-
PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler
On Fri, 9 Apr 2021 17:35:11 GMT, Vladimir Kozlov wrote: > As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Thankyou, @erikj79, for review. I restored code as you asked. For some reasons incremental webrev shows update only in Cdiffs. - PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264806: Remove the experimental JIT compiler [v2]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Java-based JIT compiler (Graal) from JDK: > > - `jdk.internal.vm.compiler` — the Graal compiler > - `jdk.internal.vm.compiler.management` — Graal's `MBean` > - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests > > Remove Graal related code in makefiles. > > Note, next two `module-info.java` files are preserved so that the JVMCI > module `jdk.internal.vm.ci` continues to build: > src/jdk.internal.vm.compiler/share/classes/module-info.java > src/jdk.internal.vm.compiler.management/share/classes/module-info.java > > @AlanBateman suggested that we can avoid it by using Module API to export > packages at runtime . It requires changes in GraalVM's JVMCI too so I will > file followup RFE to implement it. > > Tested hs-tier1-4 Vladimir Kozlov 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 three additional commits since the last revision: - Restore Graal Builder image makefile - Merge latest changes from branch 'JDK-8264805' into JDK-8264806 - 8264806: Remove the experimental JIT compiler - Changes: - all: https://git.openjdk.java.net/jdk/pull/3421/files - new: https://git.openjdk.java.net/jdk/pull/3421/files/8ff9b599..a246aaa6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3421=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3421=00-01 Stats: 102 lines in 12 files changed: 66 ins; 27 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v5]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Ahead-of-Time Compiler from JDK: > > - `jdk.aot` module — the `jaotc` tool > - `src/hotspot/share/aot` — loads AoT compiled code into VM for execution > - related HotSpot code guarded by `#if INCLUDE_AOT` > > Additionally, remove tests as well as code in makefiles related to AoT > compilation. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/3398/files - new: https://git.openjdk.java.net/jdk/pull/3398/files/6cce0f6c..71a166c1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3398=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3398=03-04 Stats: 36 lines in 9 files changed: 0 ins; 27 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/3398.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3398/head:pull/3398 PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Fri, 9 Apr 2021 16:54:35 GMT, Ioi Lam wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove exports from Graal module to jdk.aot
>
> src/hotspot/share/oops/methodCounters.cpp line 77:
>
>> 75: }
>> 76:
>> 77: void MethodCounters::metaspace_pointers_do(MetaspaceClosure* it) {
>
> MethodCounters::metaspace_pointers_do can be removed altogether (also need to
> remove the declaration in methodCounter.hpp).
Done.
-
PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Fri, 9 Apr 2021 16:34:58 GMT, Igor Veresov wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove exports from Graal module to jdk.aot
>
> src/hotspot/share/jvmci/jvmciCodeInstaller.cpp line 1184:
>
>> 1182: }
>> 1183: } else if
>> (jvmci_env()->isa_HotSpotMetaspaceConstantImpl(constant)) {
>> 1184: if (!_immutable_pic_compilation) {
>
> All _immutable_pic_compilation mentions can be removed as well. It is true
> only for AOT compiles produced by Graal. It's never going to be used without
> AOT.
Done. I removed _immutable_pic_compilation in JVMCI in Hotspot.
-
PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Fri, 9 Apr 2021 16:30:41 GMT, Igor Veresov wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove exports from Graal module to jdk.aot > > src/hotspot/share/oops/instanceKlass.hpp line 257: > >> 255: _misc_declares_nonstatic_concrete_methods = 1 << 6, // directly >> declares non-static, concrete methods >> 256: _misc_has_been_redefined = 1 << 7, // class has >> been redefined >> 257: _unused = 1 << 8, // > > Any particular reason we don't want to remove this gap? Less changes. We don't get any benefits from removing it. It is opposite - if we need a new value we will use it without changing following values again. - PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Fri, 9 Apr 2021 08:32:59 GMT, Aleksey Shipilev wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove exports from Graal module to jdk.aot
>
> src/hotspot/share/code/compiledIC.cpp line 715:
>
>> 713: tty->print("interpreted");
>> 714: } else {
>> 715: tty->print("unknown");
>
> We can probably split this cleanup into the minor issue, your call. The
> benefit of separate issue: backportability.
Reverted and filed https://bugs.openjdk.java.net/browse/JDK-8265013
-
PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Fri, 9 Apr 2021 08:29:21 GMT, Aleksey Shipilev wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove exports from Graal module to jdk.aot > > src/hotspot/cpu/x86/globalDefinitions_x86.hpp line 73: > >> 71: >> 72: #if INCLUDE_JVMCI >> 73: #define COMPRESSED_CLASS_POINTERS_DEPENDS_ON_COMPRESSED_OOPS >> (EnableJVMCI) > > Minor nit: can probably drop parentheses here. done - PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Fri, 9 Apr 2021 03:03:33 GMT, David Holmes wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove exports from Graal module to jdk.aot
>
> src/hotspot/share/memory/heap.hpp line 174:
>
>> 172: bool contains(const void* p) const { return low() <= p &&
>> p < high(); }
>> 173: bool contains_blob(const CodeBlob* blob) const {
>> 174: const void* start = (void*)blob;
>
> start seems redundant now
Done:
bool contains_blob(const CodeBlob* blob) const {
-const void* start = (void*)blob;
-return contains(start);
+return contains((void*)blob);
}
-
PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Fri, 9 Apr 2021 02:44:23 GMT, David Holmes wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove exports from Graal module to jdk.aot > > src/hotspot/cpu/x86/compiledIC_x86.cpp line 134: > >> 132: #ifdef ASSERT >> 133: CodeBlob *cb = CodeCache::find_blob_unsafe((address) _call); >> 134: assert(cb, "sanity"); > > Nit: implied boolean - use "cb != NULL" done - PR: https://git.openjdk.java.net/jdk/pull/3398
RFR: 8264806: Remove the experimental JIT compiler
As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to Java-based JIT compiler (Graal) from JDK: - `jdk.internal.vm.compiler` — the Graal compiler - `jdk.internal.vm.compiler.management` — Graal's `MBean` - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests Remove Graal related code in makefiles. Note, next two `module-info.java` files are preserved so that the JVMCI module `jdk.internal.vm.ci` continues to build: src/jdk.internal.vm.compiler/share/classes/module-info.java src/jdk.internal.vm.compiler.management/share/classes/module-info.java @AlanBateman suggested that we can avoid it by using Module API to export packages at runtime . It requires changes in GraalVM's JVMCI too so I will file followup RFE to implement it. Tested hs-tier1-4 - Depends on: https://git.openjdk.java.net/jdk/pull/3398 Commit messages: - 8264806: Remove the experimental JIT compiler Changes: https://git.openjdk.java.net/jdk/pull/3421/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3421=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264806 Stats: 441620 lines in 2886 files changed: 0 ins; 441604 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/3421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3421/head:pull/3421 PR: https://git.openjdk.java.net/jdk/pull/3421
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Fri, 9 Apr 2021 17:09:58 GMT, Vladimir Kozlov wrote: >> Hi Vladimir, >> >> This looks good to me - I went through all the files. >> >> It was nice to see how nicely compartmentalized the AOT feature was - that >> made checking the changes quite simple. The one exception was the >> fingerprinting code, which for some reason was AOT-only but not marked that >> way, so I'm not sure what the story is there. ?? >> >> I was also surprised to see some of the flags were not marked experimental, >> so there will need to a CSR request to remove those without going through >> the normal deprecation process. >> >> Thanks, >> David > >> Hi Vladimir, >> >> This looks good to me - I went through all the files. >> >> It was nice to see how nicely compartmentalized the AOT feature was - that >> made checking the changes quite simple. The one exception was the >> fingerprinting code, which for some reason was AOT-only but not marked that >> way, so I'm not sure what the story is there. ?? >> >> I was also surprised to see some of the flags were not marked experimental, >> so there will need to a CSR request to remove those without going through >> the normal deprecation process. >> >> Thanks, >> David > > Thank you, David. > We thought that we could use fingerprint code for other cases that is why we > did not put it under `#if INCLUDE_AOT`. > I filed CSR for AOT product flags removal: > https://bugs.openjdk.java.net/browse/JDK-8265000 Thank you, Igor Ignatyev, Igor Veresov, Ioi, Aleksey and Andrew for reviews. I will update changes based on your comments. - PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Fri, 9 Apr 2021 04:32:14 GMT, David Holmes wrote: > Hi Vladimir, > > This looks good to me - I went through all the files. > > It was nice to see how nicely compartmentalized the AOT feature was - that > made checking the changes quite simple. The one exception was the > fingerprinting code, which for some reason was AOT-only but not marked that > way, so I'm not sure what the story is there. ?? > > I was also surprised to see some of the flags were not marked experimental, > so there will need to a CSR request to remove those without going through the > normal deprecation process. > > Thanks, > David Thank you, David. We thought that we could use fingerprint code for other cases that is why we did not put it under `#if INCLUDE_AOT`. I filed CSR for AOT product flags removal: https://bugs.openjdk.java.net/browse/JDK-8265000 - PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Thu, 8 Apr 2021 20:59:59 GMT, Coleen Phillimore wrote:
> There's a comment above
> void VM_RedefineClasses::mark_dependent_code(InstanceKlass* ik) {
> that should be rewritten, so I think we should just file an RFE to remove it
> afterwards.
https://bugs.openjdk.java.net/browse/JDK-8264941
-
PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Thu, 8 Apr 2021 20:00:30 GMT, Vladimir Kozlov wrote:
>> GC changes look good.
>
>> void CodeCache::mark_for_evol_deoptimization(InstanceKlass* dependee) {
>> This function, its caller and the function in jvmtiRedefineClasses.cpp that
>> calls it can be deleted also, but you can file a separate RFE for that if
>> you want.
>
> Thank you, Coleen, for review. I will wait other comments and will remove
> this code.
Thank you, Erik and Stefan.
-
PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
On Thu, 8 Apr 2021 19:58:11 GMT, Stefan Karlsson wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove exports from Graal module to jdk.aot
>
> GC changes look good.
> void CodeCache::mark_for_evol_deoptimization(InstanceKlass* dependee) {
> This function, its caller and the function in jvmtiRedefineClasses.cpp that
> calls it can be deleted also, but you can file a separate RFE for that if you
> want.
Thank you, Coleen, for review. I will wait other comments and will remove this
code.
-
PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Ahead-of-Time Compiler from JDK: > > - `jdk.aot` module — the `jaotc` tool > - `src/hotspot/share/aot` — loads AoT compiled code into VM for execution > - related HotSpot code guarded by `#if INCLUDE_AOT` > > Additionally, remove tests as well as code in makefiles related to AoT > compilation. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision: Remove exports from Graal module to jdk.aot - Changes: - all: https://git.openjdk.java.net/jdk/pull/3398/files - new: https://git.openjdk.java.net/jdk/pull/3398/files/3dbc6210..6cce0f6c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3398=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3398=02-03 Stats: 39 lines in 3 files changed: 0 ins; 36 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3398.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3398/head:pull/3398 PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v3]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Ahead-of-Time Compiler from JDK: > > - `jdk.aot` module — the `jaotc` tool > - `src/hotspot/share/aot` — loads AoT compiled code into VM for execution > - related HotSpot code guarded by `#if INCLUDE_AOT` > > Additionally, remove tests as well as code in makefiles related to AoT > compilation. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits: - Merge branch 'master' into JDK-8264805 - Merge branch 'master' into JDK-8264805 - 8264805: Remove the experimental Ahead-of-Time Compiler - Changes: https://git.openjdk.java.net/jdk/pull/3398/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3398=02 Stats: 26906 lines in 375 files changed: 4 ins; 26709 del; 193 mod Patch: https://git.openjdk.java.net/jdk/pull/3398.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3398/head:pull/3398 PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v2]
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to > Ahead-of-Time Compiler from JDK: > > - `jdk.aot` module — the `jaotc` tool > - `src/hotspot/share/aot` — loads AoT compiled code into VM for execution > - related HotSpot code guarded by `#if INCLUDE_AOT` > > Additionally, remove tests as well as code in makefiles related to AoT > compilation. > > Tested hs-tier1-4 Vladimir Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Merge branch 'master' into JDK-8264805 - 8264805: Remove the experimental Ahead-of-Time Compiler - Changes: https://git.openjdk.java.net/jdk/pull/3398/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3398=01 Stats: 26906 lines in 375 files changed: 4 ins; 26709 del; 193 mod Patch: https://git.openjdk.java.net/jdk/pull/3398.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3398/head:pull/3398 PR: https://git.openjdk.java.net/jdk/pull/3398
RFR: 8264805: Remove the experimental Ahead-of-Time Compiler
As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to Ahead-of-Time Compiler from JDK: - `jdk.aot` module — the `jaotc` tool - `src/hotspot/share/aot` — loads AoT compiled code into VM for execution - related HotSpot code guarded by `#if INCLUDE_AOT` Additionally, remove tests as well as code in makefiles related to AoT compilation. Tested hs-tier1-4 - Commit messages: - 8264805: Remove the experimental Ahead-of-Time Compiler Changes: https://git.openjdk.java.net/jdk/pull/3398/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3398=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264805 Stats: 26903 lines in 375 files changed: 4 ins; 26703 del; 196 mod Patch: https://git.openjdk.java.net/jdk/pull/3398.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3398/head:pull/3398 PR: https://git.openjdk.java.net/jdk/pull/3398
Re: RFR: 8264874: Build interim-langtools for HotSpot only if Graal is enabled
On Wed, 7 Apr 2021 22:37:28 GMT, Ioi Lam wrote: > Many HotSpot developers make only the `hotspot` target, and copy the > resulting `libjvm.so` into an already-build JDK image. > > HotSpot requires `interim-langtools` only when Graal is enabled. When Graal > is disabled, we can avoid building `interim-langtools`. This improves the > build time of `make hotspot` by about 20 seconds, or about 15%: > > Old: > $ make clean > $ time make hotspot > > real 1m57.905s > user 42m22.524s > sys 3m7.372s > > New: > $ make clean > $ time make hotspot > > real 1m39.916s > user 41m59.984s > sys 3m3.188s Good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3389
Re: Integrated: 8252709: Enable JVMCI when building linux-aarch64 at Oracle
On Mon, 22 Feb 2021 20:15:33 GMT, Doug Simon wrote: > To allow for better testing of JVMCI support on AArch64 in aid of producing a > reliable GraalVM release on this platform, it should be included by default. I would wait results of testing before approval. You may need to make additional changes to tests. Testing passed clean. - PR: https://git.openjdk.java.net/jdk/pull/2677Marked as reviewed by kvn (Reviewer).
Re: RFR: 8258010: Debug build failure with clang-10 due to -Wdeprecated-copy [v5]
On Mon, 11 Jan 2021 02:14:17 GMT, Hao Sun wrote: >> 1. '-Wdeprecated-copy' >> As specified in C++11 [1], "the generation of the implicitly-defined >> copy constructor is deprecated if T has a user-defined destructor or >> user-defined copy assignment operator". The rationale behind is the >> well-known Rule of Three [2]. >> >> Introduced since gcc-9 [3] and clang-10 [4], flag '-Wdeprecated-copy' >> warns about the C++11 deprecation of implicitly declared copy >> constructor and assignment operator if one of them is user-provided. >> Defining an explicit copy constructor would suppress this warning. >> >> The main reason why debug build with gcc-9 or higher succeeds lies in >> the inconsistent warning behaviors between gcc and clang. See the >> reduced code example [5]. We suspect it might be return value >> optimization/copy elision [6] that drives gcc not to declare implicit >> copy constructor for this case. >> >> Note that flag '-Wdeprecated' in clang-8 and clang-9 would also raise >> warnings for deprecated defintions of copy constructors. However, >> '-Wdeprecated' is not enabled by '-Wall' or '-Wextra'. Hence, clang-8 >> and clang-9 are not affected. >> >> ~~2. '-Wimplicit-int-float-conversion'~~ >> ~~Making the conversion explicit would fix it.~~ >> >> ~~Flag '-Wimplicit-int-float-conversion' is first introduced in clang-10.~~ >> ~~Therefore clang-8 and clang-9 are not affected. The flag with similar~~ >> ~~functionality in gcc is '-Wfloat-conversion', but it is not enabled by~~ >> ~~'-Wall' or '-Wextra'. That's why this warning does not apprear when~~ >> ~~building with gcc.~~ >> >> [1] https://en.cppreference.com/w/cpp/language/copy_constructor >> [2] https://en.cppreference.com/w/cpp/language/rule_of_three >> [3] https://www.gnu.org/software/gcc/gcc-9/changes.html >> [4] https://releases.llvm.org/10.0.0/tools/clang/docs/ReleaseNotes.html >> [5] https://godbolt.org/z/err4jM >> [6] https://en.wikipedia.org/wiki/Copy_elision#Return_value_optimization >> >> >> Note that we have tested with this patch, debug build succeeded with >> clang-10 on Linux X86-64/AArch64 machines. >> Note that '--with-extra-cxxflags=-Wno-implicit-int-float-conversion' should >> be added when configuration. It's another issue (See JDK-8259288) > > Hao Sun has updated the pull request incrementally with one additional commit > since the last revision: > > Define the copy assign operator of class DUIterator_Last as defaulted > > The copy assignment operator of class DUIterator_Last should also be > defined as defaulted, i.e. =default, keeping consistent with the copy > constructor. > > Besides, fix the NIT for the copy ctor definition of class > DUIterator_Last. > > Change-Id: I2f9502f023443163910eea9469b72df5bf1e25e0 > CustomizedGitHooks: yes Looks good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1874
Re: RFR: 8258010: Debug build failure with clang-10 due to -Wdeprecated-copy [v4]
On Wed, 6 Jan 2021 06:14:11 GMT, Hao Sun wrote: >> 1. '-Wdeprecated-copy' >> As specified in C++11 [1], "the generation of the implicitly-defined >> copy constructor is deprecated if T has a user-defined destructor or >> user-defined copy assignment operator". The rationale behind is the >> well-known Rule of Three [2]. >> >> Introduced since gcc-9 [3] and clang-10 [4], flag '-Wdeprecated-copy' >> warns about the C++11 deprecation of implicitly declared copy >> constructor and assignment operator if one of them is user-provided. >> Defining an explicit copy constructor would suppress this warning. >> >> The main reason why debug build with gcc-9 or higher succeeds lies in >> the inconsistent warning behaviors between gcc and clang. See the >> reduced code example [5]. We suspect it might be return value >> optimization/copy elision [6] that drives gcc not to declare implicit >> copy constructor for this case. >> >> Note that flag '-Wdeprecated' in clang-8 and clang-9 would also raise >> warnings for deprecated defintions of copy constructors. However, >> '-Wdeprecated' is not enabled by '-Wall' or '-Wextra'. Hence, clang-8 >> and clang-9 are not affected. >> >> ~~2. '-Wimplicit-int-float-conversion'~~ >> ~~Making the conversion explicit would fix it.~~ >> >> ~~Flag '-Wimplicit-int-float-conversion' is first introduced in clang-10.~~ >> ~~Therefore clang-8 and clang-9 are not affected. The flag with similar~~ >> ~~functionality in gcc is '-Wfloat-conversion', but it is not enabled by~~ >> ~~'-Wall' or '-Wextra'. That's why this warning does not apprear when~~ >> ~~building with gcc.~~ >> >> [1] https://en.cppreference.com/w/cpp/language/copy_constructor >> [2] https://en.cppreference.com/w/cpp/language/rule_of_three >> [3] https://www.gnu.org/software/gcc/gcc-9/changes.html >> [4] https://releases.llvm.org/10.0.0/tools/clang/docs/ReleaseNotes.html >> [5] https://godbolt.org/z/err4jM >> [6] https://en.wikipedia.org/wiki/Copy_elision#Return_value_optimization >> >> >> Note that we have tested with this patch, debug build succeeded with >> clang-10 on Linux X86-64/AArch64 machines. >> Note that '--with-extra-cxxflags=-Wno-implicit-int-float-conversion' should >> be added when configuration. It's another issue (See JDK-8259288) > > Hao Sun has updated the pull request incrementally with one additional commit > since the last revision: > > Split the PR, addressing -Wdeprecated-copy only > > As suggested by kimbarrett, we should focus on warnings produced by > '-Wdeprecated-copy' in this PR. Because JDK-8259288 is a very different > problem and might be reviewed by folks from different teams. > > Will create a new PR to address JDK-8259288. > > Change-Id: I1b9f434ab6fcdf2763a46870eaed91641984fd76 > CustomizedGitHooks: yes Still good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1874
Re: RFR: 8258010: Debug build failure with clang-10 due to -Wdeprecated-copy [v2]
On Wed, 6 Jan 2021 13:24:22 GMT, Hao Sun wrote: >> @vnkozlov I was wondering if you could take a look at this? We're not sure >> whether 'operator=' is problematic or not. Thanks. > > I manually checked the usages of assignment operators for class DUIterator, > DUIterator_Fast and DUIterator_Last. (Simply grep the class names in the > source code and check the context). > > ~~I found there exist only a couple of re-assignment usages. However, I guess > kind of reset operations are conducted in these sites, and I suspect the > implementation of `operator=` might be good.~~ > > As I examined, only the following 3 sites are found where re-assignment > happens > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/cfgnode.cpp#L565 > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/phaseX.cpp#L1878 > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/split_if.cpp#L452 > > Please take `cfgnode.cpp` as an example. `j` is first assigned at line 568 > and it would be assigned again if flag `progress` is true. ~~However, I > suppose `j` gets reset at line 578 in such case.~~ Note that `j` might be > re-assigned at line 578. However, it's self assignment and nothing is > conducted. > > ~~It might be incorrect if I missed something.~~ > Hope that this finding would be helpful to analyze this problem. Code is correct. _last should not be updated with additional assignments which updates only node's information. - PR: https://git.openjdk.java.net/jdk/pull/1874
Re: RFR: 8258010: Debug build failure with clang-10 due to -Wdeprecated-copy and -Wimplicit-int-float-conversion [v3]
On Tue, 5 Jan 2021 03:44:10 GMT, Hao Sun wrote: >> 1. '-Wdeprecated-copy' >> As specified in C++11 [1], "the generation of the implicitly-defined >> copy constructor is deprecated if T has a user-defined destructor or >> user-defined copy assignment operator". The rationale behind is the >> well-known Rule of Three [2]. >> >> Introduced since gcc-9 [3] and clang-10 [4], flag '-Wdeprecated-copy' >> warns about the C++11 deprecation of implicitly declared copy >> constructor and assignment operator if one of them is user-provided. >> Defining an explicit copy constructor would suppress this warning. >> >> The main reason why debug build with gcc-9 or higher succeeds lies in >> the inconsistent warning behaviors between gcc and clang. See the >> reduced code example [5]. We suspect it might be return value >> optimization/copy elision [6] that drives gcc not to declare implicit >> copy constructor for this case. >> >> Note that flag '-Wdeprecated' in clang-8 and clang-9 would also raise >> warnings for deprecated defintions of copy constructors. However, >> '-Wdeprecated' is not enabled by '-Wall' or '-Wextra'. Hence, clang-8 >> and clang-9 are not affected. >> >> 2. '-Wimplicit-int-float-conversion' >> Making the conversion explicit would fix it. >> >> Flag '-Wimplicit-int-float-conversion' is first introduced in clang-10. >> Therefore clang-8 and clang-9 are not affected. The flag with similar >> functionality in gcc is '-Wfloat-conversion', but it is not enabled by >> '-Wall' or '-Wextra'. That's why this warning does not apprear when >> building with gcc. >> >> [1] https://en.cppreference.com/w/cpp/language/copy_constructor >> [2] https://en.cppreference.com/w/cpp/language/rule_of_three >> [3] https://www.gnu.org/software/gcc/gcc-9/changes.html >> [4] https://releases.llvm.org/10.0.0/tools/clang/docs/ReleaseNotes.html >> [5] https://godbolt.org/z/err4jM >> [6] https://en.wikipedia.org/wiki/Copy_elision#Return_value_optimization >> >> >> Note that we have tested with this patch, debug build succeeded with >> clang-10 on Linux X86/AArch64 machines. > > Hao Sun has updated the pull request incrementally with one additional commit > since the last revision: > > Update the copy constructors for class DUIterator, DUIterator_Fast and > DUIterator_Last > > 1. Update copyright year to 2021. > 2. Add the definition of copy constructor for class DUIterator. > Otherwise, gcc with '-fno-elide-constructors' would raise a warning. > 3. For the copy constructor of class DUIterator_Fast, we initialize > '_vdui' as false, otherwise UB is introduced. > 4. It's better to define the copy constructor of class DUIterator_Last > as explicitly-defaulted, instead of leaving it for compilers to > implicitly define. > > Change-Id: I3d2f5b396aa116d1832f52da361ff3172459a87e > CustomizedGitHooks: yes node.hpp changes seems fine. Passed tier1 builds and testing. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1874
Re: RFR: 8258856: VM build without C1/C2 fails after JDK-8243205 [v3]
On Wed, 23 Dec 2020 10:09:08 GMT, Hao Sun wrote: >> The declaration sites for JVM flags were changed by JDK-8243205 and the >> subsequent JDK-8258074. As a result, undeclared identifier errors >> occurred while building VM without compiler1 or compiler2 feature. >> >> Making the corresponding header files included would fix it. >> >> Note that we have tested locally with this patch, build without C1/C2 >> succeeded on Linux X86/AArch64 machines. > > Hao Sun has updated the pull request incrementally with one additional commit > since the last revision: > > Use JVMCI compilation condition for oopMap.cpp > > Check the compilation condition INCLUDE_JVMCI before trying to include > the header file, i.e. jvmci_globals.hpp, for oopMap.cpp > > Change-Id: I9885291d9f971984d83942669a22ee030722a206 > CustomizedGitHooks: yes Good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1876
Re: RFR: 8256430: add linux-x64-optimized to tier1
On Tue, 17 Nov 2020 00:31:24 GMT, Igor Ignatyev wrote: > Hi all, > > > [8256414](https://bugs.openjdk.java.net/browse/JDK-8256414) / #1233 added > similar profile to submit workflow, this patch defines `linux-x64-optimized` > profile in jib-profile so it can be used by mach5 and added to tier1? > > Thanks > -- Igor > > cc-ing @dcubed-ojdk Good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1244
Re: RFR: 8256414: add optimized build to submit workflow
On Mon, 16 Nov 2020 18:26:18 GMT, Igor Ignatyev wrote: > Hi all, > > Could you please review this small and trivial patch which adds > `linux-x64-optimized` build to submit workflow so breakages of this build > flavor would be easier to spot? > > Thanks, > -- Igor Marked as reviewed by kvn (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1233
Re: RFR: 8255965: LogCompilation: add sort by nmethod code size
On Thu, 5 Nov 2020 22:13:54 GMT, Eric Caspole wrote: > While profiling an issue I added this sort by code size to LogCompilation, > using -z > > $ java -ea -jar target/LogCompilation-1.0-SNAPSHOT.jar -z 2000-2.log | head > > 879 4 com.fee.fi.fo.Fum::foobar (3076 bytes)(code size: 57344) > 853 make_not_entrant > 853 3 com.fee.fi.fo.Fum::foobar (3076 bytes)(code size: 55968) > 895 4 com.fee.fi.fo.Fum::baz (2238 bytes)(code size: 46112) > 888 4 com.fee.fi.fo.Fum::quux (2165 bytes)(code size: 43200) > > The code size = stub_offset - insts_offset from what is in the log. > This makes it easier to see, for example, if changing compiler XX options > make huge differences in inlining. Good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1085
Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v3]
On Tue, 20 Oct 2020 15:46:36 GMT, Bernhard Urban-Forster wrote: >> Use r18 as allocatable register on Linux only. >> >> A bootstrap works now (it has been crashing before due to r18 being >> allocated): >> $ ./windows-aarch64-server-fastdebug/bin/java.exe >> -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI >> -version >> Bootstrapping JVMCI. in 17990 ms >> (compiled 3330 methods) >> openjdk version "16-internal" 2021-03-16 >> OpenJDK Runtime Environment (fastdebug build >> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) >> OpenJDK 64-Bit Server VM (fastdebug build >> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) >> >> Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. > > Bernhard Urban-Forster 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: > > - add missing precompiled.hpp include > - Merge remote-tracking branch 'upstream/master' into > 8254827-enable-jvmci-win-aarch64 > - rename argument to canUsePlatformRegister > - comment for platformRegister > - 8254827: JVMCI: Enable it for Windows+AArch64 > >Use r18 as allocatable register on Linux only. > >A bootstrap works now (it has been crashing before due to r18 being > allocated): >```console >$ ./windows-aarch64-server-fastdebug/bin/java.exe > -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI > -version >Bootstrapping JVMCI. in 17990 ms >(compiled 3330 methods) >openjdk version "16-internal" 2021-03-16 >OpenJDK Runtime Environment (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) >OpenJDK 64-Bit Server VM (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) >``` > >Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. Marked as reviewed by kvn (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/685
Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v3]
On Mon, 2 Nov 2020 20:05:10 GMT, Bernhard Urban-Forster wrote: >> make/autoconf/jvm-features.m4 line 309: >> >>> 307: if test "x$OPENJDK_TARGET_CPU" = "xx86_64"; then >>> 308: AC_MSG_RESULT([yes]) >>> 309: elif test "x$OPENJDK_TARGET_CPU" = "xaarch64"; then >> >> You are missing the same change for JVM_FEATURES_CHECK_JVMCI. >> Unless it is done intentionally. > > It's done a couple lines below: > https://github.com/openjdk/jdk/pull/685/files#diff-a09b08bcd422d0a8fb32a95ccf85051ac1e69bef2bd420d579f74d8efa286d2fL343 > > Or do you mean something else? Uhh. Sorry, I thought JVMCI definition will be first, before Graal and did not look below. - PR: https://git.openjdk.java.net/jdk/pull/685
Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v3]
On Tue, 20 Oct 2020 15:46:36 GMT, Bernhard Urban-Forster wrote: >> Use r18 as allocatable register on Linux only. >> >> A bootstrap works now (it has been crashing before due to r18 being >> allocated): >> $ ./windows-aarch64-server-fastdebug/bin/java.exe >> -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI >> -version >> Bootstrapping JVMCI. in 17990 ms >> (compiled 3330 methods) >> openjdk version "16-internal" 2021-03-16 >> OpenJDK Runtime Environment (fastdebug build >> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) >> OpenJDK 64-Bit Server VM (fastdebug build >> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) >> >> Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. > > Bernhard Urban-Forster 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: > > - add missing precompiled.hpp include > - Merge remote-tracking branch 'upstream/master' into > 8254827-enable-jvmci-win-aarch64 > - rename argument to canUsePlatformRegister > - comment for platformRegister > - 8254827: JVMCI: Enable it for Windows+AArch64 > >Use r18 as allocatable register on Linux only. > >A bootstrap works now (it has been crashing before due to r18 being > allocated): >```console >$ ./windows-aarch64-server-fastdebug/bin/java.exe > -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI > -version >Bootstrapping JVMCI. in 17990 ms >(compiled 3330 methods) >openjdk version "16-internal" 2021-03-16 >OpenJDK Runtime Environment (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) >OpenJDK 64-Bit Server VM (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) >``` > >Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. Changes requested by kvn (Reviewer). make/autoconf/jvm-features.m4 line 309: > 307: if test "x$OPENJDK_TARGET_CPU" = "xx86_64"; then > 308: AC_MSG_RESULT([yes]) > 309: elif test "x$OPENJDK_TARGET_CPU" = "xaarch64"; then You are missing the same change for JVM_FEATURES_CHECK_JVMCI. Unless it is done intentionally. - PR: https://git.openjdk.java.net/jdk/pull/685
Integrated: 8255616: Disable AOT and Graal in Oracle OpenJDK
On Fri, 30 Oct 2020 17:40:51 GMT, Vladimir Kozlov wrote: > We shipped Ahead-of-Time compilation (the jaotc tool) in JDK 9, as an > experimental feature. We shipped Graal as an experimental JIT compiler in JDK > 10. We haven't seen much use of these features, and the effort required to > support and enhance them is significant. We therefore intend to disable these > features in Oracle builds as of JDK 16. > > We'll leave the sources for these features in the repository, in case any one > else is interested in building them. But we will not update or test them. > > We'll continue to build and ship JVMCI as an experimental feature in Oracle > builds. > > Tested changes in all tiers. > > I verified that with these changes I still able to build Graal in open repo > and run graalunit testing: > > `open$ bash test/hotspot/jtreg/compiler/graalunit/downloadLibs.sh > /mydir/graalunit_lib/` > `open$ bash configure --with-debug-level=fastdebug > --with-graalunit-lib=/mydir/graalunit_lib/ --with-jtreg=/mydir/jtreg` > `open$ make jdk-image` > `open$ make test-image` > `open$ make run-test TEST=compiler/graalunit/HotspotTest.java` This pull request has now been integrated. Changeset: 2f7d34f2 Author:Vladimir Kozlov URL: https://git.openjdk.java.net/jdk/commit/2f7d34f2 Stats: 36 lines in 4 files changed: 21 ins; 11 del; 4 mod 8255616: Disable AOT and Graal in Oracle OpenJDK Reviewed-by: iignatyev, vlivanov, iveresov, ihse - PR: https://git.openjdk.java.net/jdk/pull/960
Re: RFR: 8255616: Disable AOT and Graal in Oracle OpenJDK
On Sun, 1 Nov 2020 20:15:01 GMT, Igor Veresov wrote: >> We shipped Ahead-of-Time compilation (the jaotc tool) in JDK 9, as an >> experimental feature. We shipped Graal as an experimental JIT compiler in >> JDK 10. We haven't seen much use of these features, and the effort required >> to support and enhance them is significant. We therefore intend to disable >> these features in Oracle builds as of JDK 16. >> >> We'll leave the sources for these features in the repository, in case any >> one else is interested in building them. But we will not update or test them. >> >> We'll continue to build and ship JVMCI as an experimental feature in Oracle >> builds. >> >> Tested changes in all tiers. >> >> I verified that with these changes I still able to build Graal in open repo >> and run graalunit testing: >> >> `open$ bash test/hotspot/jtreg/compiler/graalunit/downloadLibs.sh >> /mydir/graalunit_lib/` >> `open$ bash configure --with-debug-level=fastdebug >> --with-graalunit-lib=/mydir/graalunit_lib/ --with-jtreg=/mydir/jtreg` >> `open$ make jdk-image` >> `open$ make test-image` >> `open$ make run-test TEST=compiler/graalunit/HotspotTest.java` > > Marked as reviewed by iveresov (Reviewer). Thank you for reviews. - PR: https://git.openjdk.java.net/jdk/pull/960
RFR: 8255616: Disable AOT and Graal in Oracle OpenJDK
We shipped Ahead-of-Time compilation (the jaotc tool) in JDK 9, as an experimental feature. We shipped Graal as an experimental JIT compiler in JDK 10. We haven't seen much use of these features, and the effort required to support and enhance them is significant. We therefore intend to disable these features in Oracle builds as of JDK 16. We'll leave the sources for these features in the repository, in case any one else is interested in building them. But we will not update or test them. We'll continue to build and ship JVMCI as an experimental feature in Oracle builds. Tested changes in all tiers. I verified that with these changes I still able to build Graal in open repo and run graalunit testing: `open$ bash test/hotspot/jtreg/compiler/graalunit/downloadLibs.sh /mydir/graalunit_lib/` `open$ bash configure --with-debug-level=fastdebug --with-graalunit-lib=/mydir/graalunit_lib/ --with-jtreg=/mydir/jtreg` `open$ make jdk-image` `open$ make test-image` `open$ make run-test TEST=compiler/graalunit/HotspotTest.java` - Commit messages: - 8255616: Disable AOT and Graal in Oracle OpenJDK Changes: https://git.openjdk.java.net/jdk/pull/960/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=960=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255616 Stats: 36 lines in 4 files changed: 21 ins; 11 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/960.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/960/head:pull/960 PR: https://git.openjdk.java.net/jdk/pull/960
Re: RFR: 8223347: Integration of Vector API (Incubator)
On Fri, 25 Sep 2020 20:14:29 GMT, Paul Sandoz wrote: > This pull request is for integration of the Vector API. It was previously > reviewed under conditions when mercurial was > used for the source code control system. Review threads can be found here > (searching for issue number 8223347 in the > title): > https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/thread.html > https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/thread.html > https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/thread.html > > If mercurial was still being used the code would be pushed directly, once the > CSR is approved. However, in this case a > pull request is required and needs explicit reviewer approval. Between the > final review and this pull request no code > has changed, except for that related to merging. Good - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/367
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v5]
On Mon, 28 Sep 2020 19:08:04 GMT, Philippe Marschall wrote: >> @marschall I will sponsor it after you integrate the latest update. > > @vnkozlov done, I hope I now made it correctly with a merge commit for the > latest merge conflict hs-tier1, hs-tier3-graal testing passed - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v5]
On Wed, 23 Sep 2020 20:02:45 GMT, Erik Gahlin wrote: >> Philippe Marschall has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now >> contains one commit: >> 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate > > Marked as reviewed by egahlin (Reviewer). @marschall I will sponsor it after you integrate the latest update. - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v5]
On Wed, 23 Sep 2020 18:41:06 GMT, Philippe Marschall wrote: >> Hello, newbie here >> >> I picked JDK-8138732 to work on because it has a "starter" label and I >> believe I understand what to do. >> >> - I tried to update the copyright year to 2020 in every file. >> - I decided to change `@since` from 9 to 16 since it is a new annotation >> name in a new package. >> - I tried to keep code changes to a minimum, eg not change to imports if >> fully qualified class names are used instead of >> imports. In some cases I did minor reordering of imports to keep them >> sorted alphabetically. >> - All tier1 tests pass. >> - One jpackage/jlink tier2 test fails but I don't believe it is related. >> - Some tier3 Swing tests fail but I don't think they are related. > > Philippe Marschall has updated the pull request with a new target base due to > a merge or a rebase. The pull request now > contains one commit: > 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate Update seems fine. Thanks for JFR testing and fixing. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Wed, 9 Sep 2020 09:49:44 GMT, Philippe Marschall wrote: >> Hello, newbie here >> >> I picked JDK-8138732 to work on because it has a "starter" label and I >> believe I understand what to do. >> >> - I tried to update the copyright year to 2020 in every file. >> - I decided to change `@since` from 9 to 16 since it is a new annotation >> name in a new package. >> - I tried to keep code changes to a minimum, eg not change to imports if >> fully qualified class names are used instead of >> imports. In some cases I did minor reordering of imports to keep them >> sorted alphabetically. >> - All tier1 tests pass. >> - One jpackage/jlink tier2 test fails but I don't believe it is related. >> - Some tier3 Swing tests fail but I don't think they are related. > > Philippe Marschall has refreshed the contents of this pull request, and > previous commits have been removed. The > incremental views will show differences compared to the previous content of > the PR. Marked as reviewed by kvn (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Fri, 11 Sep 2020 21:53:12 GMT, Vladimir Kozlov wrote: >> Marked as reviewed by psandoz (Reviewer). > > You should consider upstreaming Graal changes (jdk.internal.vm.compiler) into > https://github.com/oracle/graal > Otherwise next time we do "Update Graal" in JDK they will be overwritten. Changes look good. I ran hs-tier1 and hs-tier3 test jobs which run Graal tests and they passed. - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Thu, 10 Sep 2020 17:59:54 GMT, Paul Sandoz wrote: >> Philippe Marschall has refreshed the contents of this pull request, and >> previous commits have been removed. The >> incremental views will show differences compared to the previous content of >> the PR. > > Marked as reviewed by psandoz (Reviewer). You should consider upstreaming Graal changes (jdk.internal.vm.compiler) into https://github.com/oracle/graal Otherwise next time we do "Update Graal" in JDK they will be overwritten. - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR(XS): 8244061: Disable jvmci/graal/aot when building linux-aarch64 at Oracle
Good. Thanks, Vladimir On 4/28/20 9:12 PM, Mikael Vidstedt wrote: Please review this small change which disables JVMCI, Graal, and AOT when building linux-aarch64 at Oracle, for now. JBS: https://bugs.openjdk.java.net/browse/JDK-8244061 webrev: http://cr.openjdk.java.net/~mikael/webrevs/8244061/webrev.00/open/webrev/ Cheers, Mikael
Re: 14 RFR (M) 8232080: jlink plugins for vendor information and run-time options
+1
Thanks
Vladimir
> On Oct 22, 2019, at 3:42 PM, Bob Vandette wrote:
>
> Hotspot changes look good to me.
>
> Bob.
>
>
>> On Oct 22, 2019, at 6:36 PM, mark.reinh...@oracle.com wrote:
>>
>> 2019/10/22 12:43:42 -0700, bob.vande...@oracle.com:
> On Oct 22, 2019, at 3:22 PM, mark.reinh...@oracle.com wrote:
> 2019/10/22 10:31:55 -0700, bob.vande...@oracle.com:
> In arguments.cpp, could you use a new JVMFlag to declare options
> that came from this resource as RESOURCE?
>
> - jint result = parse_each_vm_init_arg(vm_options_args,
> _mod_javabase, JVMFlag::INTERNAL);
> + jint result = parse_each_vm_init_arg(vm_options_args,
> _mod_javabase, JVMFlag::RESOURCE);
>
> This will require some minor changes to jvmFlags.hpp
>
> ...
Yes, that’d make sense, in which case I’d also change JVMFlag::print_origin
to handle the RESOURCE case (which is easy).
Is “RESOURCE” the best name here? Sounds awfully generic. How about
“JIMAGE” or “JIMAGE_RESOURCE”?
>>>
>>> JIMAGE_RESOURCE or VM_OPTIONS_RESOURCE works for me.
>>
>> JIMAGE_RESOURCE it is, then. Relative patch below; original webrev
>> updated in place (https://cr.openjdk.java.net/~mr/rev/8232080/).
>>
>> - Mark
>>
>>
>>
>>
>> # HG changeset patch
>> # Parent efca1844245ad7351418ef41efc86c5055ac3edf
>> Addendum 1 (JVMFlags): 8232080: jlink plugins for vendor information and
>> run-time options
>>
>> diff --git a/src/hotspot/share/runtime/arguments.cpp
>> b/src/hotspot/share/runtime/arguments.cpp
>> --- a/src/hotspot/share/runtime/arguments.cpp
>> +++ b/src/hotspot/share/runtime/arguments.cpp
>> @@ -2203,7 +2203,7 @@
>> set_mode_flags(_mixed);
>>
>> // Parse args structure generated from java.base vm options resource
>> - jint result = parse_each_vm_init_arg(vm_options_args,
>> _mod_javabase, JVMFlag::INTERNAL);
>> + jint result = parse_each_vm_init_arg(vm_options_args,
>> _mod_javabase, JVMFlag::JIMAGE_RESOURCE);
>> if (result != JNI_OK) {
>>return result;
>> }
>> diff --git a/src/hotspot/share/runtime/flags/jvmFlag.cpp
>> b/src/hotspot/share/runtime/flags/jvmFlag.cpp
>> --- a/src/hotspot/share/runtime/flags/jvmFlag.cpp
>> +++ b/src/hotspot/share/runtime/flags/jvmFlag.cpp
>> @@ -697,6 +697,8 @@
>> st->print("attach"); break;
>>case INTERNAL:
>> st->print("internal"); break;
>> +case JIMAGE_RESOURCE:
>> + st->print("jimage"); break;
>> }
>> st->print("}");
>> }
>> diff --git a/src/hotspot/share/runtime/flags/jvmFlag.hpp
>> b/src/hotspot/share/runtime/flags/jvmFlag.hpp
>> --- a/src/hotspot/share/runtime/flags/jvmFlag.hpp
>> +++ b/src/hotspot/share/runtime/flags/jvmFlag.hpp
>> @@ -44,8 +44,9 @@
>>ERGONOMIC= 5,
>>ATTACH_ON_DEMAND = 6,
>>INTERNAL = 7,
>> +JIMAGE_RESOURCE = 8,
>>
>> -LAST_VALUE_ORIGIN = INTERNAL,
>> +LAST_VALUE_ORIGIN = JIMAGE_RESOURCE,
>>VALUE_ORIGIN_BITS = 4,
>>VALUE_ORIGIN_MASK = right_n_bits(VALUE_ORIGIN_BITS),
>>
>
Re: 14 RFR (M) 8232080: jlink plugins for vendor information and run-time options
HotSpot changes seem fine to me.
Thanks,
Vladimir
On 10/21/19 8:22 PM, mark.reinh...@oracle.com wrote:
RFE: https://bugs.openjdk.java.net/browse/JDK-8232080
CSR: https://bugs.openjdk.java.net/browse/JDK-8232753
Webrev: https://cr.openjdk.java.net/~mr/rev/8232080/
This change implements jlink plugins, and associated changes in the VM
and libraries, to support the following new jlink options:
- --vendor-bug-url= overrides the vendor bug URL baked
into the build. The value of the system property "java.vendor.url.bug"
will be .
- --vendor-vm-bug-url= overrides the vendor VM bug
URL baked into the build. This value will be displayed in VM error
logs.
- --vendor-version= overrides the vendor version string
baked into the build, if any. The value of the system property
"java.vendor.version" will be . This value will be
displayed in the output of java --version.
- --add-options= prepends the specified string,
which may include whitespace, before any other options when invoking
the VM in the resulting image.
The vendor-information plugins work by using the JDK’s internal ASM
library to redefine static fields in the java.lang.VersionProps class.
The VM reads the vendor-version and vendor-vm-bug-url strings from that
class during startup.
The add-options plugin works by storing the requested options in an
internal resource, /java.base/jdk/internal/vm/options. The VM loads that
resource from the lib/modules jimage file during startup and prepends any
options found there to those given on the command line.
Passes tier1-3 and JCK on {linux,macos,windows}-x64.
Thanks,
- Mark
Re: RFR(XXS) 8231902: Build of jdk.internal.vm.compiler.management/module-info.java.extra failed
Good. Thanks Vladimir > On Oct 4, 2019, at 12:29 PM, dean.l...@oracle.com wrote: > > https://bugs.openjdk.java.net/browse/JDK-8231902 > http://cr.openjdk.java.net/~dlong/8231902/webrev/ > > A recent upstream Graal change causes the META-INF/providers directory to > contain only a single file, causing the $(GREP) command to not prepend the > filename to the output. The simple fix is to create an empty file in the > directory. > > dl
Re: RFR: JDK-8221766: Load-reference barriers for Shenandoah
Good (C2 part). Thanks, Vladimir On 4/3/19 10:13 AM, Roman Kennke wrote: I don't think it should be part of this cleanup. Fair enough. I have run several tests today, and removing the is_Phi() call doesn't seem to negatively impact Shenandoah. Updated webrevs: Incremental: http://cr.openjdk.java.net/~rkennke/JDK-8221766/webrev.01.diff/ Full: http://cr.openjdk.java.net/~rkennke/JDK-8221766/webrev.01/ Ok now? Thanks, Roman Please, file separate RFE to push this change with separate review and testing. Thanks, Vladimir On 4/3/19 4:18 AM, Roland Westrelin wrote: Hi Vladimir, opto/loopnode.cpp new is_Phi check was added. Please, explain. When we expand barriers, if we find a null check nearby we move the barrier close to the null check so there's a better chance of converting it to an implicit null check. That happens as part of a pass of loop opts. I think that's where that change comes from but I don't remember the details. In general we need the control that's assigned to a load to not be too conservative. Anyway, that change is not required for correctness. But it looks reasonable to me. Roland.
Re: RFR: JDK-8221766: Load-reference barriers for Shenandoah
I don't think it should be part of this cleanup. Please, file separate RFE to push this change with separate review and testing. Thanks, Vladimir On 4/3/19 4:18 AM, Roland Westrelin wrote: Hi Vladimir, opto/loopnode.cpp new is_Phi check was added. Please, explain. When we expand barriers, if we find a null check nearby we move the barrier close to the null check so there's a better chance of converting it to an implicit null check. That happens as part of a pass of loop opts. I think that's where that change comes from but I don't remember the details. In general we need the control that's assigned to a load to not be too conservative. Anyway, that change is not required for correctness. But it looks reasonable to me. Roland.
Re: RFR: JDK-8221766: Load-reference barriers for Shenandoah
This is nice cleanup :) 4294 lines changed: 977 ins; 2841 del; 476 mod First is general question. I don't understand why you need (diagnostic) ShenandoahLoadRefBarrier flag if it is new behavior and you can't use old one because you removed it. I am definitely missing something here. Thank you for thinking about Graal: >==> good for upcoming Graal (sup)port opto/loopnode.cpp new is_Phi check was added. Please, explain. I don't see other issues in C2 code. Regards, Vladimir On 4/2/19 2:12 PM, Roman Kennke wrote: (I am cross-posting this to build-dev and compiler-dev because this contains some (trivial-ish) shared build and C2 changes. The C2 changes are almost all reversals of Shenandoah-specific paths that have been introduced in initial Shenandoah push.) I would like to propose that we switch to what we came to call 'load reference barrier' as new barrier scheme for Shenandoah GC. The main difference is that instead of ensuring correct invariant when we store anything into the heap (e.g. read-barrier before reads, write-barrier before writes, plus a bunch of other stuff), we ensure the strong invariance on objects when they get loaded, by employing what is currently our write-barrier. The reason why I'm proposing it is: - simpler barrier interface - easier to get good performance out of it ==> good for upcoming Graal (sup)port - reduced maintenance burden (I intend to backport it all the way) This has a number of advantages: - Strong invariant means it's a lot easier to reason about the state of GC and objects - Much simpler barrier interface. Infact, a lot of stuff that we added to barrier interfaces after JDK11 will now become unused: no need for barriers on primitives, no need for object equality barriers, no need for resolve barriers, etc. Also, some C2 stuff that we added for Shenandoah can now be removed again. (Those are what comprise most shared C2 changes.) - Optimization is much easier: we currently put barriers 'down low' close to their uses (which might be inside a hot loop), and then work hard to optimize barriers upwards, e.g. out of loops. By using load-ref-barriers, we would place them at the outermost site already. Look how much code is removed from shenandoahSupport.cpp! - No more need for object equals barriers. - No more need for 'resolve' barriers. - All barriers are now conditional, which opens up opportunity for further optimization later on. - we can re-enable the fast JNI getfield stuff - we no longer need the nmethod initializer that initializes embedded oops to to-space - We no longer have the problem to use two registers for 'the same' value (pre- and post-barrier). The 'only' optimizations that we do in C2 are: - Look upwards and see if barrier input indicates we don't actually need the barrier. Would be the case for: constants, nulls, method parameters, etc (anything that is not like a load). Even though we insert barriers after loads, you'd be surprised to see how many loads actually disappear. - Look downwards to check uses of the barrier. If it doesn't feed into anything that requires a barrier, we can remove it. Performance doesn't seem to be negatively impacted at all. Some benchmarks benefit positively from it. Testing: Testing: hotspot_gc_shenandoah, SPECjvm2008, SPECjbb2015, all of them many times. This patch has baked in shenandoah/jdk for 1.5 months, undergone our rigorous CI, received various bug-fixes, we have had a close look at the generated code to verify it is sane. jdk/submit job expected good before push. Bug: https://bugs.openjdk.java.net/browse/JDK-8221766 Webrev: http://cr.openjdk.java.net/~rkennke/JDK-8221766/webrev.00/ Can I please get reviews for this change? Roman
