Bug ID: JDK-8290910 Wrong memory state is picked in SuperWord::co_locate

JDK-8290910 : Wrong memory state is picked in SuperWord::co_locate_pack()

Type: Bug
Component: hotspot
Sub-Component: compiler
Affected Version: 20

Priority: P3
Status: Resolved
Resolution: Fixed
OS: linux
CPU: x86_64

Submitted: 2022-07-23
Updated: 2022-10-17
Resolved: 2022-09-23

Versions (Unresolved/Resolved/Fixed)

The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.

JDK 20
20 b17Fixed

Related Reports

Duplicate :	JDK-8293216 - C2: SLP miscompilation
Duplicate :	JDK-8291054 - applications/javafuzzer/MediumTest.java fails "r2- 50" with 2 failures
Relates :	JDK-8283091 - Support type conversion between different data sizes in SLP

Description

The following test failed in the JDK20 CI:

applications/javafuzzer/BigTest.java

Here's a snippet from the log file:

----------System.out:(36/2680)----------
Using JRuby executable: /opt/mach5/mesos/work_dir/jib-master/install/org/jruby/jruby-dist/9.2.12.0/jruby-dist-9.2.12.0-bin.zip/jruby-9.2.12.0/bin/jruby
For random generator using seed: 2738850881927052418
To re-run test with same seed value please add "-Djdk.test.lib.random.seed=2738850881927052418" to command line.
Starting JavaFuzzer: '/bin/bash /opt/mach5/mesos/work_dir/jib-master/install/com/oracle/jpg/bigapps/javafuzzer/javafuzzer/1.0/javafuzzer-1.0.zip/mrt.sh -R /opt/mach5/mesos/work_dir/slaves/0c72054a-24ab-4dbb-944f-97f9341a1b96-S45783/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/d852ccfd-f943-484a-a98a-a27ec0e23007/runs/e5bf1a13-986b-4657-8197-10a42c357763/testoutput/test-support/jtreg_closed_test_hotspot_jtreg_applications_javafuzzer_BigTest_java/scratch/0 -NT 300 -NP 12 -A -conf config.yml'
[2022-07-22T22:09:57.905914816Z] Gathering output for process 7686
[2022-07-23T07:20:56.583665013Z] Waiting for completion for process 7686
[2022-07-23T07:20:56.583810329Z] Waiting for completion finished for process 7686
Output and diagnostic info for process 7686 was saved into 'pid-7686-output.log'

Summary of the JavaFuzzer run:
------------------------------
Host:     ol8-x64-641327
Tests:    12 x 300
Args:     -conf config.yml

Started  at: Fri Jul 22 22:09:58 UTC 2022


r6- 300: 177 passed, 0 crashes, 2 fails, 0 hangs, 0 incorrect tests, 122 Reference Java failures
r8- 300: 189 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 111 Reference Java failures
r3- 300: 179 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 121 Reference Java failures
r5- 300: 187 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 113 Reference Java failures
r12- 300: 178 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 122 Reference Java failures
r4- 300: 177 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 123 Reference Java failures
r11- 300: 177 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 123 Reference Java failures
r1- 300: 178 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 122 Reference Java failures
r7- 300: 180 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 120 Reference Java failures
r9- 300: 182 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 118 Reference Java failures
r2- 300: 184 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 116 Reference Java failures
r10- 300: 173 passed, 0 crashes, 0 fails, 0 hangs, 0 incorrect tests, 127 Reference Java failures

Finished at: Sat Jul 23 07:20:56 UTC 2022


[2022-07-23T07:20:56.606831515Z] Waiting for completion for process 7686
[2022-07-23T07:20:56.606973042Z] Waiting for completion finished for process 7686
----------System.err:(13/728)----------
java.lang.RuntimeException: assertEquals: expected 1 to equal 2
	at jdk.test.lib.Asserts.fail(Asserts.java:594)
	at jdk.test.lib.Asserts.assertEquals(Asserts.java:205)
	at jdk.test.lib.Asserts.assertEquals(Asserts.java:189)
	at applications.javafuzzer.JavaFuzzerRunner.main(JavaFuzzerRunner.java:235)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:312)
	at java.base/java.lang.Thread.run(Thread.java:1589)

JavaTest Message: Test threw exception: java.lang.RuntimeException
JavaTest Message: shutting down test

result: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: assertEquals: expected 1 to equal 2


I downloaded the test run's artifacts and there are no hs_err_pid files.
Taking a closer look at the above:

r6- 300: 177 passed, 0 crashes, 2 fails, 0 hangs, 0 incorrect tests, 122 Reference Java failures

so we have "2 fails", but they didn't result in crashes.

Comments

Changeset: a4dc035a Author: Fei Gao <fgao@openjdk.org> Committer: Ningsheng Jian <njian@openjdk.org> Date: 2022-09-23 01:26:21 +0000 URL: https://git.openjdk.org/jdk/commit/a4dc035a9731a32083bbd3fa28408bfaa3474b54
23-09-2022
A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/9898 Date: 2022-08-17 07:48:47 +0000
17-08-2022
To fix it, I think we can add an additional check here [1], to check if the store is in a pack. If the store is in a pack, we still take the memory state of the last load rather than the first load. The reason is, as designed[2], to schedule the store pack, all memory operations in another single pack should be moved in the same direction. We know that the store in the pack depends on one of loads in the load pack, so, whichever we pick, the memory state of the first load or the last load, the load pack should be scheduled before the store pack. And the load pack depends on the sandwiched last_mem so the last_mem must be scheduled before the load pack and also before the store pack. Therefore, we need to take the memory state of the last load for the load pack under the situation. [1] https://github.com/openjdk/jdk/blob/26e5c112daa30697a42047e78744c1c533611e10/src/hotspot/share/opto/superword.cpp#L2373 [2] https://github.com/openjdk/jdk/blob/26e5c112daa30697a42047e78744c1c533611e10/src/hotspot/share/opto/superword.cpp#L2233
04-08-2022
After JDK-8283091, the loop below can be vectorized partially. Statement 1 can be vectorized but statement 2 can't. ``` // int[] iArr; long[] lArrFld; int i1,i2; for (i1 = 6; i1 < 227; i1++) { iArr[i1] += lArrFld[i1-1]++; // statement 1 iArr[i1 + 1] -= (i2++); // statement 2 } ``` When scheduling packs in SuperWord::co_locate_pack() for statement 1, SLP visits them in the order: StoreL->LoadL->LoadI->StoreI. When finding the memory state of the LoadI pack of iArr, because of the dependency between LoadI pack and unscheduled StoreI pack, SLP goes into the branch picking the memory state of first load[1]. Then, SLP visits the StoreI pack and finds that last_mem (the memory state of last load) should be scheduled before the StoreI pack. In this way, the vector packs of iArr are scheduled incorrectly like: ``` load_vector iArr in statement 1 unvectorized loads/stores in statement 2 store_vector iArr in statement 1 ``` We cannot pick the memory state from the first load for LoadI pack here, as the LoadI vector operation must load the new values in memory after iArr writes 'iArr[i1 + 1] - (i2++)' to 'iArr[i1 + 1]'(statement 2). We must take the memory state of the last load where we have assigned new values ('iArr[i1 + 1] - (i2++)') to the iArr array. [1]https://github.com/openjdk/jdk/blob/0ae834105740f7cf73fe96be22e0f564ad29b18d/src/hotspot/share/opto/superword.cpp#L2380
04-08-2022
Thanks for your report and testcase[~thartmann]. I reproduced the error on X86. I'll investigate the root cause ASAP.
26-07-2022
This is a regression from JDK-8283091. [~fgao] could you please have a look?
25-07-2022
ILW = Incorrect execution of compiled code (regression in JDK 20 b3), with generated test and -Xcomp, no workaround but disable C2 compilation of affected method = HLM = P3
25-07-2022
To reproduce, simply run the attached Test.java with -Xcomp and -Xint and compare the output. You can use creduce (https://embed.cs.utah.edu/creduce/) to simplify the test.
25-07-2022