On Tue, 2 Dec 2025 22:08:20 GMT, Chen Liang <[email protected]> wrote:
>> I still find it confusing, especially tri-state logic part.
>>
>> For background, `isCompileConstant` was introduced as part of LF sharing
>> effort to get rid of Java-level profiling in optimized code. The pattern is
>> was designed for was:
>>
>> if (isCompileConstant(...)) {
>> return ...;
>> } else {
>> ... // do some extra work (either in interpreter, C1, or
>> not-fully-optimized version in C2)
>> }
>>
>>
>> In this patch, you don't follow that pattern and aadd new state
>> (`CONSTANT_PENDING`) to distinguish interpreter/C1 from C2. What's the
>> motivation? Why do you want to avoid cache updates coming from C2-generated
>> code?
>
> I am assuming that if C2 determines this `vh` is not a constant, we can drop
> it. Is that a right way to move along, or could C2 transition from "not a
> constant" to "is a constant" during the phases?
Sorry, I still don't understand how it is intended to work. Why does
`MethodHandleImpl.isCompileConstant(vh) == true` imply that the cached value is
compatible with the constant `vh`?
// Keep capturing - vh may suddenly get promoted to a constant by C2
Capturing happens outside compiler thread. It is not affected by C2 (except
when it completely prunes the whole block).
So, either any captured adaptation is valid/compatible or there's a concurrency
issue when C2 kicks in and there's a concurrent cache update happening with
incompatible version.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2583346750
