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

[Vladimir Kozlov]

On Wed, 24 Mar 2021 22:01:23 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> 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()`.

Okay. Lets do as @iwanowww suggesting.

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

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