RFR: 8261137: Optimization of Box nodes in uncommon_trap [v11]

[Vladimir Ivanov]

On Wed, 24 Mar 2021 16:41:30 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Wang Huang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   refact SafePointScalarObjectNode constructor
>
> src/hotspot/share/opto/callnode.cpp line 1533:
> 
>> 1531: #ifdef ASSERT
>> 1532:   if (alloc != nullptr) {
>> 1533:     assert(alloc->is_Allocate() || alloc->as_CallStaticJava()->is_boxing_method(), "unexpected alloc");
> 
> Add dumping of `alloc` **before** this assert to know it during failure without rerunning test:
> if (!alloc->is_Allocate() && !alloc->as_CallStaticJava()->is_boxing_method()) {
>   alloc->dump();
> }

> It was my suggestion to pass call node as allocation so that we could trace back for what node SafePointScalarObject was created because you may have several Box objects for which we create SafePointScalarObject nodes.

The problem with that is `alloc` quickly becomes stale since allocations/calls are removed shortly after scalarization takes place. The only usage of `alloc()` is in `PhaseMacroExpand::scalar_replacement`:
assert(scobj->alloc() == alloc, "sanity");

Regarding the assert, frankly speaking, it looks repetitive and verbose. I'd prefer to just see it folded into:
assert(alloc == NULL || alloc->is_Allocate() || alloc->as_CallStaticJava()->is_boxing_method(), "unexpected call node");

Or introduce a helper method to dump relevant info about the problematic node: 
assert(alloc == NULL || alloc->is_Allocate() || alloc->as_CallStaticJava()->is_boxing_method(), "unexpected call node: %s", dump_node(alloc));

Also, `alloc == NULL` case can be eliminated by avoiding passing `NULL` in `PhaseVector::scalarize_vbox_node()`.

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

PR: https://git.openjdk.java.net/jdk/pull/2401