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

[Igor Veresov]

On Fri, 27 Sep 2024 17:32:27 GMT, Vladimir Kozlov <kvn 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?

Normally loads would not have a MergeMem as their memory input at this stage. `Ideal()` splits the memory, so that the memory input of a load is as precise as possible. So I think, that loop is benign for a regular load.

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

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