RFR: 8337066: Repeated call of StringBuffer.reverse with double byte string returns wrong result

[Vladimir Kozlov]

On Fri, 27 Sep 2024 16:02:29 GMT, Igor Veresov <iveresov at openjdk.org> wrote:

> This is essentially a defensive forward port of a solution for an issue discovered in 11, where a use of pinned load (for which we disable almost all transforms in `LoadNode::Ideal()`) leads to a graph shape that is not expected by `insert_anti_dependencies()`
> 
> The `insert_anti_dependencies()` assumes that the memory input for a load is the root of the memory subgraph that it has to search for the possibly conflicting store. Usually this is true if we run all the memory optimizations but with pinned we don't.
> 
> Compare the good graph shape (with control dependency set to `UnknownControl`):
> <img width="1128" alt="Good graph" src="https://github.com/user-attachments/assets/d2c72d20-bc3c-403f-b340-e7cfe6d1796d">
> 
> With the graph produce with the nodes pinned:
> <img width="1206" alt="Bad graph" src="https://github.com/user-attachments/assets/bbd14b37-584b-4906-973e-93c9046975a6">
> 
> With the "bad graph" loads and store don't share the same memory graph root, and therefore are not considered by `insert_anti_dependencies()`. The solution, I think, could be to walk up the memory chain of the load, skipping MergeMems, in order to get to the real root and then run the precedence edge insertion algorithm from there.

src/hotspot/share/opto/gcm.cpp line 753:

> 751:   // root of our search tree through the corresponding slices of MergeMem nodes to
> 752:   // get to the node that really creates the memory state for this slice.
> 753:   if (load_alias_idx >= Compile::AliasIdxRaw) {

This will be executed for all loads and not only pinned. Is this okay?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21222#discussion_r1778943508