[15] RFR(S): 8238438: SuperWord::co_locate_pack picks memory state of first instead of last load

[Christian Hagedorn]

Hi

Please review the following patch:
https://bugs.openjdk.java.net/browse/JDK-8238438
http://cr.openjdk.java.net/~chagedorn/8238438/webrev.00/

When processing a load pack in SuperWord::co_locate_pack(), we pick by 
default the memory state of the last load. But if we find a store that 
is dependent on an earlier load in the pack, then we need to pick the 
memory state of the first load.

The current code, however, checks for each load 'l' if it has a 
dependency on an earlier store instead of checking if a later store is 
dependent on 'l' (regression since [1]). At [2], we start at the memory 
state of the current load and walk the memory graph to the memory state 
of the first load while checking dependency constraints. This wrongly 
checks for store->load instead of load->store dependencies. For example, 
in the test case [3], the load pack consists of LoadI 656, 652,... with 
its memory states 657 StoreI (first_mem) and 653 StoreI, respectively. 
When processing the second load in the pack, 652 LoadI, we go up the 
memory graph to first_mem (657 StoreI). The dependence graph now tells 
us that 652 LoadI is dependent on 657 StoreI. From that we make the 
wrong conclusion that we need to pick the memory state of the first load 
which results in a wrong execution for that test case.

The fix now first finds the last memory state. Afterwards, it walks the 
memory graph from the last memory state to the memory state of each load 
in the pack (but not beyond) while checking for any load->store 
dependency constraint (as done before [1]).

Thank you!

Best regards,
Christian


[1] https://bugs.openjdk.java.net/browse/JDK-8233032, 
https://hg.openjdk.java.net/jdk/jdk/rev/3b693618d084
[2] 
http://hg.openjdk.java.net/jdk/jdk/file/27e87c000b16/src/hotspot/share/opto/superword.cpp#l2306
[3] Part of the IR of the test case: 
https://bugs.openjdk.java.net/secure/attachment/86850/load_pack_memory.png