RFR: 8346664: C2: Optimize mask check with constant offset [v7]

[Andrew Dinn]

On Thu, 30 Jan 2025 08:47:25 GMT, Matthias Ernst <duke at openjdk.org> wrote:

>> Matthias Ernst has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 20 additional commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into mernst/JDK-8346664
>>  - make the check more clear: shift >= mask_width
>>  - fully randomized
>>  - JLS: only the lower bits of the shift are taken into account (aka we don't assert).
>>  - (c)
>>  - (c)
>>  - Assert that MulNode::Ideal already masks constant shift amounts for us.
>>    Avoid accidental zero mask breaking test.
>>  - "element".
>>  - avoid redundant comment
>>  - addConstNonConstMaskLong
>>  - ... and 10 more: https://git.openjdk.org/jdk/compare/b4cd6473...490cc2fb
>
>> TestShiftConvertAndNotification
>> TestAndShiftZeroCCP
> 
> Can repro. This is on me, I apparently did not run all tier1 after the assert. It has to do with mixed long/int operands. I will investigate.

@mernst-github Thanks for your response and for the constructive feedback which is appreciated. Of course, you are right that we still have a lot of work to do to improve the current code and test suite. The project's reviewers are very aware that we have a great deal of technical debt that needs to be shovelled off our lawn. I did not intend to suggest that your code was not an improvement on what we currently have and I am sorry I did not make that clear in what I wrote.

I also recognize your statement that we are struggling with the pace of change in the code base and, in particular, to keep up with reviews,-- despite the fact that we have dramatically increased the number of project reviewers in recent years. One of the major goals of the project over the decade and more that I have been a contributor (and one I have been very much involved in at Red Hat) is to bring new developers into the project. That was and still remains critical to allow us to continue to develop the platform, keeping it performant on newly emerging hardware and ensuring it meets the changing needs and priorities of middleware and application developers. It is also critical if we are to remove the technical debt that has built up over the last 25 years.

I hope you recognize that the scale and complexity of the JDK and JVM code base means we are performing a difficult balancing act here. We are trying to recruit as many new, competent contributors as we can -- people like you -- while at the same time trying as far as we can to ensure that any new commits are complete and include testing that forestalls future issues i.e. lower rather than raise our legacy of technical debt. However, scaling up committers also involves scaling up the number of people competent to review commits. That is a tall order when it comes to complex commits or commits that affect the more intricate mechanisms in the JVM (like the compiler) and that may interact in subtle ways with other, concurrent changes. It also gets harder and harder as the language/code base extends its complexity and range of delivered variants. We still do not have enough reviewers, like Emanuel, who are able to ensure that the runtime's many moving parts correctly mesh together and it
  takes us a long time to grow people with those skills.

In effect, the pace of change we have been forced to adopt means we are still bootstrapping the project 25 years in. I hope you will make allowance for that and continue to work with us all the same, perhaps even with the goal of becoming a reviewer and taking on some of that burden.

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

PR Comment: https://git.openjdk.org/jdk/pull/22856#issuecomment-2624203107