> This PR fixes an issue that has crept into the FFM API implementation. > > From very early stages, the FFM API used to disable the alignment check on > nested layout elements, in favor of an alignment check against the memory > segment base address. The rationale was that the JIT had issue with > eliminating redundant alignment checks, and accessing nested elements could > never result in alignment issues, since the alignment of the container is > provably bigger than that of the contained element. This means that, when > creating a var handle for a nested layout element, we set the nested layout > alignment to 1 (unaligned), derive its var handle, and then decorate the var > handle with an alignment check for the container. > > At some point in 22, we tweaked the API to throw > `UnsupportedOperationException` when using an access mode incompatible with > the alignment constraint of the accessed layout. That is, a volatile read on > an `int` is only possible if the access occurs at an address that is at least > 4-byte aligned. Otherwise an `UOE` is thrown. > > Unfortunately this change broke the aforementioned optimization: creating a > var handle for an unaligned layout works, but the resulting layout will *not* > support any of the atomic access modes. > > Since this optimization is not really required anymore (proper C2 support to > hoist/eliminate alignment checks has been added since then), it is better to > disable this implementation quirk, and leave optimizations to C2. > > (If we really really wanted to optimize things a bit, we could remove the > container alignment check in the case the accessed value is the first layout > element nested in the container, but this PR doesn't go that far). > > I've run relevant benchmarks before/after and found no differences. In part > this is because `arrayElementVarHandle` is unaffected. But, even after > tweaking the `LoopOverNonConstant` benchmark to add explicit tests for the > code path affected, no significant difference was found, sign that C2 is > indeed able to spot (and remove) the redundant alignment check. Note: if we > know that `aligned_to_N(base)` holds, then it's easy to prove that > `aligned_to_M(base + offset + index * scale)` holds, when `N >= M` and with > `offset` and `scale` known (the latter a power of two).
Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Add benchmark anno ------------- Changes: - all: https://git.openjdk.org/jdk/pull/19124/files - new: https://git.openjdk.org/jdk/pull/19124/files/829c5e28..40ba693c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19124&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19124&range=01-02 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19124.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19124/head:pull/19124 PR: https://git.openjdk.org/jdk/pull/19124
