RISC-V Extension Changes coming to LLVM 22

[lenary]

In LLVM 22.x, we intend to simplify how the C and Zc* extensions are handled in the LLVM backend, in the change llvm/llvm-project#155035. This should more closely align with how these instructions are specified in the RISC-V specification.

In clang, we are relying on the rules implemented in llvm/lib/TargetParser/RISCVISAInfo.cpp to expand out the complex rules around C and the Zc* extensions, to simplify the backend.

This has an impact on other frontends which are not already using the logic in RISCVISAInfo, which I think includes Zig.

To enable e.g. the Zcf instructions, it will no longer be enough to enable +c,+f in the feature string, +zcf will also be needed. The same applies for +zcd.

I note there's lots of code about risc-v features and their dependencies here (matching LLVM), but I'm not sure if you have the additional rules from the spec which are also implemented in RISCVISAInfo.

https://github.com/ziglang/zig/blob/master/lib/std/Target/riscv.zig

It might be easiest to fully expand the list of architecture features, which can be done using a recent build of clang:

$ clang --target=riscv32 -march=rv32imafdc_zicsr_zifencei -print-enabled-extensions
Extensions enabled for the given RISC-V target

    Name                 Version   Description
    i                    2.1       'I' (Base Integer Instruction Set)
    m                    2.0       'M' (Integer Multiplication and Division)
    a                    2.1       'A' (Atomic Instructions)
    f                    2.2       'F' (Single-Precision Floating-Point)
    d                    2.2       'D' (Double-Precision Floating-Point)
    c                    2.0       'C' (Compressed Instructions)
    zicsr                2.0       'Zicsr' (CSRs)
    zifencei             2.0       'Zifencei' (fence.i)
    zmmul                1.0       'Zmmul' (Integer Multiplication)
    zaamo                1.0       'Zaamo' (Atomic Memory Operations)
    zalrsc               1.0       'Zalrsc' (Load-Reserved/Store-Conditional)
    zca                  1.0       'Zca' (part of the C extension, excluding compressed floating point loads/stores)
    zcd                  1.0       'Zcd' (Compressed Double-Precision Floating-Point Instructions)
    zcf                  1.0       'Zcf' (Compressed Single-Precision Floating-Point Instructions)

Experimental extensions

ISA String: rv32i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zmmul1p0_zaamo1p0_zalrsc1p0_zca1p0_zcd1p0_zcf1p0

Where the first column gives the list of extensions to enable. (The same can be done with e.g. -mcpu=<cpu name> or -march=<profile name> instead of -march=<arch>, if that better represents the intention of the target).

The extension lists will not always match between rv32 and rv64, so care should be taken when doing this change.

[andrewrk]

Hi Sam, thanks for stopping by and sharing this information. Really appreciate it!

cc @alexrp and @Rexicon226

[alexrp]

Thanks for giving us a heads up about this.

I note there's lots of code about risc-v features and their dependencies here (matching LLVM), but I'm not sure if you have the additional rules from the spec which are also implemented in RISCVISAInfo.

This feature data is indeed extracted from the TableGen files in LLVM's RISC-V backend. So any feature dependencies are inherited from there. This also means we don't have any extra rules that are implemented by RISCVISAInfo.

To be honest, I don't love the direction of this change. LLVM already has a subtarget feature dependency system for backends. For cases that don't quite fit that system, IIRC, some backends add/remove feature flags in the subtarget depending on other feature flags or compilation options. Couldn't the RISC-V backend do something similar rather than making every frontend deal with this?

[lenary]

LLVM's subtarget implications can only deal with x implies y in features (which can also be read as x requires y). As much as possible, we have tried to move the C/Zc* implications into this structure, as can be seen by e.g. the implied features of Zce or C being the largest sets of implied extensions which don't have complex rules.

Sadly the RISC-V spec has made things more complex due to the (x and z) implies y rules that were added when the C specification was broken into parts. Thankfully there are only three of these rules.

I am not entirely convinced we want to do this in the subtarget, especially around the subtarget constructor, but maybe we do. The reason to do this in RISCVISAInfo is to have a single implementation of the rules from the spec, rather than several. I will keep thinking about the issue.

[alexrp]

Sadly the RISC-V spec has made things more complex due to the (x and z) implies y rules that were added when the C specification was broken into parts. Thankfully there are only three of these rules.

That would be these, right?

https://github.com/llvm/llvm-project/blob/dac9d8f8e06d02e5e32f28ba6a660ad759033023/llvm/lib/TargetParser/RISCVISAInfo.cpp#L859-L876

I am not entirely convinced we want to do this in the subtarget, especially around the subtarget constructor, but maybe we do. The reason to do this in RISCVISAInfo is to have a single implementation of the rules from the spec, rather than several. I will keep thinking about the issue.

I would argue that, if we really are only talking about 3 rules, then it doesn't seem too bad to duplicate the logic with comments in both places reminding people to update the other. If the number of rules grows in the future to the point that this approach becomes untenable, then it may indeed be necessary to make a more involved change. But with just 3 rules and no(?) indication that this number will grow, it seems a bit early to be pushing this concern to frontends.

[lenary]

Sadly the RISC-V spec has made things more complex due to the (x and z) implies y rules that were added when the C specification was broken into parts. Thankfully there are only three of these rules.

That would be these, right?

https://github.com/llvm/llvm-project/blob/dac9d8f8e06d02e5e32f28ba6a660ad759033023/llvm/lib/TargetParser/RISCVISAInfo.cpp#L859-L876

Yes

I am not entirely convinced we want to do this in the subtarget, especially around the subtarget constructor, but maybe we do. The reason to do this in RISCVISAInfo is to have a single implementation of the rules from the spec, rather than several. I will keep thinking about the issue.

I would argue that, if we really are only talking about 3 rules, then it doesn't seem too bad to duplicate the logic with comments in both places reminding people to update the other. If the number of rules grows in the future to the point that this approach becomes untenable, then it may indeed be necessary to make a more involved change. But with just 3 rules and no(?) indication that this number will grow, it seems a bit early to be pushing this concern to frontends.

I think the same argument could be made in exactly the opposite direction, especially as frontends seem to be better for emitting error messages etc.

Zig does seem to be the only frontend not happy to make the change, from those I've notified, but I am still thinking about what we can do in the subtarget that won't be confusing for LLVM.