Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms [v5]
On Fri, 19 May 2023 10:06:53 GMT, Maurizio Cimadamore wrote: >> The FFM API exposes layout constants for Java primitives. Among those there >> are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of >> these layouts is set the same as their size (e.g. 8 bytes). >> >> This is obviously correct on 64-bit platforms, but on 32-bit platform it is >> not, as such platforms cannot guarantee that doubles and longs will be >> always 64-bit aligned. This will also result in problems when trying to use >> e.g. `JAVA_DOUBLE` to model a C double for the linker API on 32-bit >> platforms. >> >> For these reasons, it would be preferable to define the alignment of >> `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`. >> >> This patch rectifies alignment of those layout constants to reflect >> platform-dependent constraints. It also fixes the maximum alignment >> constraint supported by heap segments, so that it is 4 for long[] and >> double[] on 32-bit platforms. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests for x86 Marked as reviewed by pminborg (Committer). - PR Review: https://git.openjdk.org/jdk/pull/14007#pullrequestreview-1434264181
Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms [v5]
> The FFM API exposes layout constants for Java primitives. Among those there > are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of > these layouts is set the same as their size (e.g. 8 bytes). > > This is obviously correct on 64-bit platforms, but on 32-bit platform it is > not, as such platforms cannot guarantee that doubles and longs will be always > 64-bit aligned. This will also result in problems when trying to use e.g. > `JAVA_DOUBLE` to model a C double for the linker API on 32-bit platforms. > > For these reasons, it would be preferable to define the alignment of > `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`. > > This patch rectifies alignment of those layout constants to reflect > platform-dependent constraints. It also fixes the maximum alignment > constraint supported by heap segments, so that it is 4 for long[] and > double[] on 32-bit platforms. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Fix tests for x86 - Changes: - all: https://git.openjdk.org/jdk/pull/14007/files - new: https://git.openjdk.org/jdk/pull/14007/files/19ae22a1..53db756e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14007&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14007&range=03-04 Stats: 29 lines in 2 files changed: 11 ins; 7 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/14007.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14007/head:pull/14007 PR: https://git.openjdk.org/jdk/pull/14007
Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms [v4]
> The FFM API exposes layout constants for Java primitives. Among those there > are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of > these layouts is set the same as their size (e.g. 8 bytes). > > This is obviously correct on 64-bit platforms, but on 32-bit platform it is > not, as such platforms cannot guarantee that doubles and longs will be always > 64-bit aligned. This will also result in problems when trying to use e.g. > `JAVA_DOUBLE` to model a C double for the linker API on 32-bit platforms. > > For these reasons, it would be preferable to define the alignment of > `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`. > > This patch rectifies alignment of those layout constants to reflect > platform-dependent constraints. It also fixes the maximum alignment > constraint supported by heap segments, so that it is 4 for long[] and > double[] on 32-bit platforms. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Add simple sanity check - Changes: - all: https://git.openjdk.org/jdk/pull/14007/files - new: https://git.openjdk.org/jdk/pull/14007/files/0cb7d5a9..19ae22a1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14007&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14007&range=02-03 Stats: 7 lines in 1 file changed: 7 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14007.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14007/head:pull/14007 PR: https://git.openjdk.org/jdk/pull/14007
Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms [v3]
On Thu, 18 May 2023 09:33:00 GMT, Maurizio Cimadamore wrote: >> The FFM API exposes layout constants for Java primitives. Among those there >> are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of >> these layouts is set the same as their size (e.g. 8 bytes). >> >> This is obviously correct on 64-bit platforms, but on 32-bit platform it is >> not, as such platforms cannot guarantee that doubles and longs will be >> always 64-bit aligned. This will also result in problems when trying to use >> e.g. `JAVA_DOUBLE` to model a C double for the linker API on 32-bit >> platforms. >> >> For these reasons, it would be preferable to define the alignment of >> `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`. >> >> This patch rectifies alignment of those layout constants to reflect >> platform-dependent constraints. It also fixes the maximum alignment >> constraint supported by heap segments, so that it is 4 for long[] and >> double[] on 32-bit platforms. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments > Actually tweak alignment of ValueLayout.OfLong/ValueLayout.OfDouble Does it make sense to add two test classes, each running on a 64/32-bit platform that verifies the two alignment variants? - PR Comment: https://git.openjdk.org/jdk/pull/14007#issuecomment-1552828503
Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms [v3]
On Thu, 18 May 2023 09:28:14 GMT, Maurizio Cimadamore
wrote:
>> The FFM API exposes layout constants for Java primitives. Among those there
>> are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of
>> these layouts is set the same as their size (e.g. 8 bytes).
>>
>> This is obviously correct on 64-bit platforms, but on 32-bit platform it is
>> not, as such platforms cannot guarantee that doubles and longs will be
>> always 64-bit aligned. This will also result in problems when trying to use
>> e.g. `JAVA_DOUBLE` to model a C double for the linker API on 32-bit
>> platforms.
>>
>> For these reasons, it would be preferable to define the alignment of
>> `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`.
>>
>> This patch rectifies alignment of those layout constants to reflect
>> platform-dependent constraints. It also fixes the maximum alignment
>> constraint supported by heap segments, so that it is 4 for long[] and
>> double[] on 32-bit platforms.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Address review comments
> Actually tweak alignment of ValueLayout.OfLong/ValueLayout.OfDouble
src/java.base/share/classes/jdk/internal/foreign/layout/ValueLayouts.java line
298:
> 296:
> 297: public static OfLong of(ByteOrder order) {
> 298: return new OfLongImpl(order, ADDRESS_SIZE_BITS,
> Optional.empty());
Whoops - I forgot these all-important changes :-)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14007#discussion_r1197604492
Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms [v3]
> The FFM API exposes layout constants for Java primitives. Among those there > are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of > these layouts is set the same as their size (e.g. 8 bytes). > > This is obviously correct on 64-bit platforms, but on 32-bit platform it is > not, as such platforms cannot guarantee that doubles and longs will be always > 64-bit aligned. This will also result in problems when trying to use e.g. > `JAVA_DOUBLE` to model a C double for the linker API on 32-bit platforms. > > For these reasons, it would be preferable to define the alignment of > `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`. > > This patch rectifies alignment of those layout constants to reflect > platform-dependent constraints. It also fixes the maximum alignment > constraint supported by heap segments, so that it is 4 for long[] and > double[] on 32-bit platforms. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address review comments Actually tweak alignment of ValueLayout.OfLong/ValueLayout.OfDouble - Changes: - all: https://git.openjdk.org/jdk/pull/14007/files - new: https://git.openjdk.org/jdk/pull/14007/files/731095e9..0cb7d5a9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14007&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14007&range=01-02 Stats: 6 lines in 2 files changed: 2 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/14007.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14007/head:pull/14007 PR: https://git.openjdk.org/jdk/pull/14007
Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms [v2]
On Wed, 17 May 2023 11:15:11 GMT, Maurizio Cimadamore
wrote:
>> The FFM API exposes layout constants for Java primitives. Among those there
>> are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of
>> these layouts is set the same as their size (e.g. 8 bytes).
>>
>> This is obviously correct on 64-bit platforms, but on 32-bit platform it is
>> not, as such platforms cannot guarantee that doubles and longs will be
>> always 64-bit aligned. This will also result in problems when trying to use
>> e.g. `JAVA_DOUBLE` to model a C double for the linker API on 32-bit
>> platforms.
>>
>> For these reasons, it would be preferable to define the alignment of
>> `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`.
>>
>> This patch rectifies alignment of those layout constants to reflect
>> platform-dependent constraints. It also fixes the maximum alignment
>> constraint supported by heap segments, so that it is 4 for long[] and
>> double[] on 32-bit platforms.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix max alignment constants
Marked as reviewed by psandoz (Reviewer).
src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 488:
> 486: /**
> 487: * A value layout constant whose size is the same as that of a Java
> {@code long},
> 488: * bit alignment set to {@code ADDRESS.bitSize()}, and byte order
> set to {@link ByteOrder#nativeOrder()}.
Suggestion:
* (platform dependent) bit alignment set to {@code ADDRESS.bitSize()}, and
byte order set to {@link ByteOrder#nativeOrder()}.
?
-
PR Review: https://git.openjdk.org/jdk/pull/14007#pullrequestreview-1431604601
PR Review Comment: https://git.openjdk.org/jdk/pull/14007#discussion_r1197038551
Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms [v2]
> The FFM API exposes layout constants for Java primitives. Among those there > are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of > these layouts is set the same as their size (e.g. 8 bytes). > > This is obviously correct on 64-bit platforms, but on 32-bit platform it is > not, as such platforms cannot guarantee that doubles and longs will be always > 64-bit aligned. This will also result in problems when trying to use e.g. > `JAVA_DOUBLE` to model a C double for the linker API on 32-bit platforms. > > For these reasons, it would be preferable to define the alignment of > `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`. > > This patch rectifies alignment of those layout constants to reflect > platform-dependent constraints. It also fixes the maximum alignment > constraint supported by heap segments, so that it is 4 for long[] and > double[] on 32-bit platforms. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Fix max alignment constants - Changes: - all: https://git.openjdk.org/jdk/pull/14007/files - new: https://git.openjdk.org/jdk/pull/14007/files/b003bd6a..731095e9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14007&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14007&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/14007.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14007/head:pull/14007 PR: https://git.openjdk.org/jdk/pull/14007
Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms
On Tue, 16 May 2023 11:18:09 GMT, Maurizio Cimadamore wrote: > The FFM API exposes layout constants for Java primitives. Among those there > are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of > these layouts is set the same as their size (e.g. 8 bytes). > > This is obviously correct on 64-bit platforms, but on 32-bit platform it is > not, as such platforms cannot guarantee that doubles and longs will be always > 64-bit aligned. This will also result in problems when trying to use e.g. > `JAVA_DOUBLE` to model a C double for the linker API on 32-bit platforms. > > For these reasons, it would be preferable to define the alignment of > `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`. > > This patch rectifies alignment of those layout constants to reflect > platform-dependent constraints. It also fixes the maximum alignment > constraint supported by heap segments, so that it is 4 for long[] and > double[] on 32-bit platforms. Javadoc: https://cr.openjdk.org/~mcimadamore/jdk/8308248/8308248/v1/javadoc/java.base/module-summary.html Specdiff: https://cr.openjdk.org/~mcimadamore/jdk/8308248/8308248/v1/specdiff_out/overview-summary.html - PR Comment: https://git.openjdk.org/jdk/pull/14007#issuecomment-1551095329
RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms
The FFM API exposes layout constants for Java primitives. Among those there are constants for `JAVA_LONG` and `JAVA_DOUBLE`. Currently, the alignment of these layouts is set the same as their size (e.g. 8 bytes). This is obviously correct on 64-bit platforms, but on 32-bit platform it is not, as such platforms cannot guarantee that doubles and longs will be always 64-bit aligned. This will also result in problems when trying to use e.g. `JAVA_DOUBLE` to model a C double for the linker API on 32-bit platforms. For these reasons, it would be preferable to define the alignment of `JAVA_LONG` and `JAVA_DOUBLE` constants as `ADDRESS.byteSize()`. This patch rectifies alignment of those layout constants to reflect platform-dependent constraints. It also fixes the maximum alignment constraint supported by heap segments, so that it is 4 for long[] and double[] on 32-bit platforms. - Commit messages: - Add more comprehensive javadoc - Initial push Changes: https://git.openjdk.org/jdk/pull/14007/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14007&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8308248 Stats: 42 lines in 3 files changed: 16 ins; 6 del; 20 mod Patch: https://git.openjdk.org/jdk/pull/14007.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14007/head:pull/14007 PR: https://git.openjdk.org/jdk/pull/14007
