RFR: 8342387: C2 SuperWord: refactor and improve compiler/loopopts/superword/TestDependencyOffsets.java [v4]

Christian Hagedorn chagedorn at openjdk.org
Wed Oct 23 07:55:11 UTC 2024


On Wed, 23 Oct 2024 07:51:35 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> I want to refactor `TestDependencyOffsets.java` using the `CompileFramework`.
>> 
>> Reasons:
>> - I soon need to modify the IR rules in this test soon anyway (https://github.com/openjdk/jdk/pull/21521), and so a refactor might be good to do first.
>> - The generator script used to be a `generator.py`, stored not on `git` but `JBS`. Not great. Now we have it in Java code, and maintenance is easier.
>> - Since I first wrote that test, I have now introduced the `IRNode.VECTOR_SIZE`. This allows:
>>   - Simplifying the logic for the IR rules (removed the `IRRange` and `IRBool`, and the `Platform`).
>>   - Strengthening the rules.
>> - I was able to add `random` offsets. This means we have better coverage, and do not rely on just hand-crafted values.
>> 
>> I extensively use `String.format` and `StringBuilder`... would be nicer to have string-templates but they don't exist yet.
>> 
>> Recommendation for review: the old file was huge. Finding the new code in the diff can be hard. I would start by only reading the new file.
>> 
>> Ah. And about runtime of the test. On my machine I get this (in ms):
>> 
>> Generate: 27
>> Compile:  5845
>> Run:      23435
>> 
>> Test generation is negligible. 6sec on compilation, 20+sec on execution. I think that is an ok balance, at least we can say that generation and compilation only take about 1/6 of total time - an acceptable overhead I would say.
>
> Emanuel Peter 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 32 additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8342387-refactor-TestDependencyOffsets
>  - Suggestions by Christian
>  - add @compile for IR Framework
>  - more comment
>  - simplify further by removing explicit CPU/Platform vector width
>  - remove 2 useless runs
>  - whitespace
>  - aliasing modes
>  - further cosmetics and comments
>  - add more cases
>  - ... and 22 more: https://git.openjdk.org/jdk/compare/51d48b54...fac481de

Thanks for the update, looks good!

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21541#pullrequestreview-2387622948


More information about the hotspot-compiler-dev mailing list