On Mon, 1 Jun 2026 08:12:55 GMT, Manuel Hässig <[email protected]> wrote:

>> Vladimir Ivanov has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   updates
>
> src/hotspot/share/opto/library_call.cpp line 2418:
> 
>> 2416:   bool can_access_non_heap = 
>> TypePtr::NULL_PTR->higher_equal(_gvn.type(base));
>> 2417: 
>> 2418:   assert(!is_non_heap_access || can_access_non_heap, "sanity"); // 
>> is_non_heap_access implies can_access_non_heap
> 
> Suggestion:
> 
>   assert(!is_non_heap_access || can_access_non_heap, "is_non_heap_access must 
> imply can_access_non_heap");
> 
> Makes the assert a bit more descriptive.

Well, the condition is sort of self-explanatory. I don't see much value in 
expanding the message part. IMO a comment is enough to clarify the intention.

> test/hotspot/jtreg/compiler/unsafe/UnsafeBooleanTest.java line 125:
> 
>> 123:             Short.MIN_VALUE, Short.MAX_VALUE,
>> 124:             Character.MIN_VALUE , Character.MAX_VALUE,
>> 125:             Integer.MIN_VALUE, Integer.MAX_VALUE };
> 
> I think it would be a good idea to add a random int input as well for 
> additional coverage at least for the non-constant tests.

Ok, sounds reasonable. Added.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31249#discussion_r3336609072
PR Review Comment: https://git.openjdk.org/jdk/pull/31249#discussion_r3336616211

Reply via email to