On Thu, 26 Mar 2026 15:03:38 GMT, Chen Liang <[email protected]> wrote:

>> Brings in JDK-8366837 changes and adopt it for lworld.
>> 
>> Tested by examining the generated files with git diff - completely identical.
>> 
>> This patch against mainline is much smaller, and is left here for now: 
>> https://gist.github.com/592a8d8e667f4cf950b934b1d19187d9
>
> Chen Liang has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains five additional commits since 
> the last revision:
> 
>  - Merge branch 'lworld' of https://github.com/openjdk/valhalla into 
> cleanup/lworld-vh-gen
>  - typo
>  - Remove debug output
>  - More debug
>  - Stage

I have one query about the logic for adding in the `NonPlainAccess` value
where I think the new logic is backwards. There's also a query about the
comment in that block.

Reviewing your patch against main-line is a much easier way to see your
proposed Valhalla specific changes.

make/modules/java.base/gensrc/GensrcVarHandles.gmk line 69:

> 67:     # Only plain access for non-atomic storage
> 68:     $1_KEYS += NonPlainAccess
> 69:   endif

Should the `ifeq` be `ifneq` since I think you are testing for the presence of 
NonAtomic stuff here?

The comment on L67 doesn't match the code on L68: `# Only plain access` versus 
`+= NonPlainAccess`.

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

Changes requested by dcubed (Committer).

PR Review: 
https://git.openjdk.org/valhalla/pull/2265#pullrequestreview-4016782742
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2265#discussion_r2997155562

Reply via email to