RFR: 8362832: compiler/macronodes/TestTopInMacroElimination.java hits assert(false) failed: unexpected node [v2]

[Benoît Maillard]

> This PR prevents hitting an assert caused by encountering `top` while following the memory
> slice associated with a field when eliminating allocations in macro node elimination. This situation
> is the result of another elimination (boxing node elimination) that happened at the same
> macro expansion iteration.
> 
> ### Analysis
> 
> The issue appears in the macro expansion phase. We have a nested `synchronized` block,
> with the outer block synchronizing on `new A()` and the inner one on `TestTopInMacroElimination.class`.
> In the inner `synchronized` block we have an `Integer.valueOf` call in a loop.
> 
> In `PhaseMacroExpand::eliminate_boxing_node` we are getting rid of the `Integer.valueOf`
> call, as it is a non-escaping boxing node. After having eliminated the call,
> `PhaseMacroExpand::process_users_of_allocation` takes care of the users of the removed node.
> There, we replace usages of the fallthrough memory projection with `top`.
> 
> In the same macro expansion iteration, we later attempt to get rid of the `new A()` allocation
> in `PhaseMacroExpand::create_scalarized_object_description`. There, we have to make
> sure that all safepoints can still see the object fields as if the allocation was never deleted.
> For this, we attempt to find the last value on the slice of each specific field (`a`
> in this case). Because field `a` is never written to, and it is not explicitely initialized,
> there is no `Store` associated to it and not even a dedicated memory slice (we end up
> taking the `Bot` input on `MergeMem` nodes). By going up the memory chain, we eventually
> encounter the `MemBarReleaseLock` whose input was set to `top`. This is where the assert
> is hit.
> 
> ### Proposed Fix
> 
> In the end I opted for an analog fix as the similar [JDK-8325030](https://git.openjdk.org/jdk/pull/23104).
> If we get `top` from `scan_mem_chain` in `PhaseMacroExpand::value_from_mem`, then we can safely
> return `top` as well. This means that the safepoint will have `top` as data input, but this will
> eventually cleaned up by the next round of IGVN.
> 
> Another (tempting) option would have been to simply return `nullptr` from `PhaseMacroExpand::value_from_mem` when encoutering `top`. However this would result in bailing
> out from eliminating this allocation temporarily and effectively delaying it to a subsqequent
> macro expansion round.
> 
> 
> ### Testing
> - [x] [GitHub Actions](https://github.com/benoitmaillard/jdk/actions?query=branch%3AJDK-8362832)
> - [x] tier1-4, plus some internal testing
> 
> Thank you for reviewing!

Benoît Maillard has updated the pull request incrementally with one additional commit since the last revision:

  Update src/hotspot/share/opto/macro.cpp
  
  Co-authored-by: Daniel Lundén <daniel.lunden at oracle.com>

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/27903/files
  - new: https://git.openjdk.org/jdk/pull/27903/files/c3c92f53..0955e23d

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

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/27903.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/27903/head:pull/27903

PR: https://git.openjdk.org/jdk/pull/27903