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

[Dean Long]

On Thu, 10 Oct 2024 23:09:00 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Even if we check for stale Methods in various places, including invoke(), there is nothing to prevent the method from going stale after the last spot-check.  My understanding was that we already handle stale metadata as a precondition to creating the nmethod.  If we have a loophole there that lets stale metadata get through, then that's a separate existing bug for C1 and C2.
>> I was tempted to add a bailout, but the reason would be as a performance improvement to short-circuit wasted work, not to correct a stale metadata problem.
>
>> 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) .
> 
> If I do that, then I can also revert the VM state changes.  However, I wasn't able to convince myself that this check is not needed.  If we end up returning root_m here as cha_monomorphic_target, it seems possible that it could be a new version of the method, and then target == cha_monomorphic_target would fail.

C1 does call 

2159            dependency_recorder()->assert_evol_method(inline_target);

which in the CHA case would be `cha_monomorphic_target`, not `target`, so it looks like we may not detect if `target` is stale as long as `cha_monomorphic_target` is not.  It seems like a minor loophole, but I'm not sure what kind of problems it could cause, especially if the bytecodes of `target` are not used.

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

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