[lworld] Integrated: 8301007: [lworld] Handle mismatches of the preload attribute in the calling convention

[Tobias Hartmann]

On Tue, 18 Apr 2023 11:12:13 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

> [Don't be afraid by the ~4,700 LOC, it's mostly tests 😁]
> 
> C2 passes value class arguments in scalarized form if the calling convention of the resolved method supports this. This requires that the calling conventions of all overriding methods need to agree on which arguments are passed in scalarized form. Since the preload attribute added with [JDK-8281116](https://bugs.openjdk.org/browse/JDK-8281116) does not guarantee any consistency between the overridden and overriding method, we need to handle mismatches at runtime. The same applies to returning value classes in scalarized form.
> 
> [JDK-8301007](https://bugs.openjdk.org/browse/JDK-8301007) discusses several options for handling mismatches. Unfortunately, they are either very complex to implement (mismatch detection in the callee leading to deoptimization and re-execution in the caller) and/or have a significant impact on performance/footprint (global dictionary), sometimes even affecting methods not participating in the mismatch.
> 
> After several iterations of thinking and prototyping, I came up with the following solution.
> 
> **Scalarized calls:**
> The calling convention is determined per method at link time. Mismatches can always be detected at that point by walking up the chain of overridden methods and checking if their calling conventions match for each argument with what we would like to set up for the to-be-linked method. In most of the cases, we can handle a mismatch by simply sticking to the calling convention that the super method uses. I.e., if the super method uses a non-scalarized calling convention because the argument was not yet loaded at link time due to a missing preload attribute, we don't scalarize that argument of the overriding method(s) either. In fact, a fallback to the non-scalarized calling convention is always sufficient because it can't happen that the argument type was loaded when the super method was linked and is not loaded now that an overriding method is linked.
> 
> However, this does not work in the following case of multiple inheritance where `MyValue` was not yet loaded when `I1::m` was linked but was loaded when `I2::m` was linked. Since the interfaces are completely independent, we scalarize the argument for `I2::m` but not for `I1::m`:
> 
> 
> interface I1 {
>   void m(LMyValue arg); // Non-scalarized due to missing preload attribute
> }
> 
> interface I2 {
>   void m(LMyValue arg); // Scalarized due to preload attribute
> }
> 
> class C implements I1, I2 {
>   void m(LMyValue arg) { }
> }
> 
> 
> Now once `C::m` is linked, we can't simply fall back to the non-scalarized calling convention because the method can still be called with a scalarized calling convention through `I2::m`. We mark such "offending" methods, in this case only `I2::m`, in the overriding chain as mismatched, deoptimize all C2 compiled methods that include a scalarized call via that method and re-compile them by using the non-scalarized calling convention. To do that, we register an evol dependency when compiling a scalarized call site (via `C->dependencies()->assert_evol_method(call->method())`) and deoptimize all dependent nmethods via `CodeCache::flush_dependents_on_method` after mismatch detection.
> 
> **Scalarized returns:**
> Due to the lack of adapters on returns, value objects are always (from interpreter, C1 and C2) returned in scalarized form if the return type is a value class. Similar to scalarized calls, an issue occurs when overridden and overriding methods do not agree if the return value should be passed in scalarized form. For example, we might compile a call to a method with an unloaded return type. Later, the return type is loaded and the method (or an overriding method) returns a value in scalarized form that the previously compiled caller can't handle.
> 
> Since the interpreter always checks if a to-be-returned value or a value returned from a call is in scalarized form, we only need additional handling in C1 and C2. For C2, we replace the null assert, that is added after a call site with an unloaded return type, by a full-blown trap. This should have minimal impact and will be further improved (see remaining work section below). For C1, we need additional checks when returning an unloaded type and after invoking a method with an unloaded type. In both cases, the overhead is negligible because it boils down to a bit-check on a register to determine if we need to go to the slow path to handle a scalarized return.
> 
> **Other changes:**
> - Lots of tests relying on jasm to introduce preload attribute mismatches.
> - Extended the `StressCallingConvention` flag to stress test new code paths.
> - Some refactoring/cleanup of related code.
> 
> Since this change became quite large, I'm postponing the following remaining work to [JDK-8284443](https://bugs.openjdk.org/browse/JDK-8284443):
> - A static call to a mismatched method should still be scalarized, we currently fall back to the non-scalarized calling convention even for static calls.
> - If C2 compiles a call with an unloaded return type, a trap is added. We can do better here by adding a null assert and handling scalarized returns similar to the `PhaseMacroExpand::expand_mh_intrinsic_return` logic.
> - Enable `StressCallingConvention` in `InterpreterMacroAssembler::remove_activation`, this still triggers some issues.
> - The calling convention should only be computed once, it's currently re-computed during C1 compilation.
> 
> Thanks,
> Tobias

This pull request has now been integrated.

Changeset: 6b0631cc
Author:    Tobias Hartmann <thartmann at openjdk.org>
URL:       https://git.openjdk.org/valhalla/commit/6b0631ccc0d4aaa05177b643b69c2177b2887c61
Stats:     4751 lines in 30 files changed: 4626 ins; 13 del; 112 mod

8301007: [lworld] Handle mismatches of the preload attribute in the calling convention

Reviewed-by: dsimms

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

PR: https://git.openjdk.org/valhalla/pull/834