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

[Emanuel Peter]

On Mon, 6 May 2024 12:03:12 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>>> I'm thinking in particular about your most recent changes with:
>>> 
>>>     * `class Invariance`
>>> 
>>>     * `estimate_if_peeling_possible`
>>> 
>>> 
>>> Don't get me wrong: I like those refactorings, but they should be done separately.
>> 
>> The problem I see is that they have little value unless this patch is integrated as it is. What if another reviewer thinks it's better to keep everything related to loop predication together? There's no need to change the class `Invariance` then.
>
>> @rwestrel one idea to split things here:
>> 
>>     * Early inline of ScopedValue methods
>> 
>>     * Parsing to IR nodes and expansion back.
>> 
>>     * Optimization
>> 
>>     * Tests
>> 
>> 
>> This way, I can spend only a few hours on one at a time, and we can get this done. Of course, you cannot really integrate an individual one, maybe there is a way to use skara for dependent PR's?
> 
> Would one commit per line above work? Or do you think it needs to be different PRs?

@rwestrel
Just using commits is probably not really helpful. What would you do if there needs to be an update to commit 1, requested by a reviewer?

Honestly, I would like to take a break from this for now.
I leave it up to you how to present it in a way that is easier to review.
Once you get someone to review and accept it, I can see if I find time to review again.

I think the code is significantly better/readable than when we first started. So if someone like @vnkozlov simply scans and approves it, and as such takes the responsibility of "first reviewer", then I'm totally fine with that.

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

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