RFR: 8295486: Inconsistent constant field values observed during compilation

[Vladimir Kozlov]

On Thu, 5 Jan 2023 13:26:11 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

> We hit a "not monotonic" assert because the new type of a load from a stable final field is more narrow than the old type which contradicts the assumption that types should only go from TOP to BOTTOM during CCP:
> 
> old: `narrowoop: java/lang/Integer:BotPTR:exact *`
> new: `narrowoop: java/lang/Integer java.lang.Integer {0x000000062c41e548} ...`
> 
> or 
> 
> old: `narrowoop: java/lang/Integer java.lang.Integer {0x000000062c41e538} ...`
> new: `narrowoop: java/lang/Integer java.lang.Integer {0x000000062c41e548} ...`
> 
> The problem is that a stable field can be (re-)initialized during compilation and since the value is not cached, contradicting types can be observed. In `LoadNode::Value`, we re-read the field value each time: 
> 
> https://github.com/openjdk/jdk/blob/872384707e89d03ede655aad16f360dc94f10152/src/hotspot/share/opto/memnode.cpp#L1994-L1997
> 
> https://github.com/openjdk/jdk/blob/872384707e89d03ede655aad16f360dc94f10152/src/hotspot/share/opto/type.cpp#L332-L337
> 
> The same problem exists for loads from stable arrays:
> https://github.com/openjdk/jdk/blob/872384707e89d03ede655aad16f360dc94f10152/src/hotspot/share/opto/memnode.cpp#L1923
> 
> Caching the field value is not feasible as it would require a cache per ciInstance for all the fields and per ciArray for all the elements. Alternatively, we could keep track of the lookup and only do it once but that would also be lots of additional complexity for a benign issue.
> 
> Instead, I propose to skip verification during CCP when folding loads from stable fields. Non-stable, constant fields are not affected as `null` is a valid value for them and they would already be folded before CCP.
> 
> Thanks,
> Tobias

@vlivanov Do we  have a mechanism to verify that `Stable` fields are initialized only once? Based on Tobias's test (which continuously modifies values) it does not work if it even exists. Then what is definition of `Stable` attribute then? We can't constant fold a field's value which can be changed (except from NULL to a value).

Tobias's comment in JBS about `field point to different DirectMethodHandles` concerns me.

For me the only valid solution is throw out compilation if it see change in stable filed's state. We have validation step in `ciEnv::register_method()`. May be create table of stable fields values we folded and verify during registration to make sure it did not change. It may not catch case in Tobias's test where the same values are rotated.

Note, **one** change from  null to not-null is valid case. May be new code in `PhaseCCP::verify_type()` should check for initial NULL value before skipping following checks.

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

PR: https://git.openjdk.org/jdk/pull/11861