RFR: 8340141: C1: rework ciMethod::equals following 8338471 [v3]

[Vladimir Ivanov]

On Thu, 10 Oct 2024 22:40:44 GMT, Dean Long <dlong at openjdk.org> wrote:

>> This PR changes ciMethod::equals() to a special-purpose debug helper method for the one place in C1 that uses it in an assert.  The reason why making it general purpose is difficult is because JVMTI can add and delete methods.   See the bug report and JDK-8338471 for more details.  I'm open to suggestions for a better name than equals_ignore_version().
>> 
>> An alternative approach, which I think may actually be better, would be to check for old methods first, and bail out if we see any.  Then we can change the assert back to how it was originally, using ==.
>
> Dean Long has updated the pull request incrementally with one additional commit since the last revision:
> 
>   make sure to be in VM state when checking is_old

src/hotspot/share/ci/ciMethod.cpp line 695:

> 693:   // Redefinition support.
> 694:   if (this->is_old() || root_m->is_old()) {
> 695:     return nullptr;

IMO you can safely drop this particular check. The one after `Dependencies::find_unique_concrete_method()` should be enough to preserve the invariant (`target == cha_monomorphic_target`) .

But thinking more about it, now I'm curious what happens when an old method is actually encountered. The fix conservatively rejects possible inlining opportunity, but it seems it doesn't invalidate resulting nmethod anymore. So, no recompilation attempt follows to recuperate that.

We could either record a evol dependency on a stale `Method` (to fail during nmethod installation step) or fail-fast the compilation (probably, implies additional checks to propagate the failure status).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21148#discussion_r1796190689