[lworld] RFR: 8335256: [lworld] compiler/valhalla/inlinetypes/TestValueConstruction.java fails intermittently [v3]
Quan Anh Mai
qamai at openjdk.org
Tue Feb 11 15:51:04 UTC 2025
> Hi,
>
> The assert fires because in this place:
>
> if (is_osr_parse()) {
> Node* osr_buf = entry_map->in(TypeFunc::Parms+0);
> entry_map->set_req(TypeFunc::Parms+0, top());
> set_map(entry_map);
> load_interpreter_state(osr_buf);
> } else {
> set_map(entry_map);
> do_method_entry();
> }
>
> `load_interpreter_state` assumes that all incoming inline types are non-larval, while in the following place:
>
> if (t->is_inlinetypeptr()) {
> // Create InlineTypeNode from the oop and replace the parameter
> bool is_larval = (i == 0) && method()->is_object_constructor() && !method()->holder()->is_java_lang_Object();
> Node* vt = InlineTypeNode::make_from_oop(this, parm, t->inline_klass(), !t->maybe_null(), is_larval);
> replace_in_map(parm, vt);
> }
>
> `is_larval == true` because we are osr-compiling the constructor of a value class. As a result, we pass a non-larval object into `InlineTypeNode::make_from_oop` and want to make it larval, triggering the assert.
>
> Thinking more deeply into this, I think there is no way we can know the larval-ness of an incoming object in an osr-compilation unless `ciTypeFlow` does so. For example, consider this bytecode sequence:
>
> ValueObject x = new ValueObject;
> for {
> }
> x.<init>();
>
> Then, when we osr-compile the method from the loop, `x` will be a larval object and it can be at any place on the JVM stack or in the local slots.
>
> As a result, I propose optimistically assuming all osr parameters be non-larval, then we can change them to larval when encountering a constructor invocation or a field store instruction. The verifier should ensure that we cannot do anything else on a larval object as well as do these actions on a non-larval object, so we should recognize the correct state of the value object if any operation is performed on them. Otherwise, they should be dead and not apear here.
>
> Please take a look and give your advices, thanks a lot.
Quan Anh Mai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
- Merge branch 'lworld' into osrinlinetype
- typo
- Fix osr inline type parameter
-------------
Changes:
- all: https://git.openjdk.org/valhalla/pull/1358/files
- new: https://git.openjdk.org/valhalla/pull/1358/files/d4b28aae..49991764
Webrevs:
- full: https://webrevs.openjdk.org/?repo=valhalla&pr=1358&range=02
- incr: https://webrevs.openjdk.org/?repo=valhalla&pr=1358&range=01-02
Stats: 40002 lines in 2969 files changed: 17505 ins; 12109 del; 10388 mod
Patch: https://git.openjdk.org/valhalla/pull/1358.diff
Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1358/head:pull/1358
PR: https://git.openjdk.org/valhalla/pull/1358
More information about the valhalla-dev
mailing list