On Mon, 8 Sep 2025 07:17:11 GMT, Marc Chevalier <[email protected]> wrote:
>> `Unsafe::compareAndSetFlatValue` calls >> `Unsafe::compareAndSetFlatValueAsBytes` which then calls >> `Unsafe::putFlatValue` on a flat array created by `Unsafe::newSpecialArray`. >> >> https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/java.base/share/classes/jdk/internal/misc/Unsafe.java#L2870-L2875 >> >> `putFlatValue` can be intrinsified in >> >> https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/hotspot/share/opto/library_call.cpp#L2727 >> >> that calls `cast_to_flat_array` >> >> https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/hotspot/share/opto/library_call.cpp#L2831 >> >> which fails the assert >> >> https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/hotspot/share/opto/graphKit.cpp#L1875-L1876 >> >> because of >> >> https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/hotspot/share/oops/inlineKlass.cpp#L287-L290 >> >> Of course, if array flattening is disabled, one can't have flat arrays. And >> indeed, `Unsafe::newSpecialArray` will raise if called. So at runtime, the >> call to `Unsafe::compareAndSetFlatValue` should simply raise. But when >> compiled the assert is hit, crashing the VM, because it cannot tell that the >> code is dead, but can check that if it's not the array is not flat. That >> doesn't seem reasonable to me. I propose to insert a trap instead. Even in >> the case where this code wouldn't be dead (like if `Unsafe::newSpecialArray` >> is just undefined behavior instead of throwing), crashing the compiler >> doesn't seem like a good option. >> >> The situation is actually more surprising than it seems: using >> `Unsafe::compareAndSetFlatValue` on a flat field with >> `-XX:-UseArrayFlattening` sounds reasonable, but doesn't actually work since >> the implementation will (attempt to) create flat arrays under the hood. It >> is not clear to me whether it's actually desirable, as >> `Unsafe::compareAndSetFlatValue` introduce some coupling of field and array >> flattening, but on the other hand, it's an unsafe API, so it's not that >> crazy to require more constrains to use. >> >> Thanks, >> Marc > > Marc Chevalier has updated the pull request incrementally with one additional > commit since the last revision: > > Add +UseFieldFlattening Thanks for the detailed analysis Marc. I don't like the coupling between flat arrays and fields in the compareAndSet implementation but that's not something that the JIT can fix :) Looks good to me! ------------- Marked as reviewed by thartmann (Committer). PR Review: https://git.openjdk.org/valhalla/pull/1549#pullrequestreview-3195269182
