[lworld] RFR: 8335256: [lworld] compiler/valhalla/inlinetypes/TestValueConstruction.java fails intermittently [v2]

[Quan Anh Mai]

> 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 incrementally with one additional commit since the last revision:

  typo

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

Changes:
  - all: https://git.openjdk.org/valhalla/pull/1358/files
  - new: https://git.openjdk.org/valhalla/pull/1358/files/7b69cccd..d4b28aae

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=valhalla&pr=1358&range=01
 - incr: https://webrevs.openjdk.org/?repo=valhalla&pr=1358&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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