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

[merykitty]

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.

---

Progress

• Change must not contain extraneous whitespace

Issue

• JDK-8335256: [lworld] compiler/valhalla/inlinetypes/TestValueConstruction.java fails intermittently (Bug - P3)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1358

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back qamai! 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]

@merykitty This change is no longer ready for integration - check the PR body for details.

[mlbridge]

Webrevs

• 02: Full - Incremental (49991764)
• 01: Full - Incremental (d4b28aae)
• 00: Full (7b69cccd)

[jatin-bhateja]

The implicit larval transition has a limited life span, it begins from the new allocation and ends just before exiting constructor.

valhalla/src/hotspot/share/opto/doCall.cpp

Line 830 in d94c589

cloned_updated_receiver->set_is_larval(false);

The only possibility to create a larval value that gets passed on to OSR buffer is by explicitly marking it as larval using Unsafe.makePrivateBuffer API before the loop.

May I request you to kindly add a new or identify an existing test case for this scenario

[TobiHartmann]

The only possibility to create a larval value that gets passed on to OSR buffer is by explicitly marking it as larval using Unsafe.makePrivateBuffer API before the loop.

Why is that? We can have a loop triggering OSR within that limited life span, right?