[lworld] RFR: 8369185: [lworld] Wrong execution in TestMismatchHandling after regenerating TestMismatchHandling.jcod [v2]

Marc Chevalier mchevalier at openjdk.org
Wed Oct 15 13:08:19 UTC 2025


On Wed, 15 Oct 2025 06:56:53 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> src/hotspot/share/code/nmethod.hpp line 1066:
>> 
>>> 1064:   // Used for fast breakpoint support if only_calling_convention is false;
>>> 1065:   // used for updating the calling convention if true.
>>> 1066:   bool is_dependent_on_method(Method* dependee, bool only_calling_convention);
>> 
>> I'm not really happy about this `bool only_calling_convention`. I'd rather like a `Dependencies::DepType` instead because it is only used in
>> 
>> Dependencies::DepType dep_type = only_calling_convention ? Dependencies::mismatch_calling_convention : Dependencies::evol_method;
>> 
>> 
>> The problem is that then I get a cyclic include between `nmethod.hpp` and `dependencies.hpp`. It's probably avoidable, but I need to refactor a bit too intensely than I feel comfortable in such a small fix.
>
> I guess an alternative would be to add separate methods, similar to what we have for `CodeCache::mark_dependents_on_method_for_breakpoint` -> `CodeCache::mark_dependents_on_method_for_mismatch` or something. That would at least limit the `only_calling_convention` arg to `is_dependent_on_method`.
> 
> Or what about always checking both dependencies in `nmethod::is_dependent_on_method`? After all, both dependencies represent an actual dependency:
> - If the nmethod has a `evol_method` dependency, it's supposed to be deopted anyway. It doesn't matter where this happens.
> - If the nmethod has a `mismatch_calling_convention` dependency, in the worst case we deopt it when we reach this code via `CodeCache::mark_dependents_on_method_for_breakpoint` or `WB_DeoptimizeMethod`, i.e. when we want to make sure that all compiled versions (via inlining) of a method are deopted. So we would unnecessarily deopt the caller of a mismatched method when we set a breakpoint in that mismatched method (or deopt it via the WB API). I think that's fine.

I started with separate methods, but that was unfortunate duplication. I went for the second option, since testing seems happy. I'm not sure about the new names I introduced tho (`method_types` and `has_method_dep`). If you have better idea, I'll be happy to change them.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1677#discussion_r2432489457


More information about the valhalla-dev mailing list