On Wed, 17 Apr 2024 15:26:52 GMT, Matias Saavedra Silva <matsa...@openjdk.org> 
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.

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`?

src/hotspot/share/classfile/resolutionErrors.hpp line 60:

> 58: 
> 59:   // This function is used to encode an invokedynamic index to 
> differentiate it from a
> 60:   // constant pool index.  It assumes it is being called with a index 
> that is less than 0

Is this comment still correct?

src/hotspot/share/interpreter/bootstrapInfo.cpp line 77:

> 75:     return true;
> 76:   } else if (indy_entry->resolution_failed()) {
> 77:     int encoded_index = 
> ResolutionErrorTable::encode_indy_index(_indy_index);

That's an improvement, from two levels of encoding to only one!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18819#discussion_r1569625123
PR Review Comment: https://git.openjdk.org/jdk/pull/18819#discussion_r1569626519
PR Review Comment: https://git.openjdk.org/jdk/pull/18819#discussion_r1569627219

Reply via email to