On Wed, 16 Jul 2025 05:28:55 GMT, Tobias Hartmann <thartm...@openjdk.org> wrote:

>> Volkan Yazici 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 12 additional 
>> commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'upstream/master' into strIntrinCheck
>>  - Apply review feedback (styling changes)
>>  - Minimize the number of touched lines in `vmIntrinsics.hpp`
>>  - Remove Markdown-styling in comments
>>  - Improve wording of the `VerifyIntrinsicChecks` flag
>>  - Merge remote-tracking branch 'upstream/master' into strIntrinCheck
>>  - Fix `EUC_JP.java.template` broken due to `encodeASCII` rename
>>  - Remove `StringCodingCountPositives`, `String{En,De}code` already cover 
>> our cases
>>    
>>    This reverts commit 196fc5d406851b8e7070c97ac53ca59c4615aad9.
>>  - Improve intrinsics in `StringCoding`
>>  - Add `StringCodingCountPositives` benchmark
>>  - ... and 2 more: https://git.openjdk.org/jdk/compare/921f85ab...85f19864
>
> src/hotspot/share/opto/c2_globals.hpp line 665:
> 
>> 663:           "prints attempted and successful inlining of intrinsics")     
>>     \
>> 664:                                                                         
>>     \
>> 665:   develop(bool, VerifyIntrinsicChecks, false,                           
>>     \
> 
> We should add testing that uses this new flag. Maybe we could add a run to 
> the tests that check the affected intrinsics? We could also add it to our 
> (Oracle internal) stress test jobs at higher tiers.

@TobiHartmann, agreed. I've pushed two commits:

1. bcb073cbb41 adds `TestVerifyIntrinsicChecks` to verify the effectiveness of 
the `VerifyIntrinsicChecks` VM flag. That is, does VM indeed crash if intrinsic 
Java wrapper fails to properly validate the input?
2. abc0eeb3bfc adds a separate `/* @test ... @run ... 
-XX:+VerifyIntrinsicChecks` block to the tests using `StringCoding` intrinsics

Please let me know if these two address your remarks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25998#discussion_r2212371103

Reply via email to