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

[TobiHartmann]

[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 if 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 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 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:

• 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

---

Progress

• Change must not contain extraneous whitespace

Issue

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

Reviewers

• David Simms (@MrSimms - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/834/head:pull/834
$ git checkout pull/834

Update a local copy of the PR:
$ git checkout pull/834
$ git pull https://git.openjdk.org/valhalla.git pull/834/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 834

View PR using the GUI difftool:
$ git pr show -t 834

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/834.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back thartmann! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@TobiHartmann This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

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

Reviewed-by: dsimms

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the lworld branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the lworld branch, type /integrate in a new comment.

[mlbridge]

Webrevs

• 00: Full (5c2cfd36)

[MrSimms]

Runtime bits look good, thanks for the cleanups along the way

[TobiHartmann]

Thanks for the review, @MrSimms!

[TobiHartmann]

/integrate

[openjdk]

Going to push as commit 6b0631c.
Since your change was applied there has been 1 commit pushed to the lworld branch:

• 27cde0a: 8306301: [lworld] Circular dependency when unpacking

Your commit was automatically rebased without conflicts.

[openjdk]

@TobiHartmann Pushed as commit 6b0631c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.