RFR: 8338407: Support grouping several of existing regs into a new one

Hamlin Li mli at openjdk.org
Thu Aug 29 15:17:33 UTC 2024


Hi,
Can you help to review this patch to add `group` support to operand?

### Some background about this pr

In some platforms, there is some concept like a group of registers, for example on riscv there is vector group, which is a group of other single vectors. For example, m2 could be v2+v3, or v4+v5, m4 could be v4+v5+v6+v7, or v8+v9+v10+v11.
And, it's helpful to represent these vector group explicitly, otherwise it's tedious and error-prone. For example, in existing code, there's some like below:

instruct vstring_compareUL(iRegP_R11 str1, iRegI_R12 cnt1, iRegP_R13 str2, iRegI_R14 cnt2,
                           iRegI_R10 result, vReg_V4 v4, vReg_V5 v5, vReg_V6 v6, vReg_V7 v7,
                           vReg_V8 v8, vReg_V9 v9, vReg_V10 v10, vReg_V11 v11,
                           iRegP_R28 tmp1, iRegL_R29 tmp2)
// ...
  effect(KILL tmp1, KILL tmp2, USE_KILL str1, USE_KILL str2, USE_KILL cnt1, USE_KILL cnt2,
         TEMP v4, TEMP v5, TEMP v6, TEMP v7, TEMP v8, TEMP v9, TEMP v10, TEMP v11);
// ...
    __ string_compare_v($str1$$Register, $str2$$Register,
                        $cnt1$$Register, $cnt2$$Register, $result$$Register,
                        $tmp1$$Register, $tmp2$$Register,
                        StrIntrinsicNode::UL);

The potential problems of the above code are that we need to
1. write v4~v11 explicitly in its `instruct` and its `effect`, it's tedious;
2. vector group are represented implicitly, which is not clear and error-prone;
3. in its encoding `string_compare_v`, we need to specify m4, and v4/v8 explicitly.
4. if some day we need to adjust from m4 to m2 or m8, it's really tedious and error-prone to make that change in both ad file and macro assembler files.


### This PR

The proposed solution is to represent a group of vector registers with a real vector group, e.g. `vReg_V4 v4, vReg_V5 v5, vReg_V6 v6, vReg_V7 v7` with `vReg_V4M4 v4m4`, `TEMP v4, TEMP v5, TEMP v6, TEMP v7` with `TEMP v4m4` and in `string_compare_v` implementation, we could query the length of of vector group (i.e. m4 in this case) and set its vtype automatically.
This solution solve the above listed issues, especially the last issue, that means in the future if we need to adjust m4 to m2 or m8, we only need to change the code in ad file and the change is simpler, and no change in string_compare_v is needed.

### What it looks like

For more usage details, please please check [here](https://github.com/openjdk/jdk/compare/master...Hamlin-Li:jdk:explicit-v-reg-group-v3), the riscv part.

Basically it looks like below:

operand reg_x()
%{
  group(reg_y, reg_z)
%}

And this reg_x can be used in an instruct as its input operand, a TEMP in effect list, in ins_encode as parameters. Underlying, a group operand will be ungrouped automatically as separate operands.

### Alternative

I tried several solutions.
1. One of them is to just add some new reg class and operand, it kindly worked, but can only prevent other regs in an instruct using the one of the vector regs in a vector group.
2. Another is to make modification in chaitin of C2 to support vector group, seems to me it finally turned out not practical, too much basic code change in chaitin and the impact is too big to be acceptable.

Current solution is to change the adlc parser, basically it works like a macro expansion, replacing vector groups with vectors, so in this way chaitin implementation does not need any changes, also means it's simpler to implement and more acceptable for the community (I hope so).

Thanks!

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

Commit messages:
 - clean
 - Initial commit
 - rename from expand/unexpand to ungroup/group
 - simplify group
 - warning
 - assert -> syntax error
 - Merge branch 'master' into explicit-v-reg-group-v3
 - fix assert
 - Merge branch 'master' into explicit-v-reg-group-v3
 - fix warning
 - ... and 10 more: https://git.openjdk.org/jdk/compare/129f527f...f4be2c6c

Changes: https://git.openjdk.org/jdk/pull/20775/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20775&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8338407
  Stats: 398 lines in 8 files changed: 320 ins; 30 del; 48 mod
  Patch: https://git.openjdk.org/jdk/pull/20775.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20775/head:pull/20775

PR: https://git.openjdk.org/jdk/pull/20775


More information about the hotspot-compiler-dev mailing list