Re: RFR: 8330388: Remove invokedynamic cache index encoding [v2]
On Thu, 18 Apr 2024 16:22:31 GMT, Matias Saavedra Silva wrote: >> Before [JDK-8307190](https://bugs.openjdk.org/browse/JDK-8307190), >> [JDK-8309673](https://bugs.openjdk.org/browse/JDK-8309673), and >> [JDK-8301995](https://bugs.openjdk.org/browse/JDK-8301995), invokedynamic >> operands needed to be rewritten to encoded values to better distinguish indy >> entries from other cp cache entries. The above changes now distinguish >> between entries with `to_cp_index()` using the bytecode, which is now >> propagated by the callers. >> >> The encoding flips the bits of the index so the encoded index is always >> negative, leading to access errors if there is no matching decode call. >> These calls are removed with some methods adjusted to distinguish between >> indices with the bytecode. Verified with tier 1-5 tests. The changes show no >> issues when tested against libgraal. > > Matias Saavedra Silva has updated the pull request incrementally with one > additional commit since the last revision: > > Dean and Gilles comments Still good! - Marked as reviewed by coleenp (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18819#pullrequestreview-2015844122
Re: RFR: 8330388: Remove invokedynamic cache index encoding [v2]
On Wed, 17 Apr 2024 22:41:08 GMT, Dean Long wrote: >> Matias Saavedra Silva has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Dean and Gilles comments > > src/hotspot/share/ci/ciEnv.cpp line 1513: > >> 1511: // process the BSM >> 1512: int pool_index = indy_info->constant_pool_index(); >> 1513: BootstrapInfo bootstrap_specifier(cp, pool_index, indy_index); > > Why not just change the incoming parameter name to `index`? `indy_index` is used frequently even in functions that are only used in the context of invokedynamic. I think it helps with clarity. - PR Review Comment: https://git.openjdk.org/jdk/pull/18819#discussion_r1571043673
Re: RFR: 8330388: Remove invokedynamic cache index encoding [v2]
> Before [JDK-8307190](https://bugs.openjdk.org/browse/JDK-8307190), > [JDK-8309673](https://bugs.openjdk.org/browse/JDK-8309673), and > [JDK-8301995](https://bugs.openjdk.org/browse/JDK-8301995), invokedynamic > operands needed to be rewritten to encoded values to better distinguish indy > entries from other cp cache entries. The above changes now distinguish > between entries with `to_cp_index()` using the bytecode, which is now > propagated by the callers. > > The encoding flips the bits of the index so the encoded index is always > negative, leading to access errors if there is no matching decode call. These > calls are removed with some methods adjusted to distinguish between indices > with the bytecode. Verified with tier 1-5 tests. Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision: Dean and Gilles comments - Changes: - all: https://git.openjdk.org/jdk/pull/18819/files - new: https://git.openjdk.org/jdk/pull/18819/files/87926aee..3ef92512 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18819=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18819=00-01 Stats: 6 lines in 2 files changed: 0 ins; 1 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/18819.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18819/head:pull/18819 PR: https://git.openjdk.org/jdk/pull/18819
