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

[Vladimir Kozlov]

On Wed, 31 Mar 2021 02:29:33 GMT, Wang Huang <whuang at openjdk.org> wrote:

>>> 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.
> 
> Should we implement `dump_node` or should we just use `assert(alloc == NULL || alloc->is_Allocate() || alloc->as_CallStaticJava()->is_boxing_method(), "unexpected call node");`  without dumping?

Creating node's dump string in separate method is complicated.
We can just use `false` in assert:
#ifdef ASSERT
  if (alloc != nullptr && !alloc->is_Allocate() && !alloc->as_CallStaticJava()->is_boxing_method()) {
    alloc->dump();
    assert(false, "unexpected call node for scalar replacement");
  }
#endif

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

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