Re: RFR: 8308248: Revisit alignment of layout constants on 32-bit platforms [v2]

2023-05-17 Thread Paul Sandoz
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]

2023-05-17 Thread Maurizio Cimadamore
> 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=14007=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=14007=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