RFR: 8320649: C2: Optimize scoped values [v7]

[Roland Westrelin]

On Thu, 8 Feb 2024 14:31:04 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> Ok, this is all for today. I only looked at the first half. Maybe you can react to my comments this far, and then I can continue?

Yes, sounds good. Thanks for the new review.

> Maybe a more general question: How bad would it be if we simply did not inline the "get" call, but directly transformed the graph? Is that feasible? Or would it be a performance hit because we could not inspect the profiling inside "get"? It is just a lot of code here, with lots of potential for bugs.

It's probably feasible. I think we want to avoid the slow path (when there's a missed in the cache) as much as possible because having a call or some complicated logic to handle the cache miss will affect all sort of optimizations. So we would have to speculate and then take a trap if that fails. Given we have profile data in the get(), it seems unfortunate to not use it and have it guide the decisions. The pattern matching is complicated for sure but it's also fairly mechanical. The java code being matches is fairly simple. What can affect pattern matching is whether one of the 3 branches can be never taken. There doesn't seem to be that many ways it could break.

The most fragile part I would say is that the ScopedValueGetHitsInCacheNode/ScopedValueGetLoadFromCacheNode pair have to stay together. If some transformation changes the IR so we can't go from one to the other, then expanding the ScopedValue.get() fails.

> I also wonder about test coverage. How many tests do we have? Fuzzer tests currently do not use ScopedValues. So we would never get any really strange patterns.

That's also a weakness for this. Obviously, it's a new feature so there's not that much code using it. There are some functional tests and benchmarks.

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

PR Comment: https://git.openjdk.org/jdk/pull/16966#issuecomment-1934396712