RFR: 8310308: IR Framework: check for type and size of vector nodes [v18]

[Tobias Hartmann]

On Mon, 3 Jul 2023 09:37:22 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> For some changes to `SuperWord`, and maybe auto-vectorization in general, I want to strengthen the IR Framework.
>> 
>> **Motivation**
>> I want to not just find the relevant IR nodes, but also assert that they have the maximal length that they could have on the respective platform (given the CPU features and `MaxVectorSize`). Without this verification it is possible that a future change leads to a regression where we still vectorize but at shorter vector widths as before - leading to performance loss.
>> 
>> **How to use it**
>> 
>> All `IRNode`s in `test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java` that are created with `vectorNode` are now all matched with their `type` and `size`. The regex might now look something like this:
>> 
>> `"(\d+(\s){2}(VectorCastF2X.*)+(\s){2}===.*vector[A-Za-z][8]:{int})"`
>> which would match with IR nodes dumped like that:
>> `1150  VectorCastF2X  === _ 1151  [[ 1146 ]]  #vectory[8]:{int} ...`
>> 
>> The goal was to keep it simple and straight forward. In most cases, you can just use the nodes as before, and implicitly we now check for maximal size automatically. However, in some cases we want to ensure there is no or only a limited number of nodes (`failOn` or comparison `<` or `<=` or `=0`) - in those cases we usually want to make sure there is not any node of any size, so we match with any size by default. The size can also explicitly be constrained using `IRNode.VECTOR_SIZE`.
>> 
>> Some examples:
>> 1. `@IR(counts = {IRNode.LOAD_VECTOR_I,  " >0 "})` -> search for a `LoadVector` node with `type` `int`, and maximal `size` possible on the machine (limited by CPU features and `MaxVectorSize`). This is the most common use case.
>> 2. `@IR(failOn = { IRNode.LOAD_VECTOR_L, IRNode.STORE_VECTOR })` -> fail if there is a `LoadVector` with type `long`, of `any` size.
>> 3. `@IR(counts = { IRNode.XOR_VI, IRNode.VECTOR_SIZE_4, " > 0 "})` -> find at least one `XorV` node with type `int` and exactly `4` elements. Useful for VectorAPI when the vector species is fixed.
>> 4. `@IR(counts = { IRNode.LOAD_VECTOR_D, IRNode.VECTOR_SIZE + "min(4, max_double)", " >0 " })` -> search for a `LoadVector` node with `type` `double`, and `size` exactly equals to `min(4, max_double)` (so 4 elements, or if the hardware allows fewer `doubles`, then that number).
>> 5. `@IR(counts = { IRNode.ABS_VF, IRNode.VECTOR_SIZE + "min(LoopMaxUnroll, max_float)", ">= 1" })` -> find at least one `AbsV` nodes with type `float`, and the `size` exactly equals to the smaller of...
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   more for Christian's reviews

Nice enhancement! I skimmed through the changes and it looks good to me.

test/hotspot/jtreg/compiler/lib/ir_framework/driver/irmatching/irrule/constraint/raw/RawCountsConstraint.java line 62:

> 60:         switch (comparison.getComparator()) {
> 61:             case "<" -> {
> 62:                 TestFormat.checkNoReport(comparison.getGivenValue() > 1, "Node count comparison \"<" +

What if `comparison.getGivenValue()` is negative?

test/hotspot/jtreg/compiler/lib/ir_framework/driver/irmatching/irrule/constraint/raw/RawCountsConstraint.java line 86:

> 84:             }
> 85:             case "!=" -> throw new TestFormatException("Not-equal comparator not supported for node count: \"" +
> 86:                                                        comparison.getComparator() + "\". Please rewrite the rule.");

No need to call `comparison.getComparator()` again here.

test/hotspot/jtreg/compiler/lib/ir_framework/driver/irmatching/parser/VMInfo.java line 59:

> 57:             return Long.parseLong(getStringValue(key));
> 58:         } catch (NumberFormatException e) {
> 59:             throw new TestFrameworkException("VMInfo value for \"" + key + "\" is not long, got \"" + getStringValue(key) + "\"");

Suggestion:

            throw new TestFrameworkException("VMInfo value for "" + key + "" is not a long, got "" + getStringValue(key) + """);

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14539#pullrequestreview-1512584953
PR Review Comment: https://git.openjdk.org/jdk/pull/14539#discussion_r1251833266
PR Review Comment: https://git.openjdk.org/jdk/pull/14539#discussion_r1251830428
PR Review Comment: https://git.openjdk.org/jdk/pull/14539#discussion_r1251834541