JDK-8288683 : C2: And node gets wrong type due to not adding it back to the worklist in CCP
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 18,19,20
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2022-06-17
  • Updated: 2022-07-27
  • Resolved: 2022-06-27
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 19 JDK 20
19 b29Fixed 20Fixed
Related Reports
Blocks :  
Relates :  
Relates :  
Description
JDK-8277850 added some new Value() optimizations in AndI/AndL to optimize pattern similar to "(v << 2) & 3" which can directly be replaced by zero. To do that, we look at the type of the shift value of the LShift input.

This optimization works fine but causes problems in CCP. After calling Value() for a node in CCP, we only add the direct users of it back to the worklist if the type changed. We special case some nodes where we need to add additional nodes back to the worklist to not miss updating them. We should also explicitly add AndI/AndL users of an LShift node that is being put on the worklist. In rare cases, we could wrongly replace and AndI/AndL node by zero in CCP because we miss another Value() call. This can lead to a wrong execution which was found by the attached reduced fuzzer test:

$ java -Xint Test.java > int
$ java -Xbatch Test.java > c2
$ diff int c2
1c1
< result: 35072
---
> result: 0

We could think about adding a new verification phase to CCP to call Value() for each node one more time after we've reached a fixed point. Finding another type change could indicate a bug. This, however, should be investigated separately.
Comments
New test compiler/c2/TestAndShiftZeroCCP.java passed in JDK 19 CI.
11-07-2022

Changeset: 784a0f04 Author: Christian Hagedorn <chagedorn@openjdk.org> Date: 2022-06-27 11:32:13 +0000 URL: https://git.openjdk.org/jdk19/commit/784a0f049665afde4723942e641a10a1d7675f7a
27-06-2022

A pull request was submitted for review. URL: https://git.openjdk.org/jdk19/pull/65 Date: 2022-06-24 08:54:03 +0000
24-06-2022

Thanks for the pointers! Looks like we've thought about these problems before. I think we should definitely think about adding a verification phase (JDK-8257197) to prevent bugs like this one here in the future. In general, most of the missed Value() calls are probably benign but some will lead to wrong executions (like this case here) and are hard to even notice.
20-06-2022

This reminds me of JDK-8273496 and JDK-8257197.
20-06-2022

ILW = Wrong execution due to missing update in CCP, rare and specific to CCP order, no workaround = HLH = P2
17-06-2022