On Tue, 14 Oct 2025 08:59:48 GMT, Marc Chevalier <[email protected]> wrote:
> When regenerating > `test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestMismatchHandling.jcod`, > the preload attribute are back, after being removed in [8325660: [lworld] > Update C2 to support new value construction scheme from JEP > 401](https://bugs.openjdk.org/browse/JDK-8325660). This change basically > disabled the test > `test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestMismatchHandling.java`. > It is not quite clear why the test broke in between, but it doesn't work now! > It seems there are two problems. > > The symptom is a wrong execution: we get a null pointer exception, when the > pointer is clearly not null. The setup is around a call where the callee > takes a value object as parameter (non-receiver), but the method happens to > be mismatch, as detailed in [8301007: [lworld] Handle mismatches of the > preload attribute in the calling > convention](https://bugs.openjdk.org/browse/JDK-8301007). The caller is > C2-compiled, the callee is interpreted. > > The caller is correctly compiled to pass a pointer to the callee, but the > adapter is expecting a scalar convention, and interpret everything wrong, > leading to the wrong execution. > > First problem is that optimized virtual calls are wrongly expected to never > use the non-scalar convention: > https://github.com/openjdk/valhalla/blob/lworld/src/hotspot/share/runtime/sharedRuntime.cpp#L1374-L1376 > > This fixes the original problems, but create a lot more! Well, just flavor of > the same thing. > > They all come from piggybacking on the `evol_method` dependency that is used > for JVMTI. This have various side effects that makes the code fail assertions > a bit everywhere. Overall, dependencies coming from breakpoints are confused > with some coming from mismatch calling convention, and some functions are > used in both context, but not all. For instance (I might be a blurry on the > details), it happens that a function is marked as having a mismatch calling > convention, but later, some JVMTI related code will read the dependency as > the existence of breakpoints (or something related), and refuse to compile > it, making the test fail with `AbortVMOnCompilationFailure`. Distinguishing > the cases becomes too complicated: while we can probably tell whether we > added the dependency for JVMTI- or convention-related reasons, it is painful > to propagate what we are looking for down the chain of calls. The best, and > simplest, way is to introduce a new kind of dependency for calling convention > mismatch . It mostly behaves live the `evol_method` but it's not the sa... 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. test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestMismatchHandling.jcod line 116: > 114: 0x2AB40007102A9F00; > 115: 0x0DBB000D59120FB7; > 116: 0x0011BFB1; Interesting how `javac` now simplifies it into a "return void". test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestMismatchHandling.jcod line 996: > 994: Attr(#20) { // LoadableDescriptors > 995: 0x00010015; > 996: } // end LoadableDescriptors That was missing. If we read a bit, it means: there is 0x0001 = 1 class to preload. And it is the number 0x0015 = 21, which is `LMyValue1` in the constant pool. Which is pretty much what we expect. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1677#discussion_r2428453083 PR Review Comment: https://git.openjdk.org/valhalla/pull/1677#discussion_r2428433306 PR Review Comment: https://git.openjdk.org/valhalla/pull/1677#discussion_r2428439996
