On Wed, 17 Jun 2026 23:19:51 GMT, Dean Long <[email protected]> wrote:

>> Roland Westrelin has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains 11 commits:
>> 
>>  - review
>>  - Merge branch 'lworld' into JDK-8385328
>>  - more
>>  - review
>>  - review
>>  - Merge branch 'lworld' into JDK-8385328
>>  - more
>>  - review
>>  - whitespaces
>>  - more
>>  - ... and 1 more: 
>> https://git.openjdk.org/valhalla/compare/abebcaa0...fa48b671
>
> src/hotspot/share/opto/escape.cpp line 4549:
> 
>> 4547: //
>> 4548: #define FIND_INST_MEM_RECURSION_DEPTH_LIMIT 1000
>> 4549: 
> 
> I suggest a helper function that adds asserts, something like:
> Suggestion:
> 
> static bool same_instance_as(const TypeOopPtr* t1, const TypeOopPtr* t2) {
>   assert(t1->is_known_instance(), "known instance expected during EA phase");
>   assert(t2->is_known_instance(), "known instance expected during EA phase");
>   return t1->instance_id() == t2->instance_id();
> }
> 
> and using it below.

It would have to be:

assert(t1->is_known_instance() || t2->is_known_instance(), "known instance 
expected during EA phase");

There's no guarantee both of them are known instances and that code is correct 
if only one is as, I think, we discussed before. Would you be ok with this?

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2489#discussion_r3434471896

Reply via email to