[RISCV] Make v extension imply zvamo, zvlsseg

[simoncook]

Summary

The V (vector) extension should enable the zvamo and zvlsseg extensions,
but currently is defined the other way around in TableGen. This updates
the SubtargetFeatures to reverse the inheritence order to be correct.

[craig.topper]

Doesn't this mean that if you only enable zvlsseg, you'll be able to use the instruction in that extension but not the vsetvli instruction that you need to program the VL register?

[simoncook]

In D95146#2512871, @craig.topper wrote:

Doesn't this mean that if you only enable zvlsseg, you'll be able to use the instruction in that extension but not the vsetvli instruction that you need to program the VL register?

@craig.topper To be honest, I'm not at all familiar with the v extension or any of the zv* extensions. I wrote this in response to D94931 which says that V should imply zv* but not the other way around. Looking back now at D85069 it looks there was some discussion suggesting which way around it should be, so it could be this patch is unnecessary and breaks stuff. Do you have any better understand of the vector spec as to which way round is correct?

@kito-cheng Similarly, do you know if the correct order of implication/requirements is well defined somewhere. If there are gcc/binutils patches what does it do here?

[kito-cheng]

In D95146#2513028, @simoncook wrote:

In D95146#2512871, @craig.topper wrote:

Doesn't this mean that if you only enable zvlsseg, you'll be able to use the instruction in that extension but not the vsetvli instruction that you need to program the VL register?

@craig.topper To be honest, I'm not at all familiar with the v extension or any of the zv* extensions. I wrote this in response to D94931 which says that V should imply zv* but not the other way around. Looking back now at D85069 it looks there was some discussion suggesting which way around it should be, so it could be this patch is unnecessary and breaks stuff. Do you have any better understand of the vector spec as to which way round is correct?

@kito-cheng Similarly, do you know if the correct order of implication/requirements is well defined somewhere. If there are gcc/binutils patches what does it do here?

Hmmmmm, I don't think the spec has well defined that, it's kind of vague in this part, I've argue with ISA guys for this, and then got this change in spec.

So how about GCC/binutils, GNU toolchain using a weird work-around here, v implied zvamo and zvlsseg, and zvamo/zvlsseg/zvqmacc implied v...

@craig.topper that's good point, I create a issue on spec: https://github.com/riscv/riscv-v-spec/issues/626

[simoncook]

In D95146#2514390, @kito-cheng wrote:

So how about GCC/binutils, GNU toolchain using a weird work-around here, v implied zvamo and zvlsseg, and zvamo/zvlsseg/zvqmacc implied v...

Having the implications both ways round is a bit annoying to implement this way. I can't forward declare SubtargetFeatures in TableGen, and presumably even if I could I would send the tools in an infinite loop trying to initialise things in this case, so a more detailed/careful change would be needed.

In the mean time I will park this change, and add 'v' implying its subfeatures in clang, since this would catch most cases we expect users to use (I don't think manually enabling features is a common use case). I can then later look at a more detailed/complete fix when enabling features directly with -mattr.

@kito-cheng To clarify, if someone asks for e.g -march=rv32izvamo0p9, I enable 'v' because that's implied by zvamo, do we also enable zvlsseg, or does asking just for zvamo mean 'v+zvamo' but not 'v+zvamo+zvlsseg'/do I imply these features recursively?

[kito-cheng]

In the mean time I will park this change, and add 'v' implying its subfeatures in clang, since this would catch most cases we expect users to use (I don't think manually enabling features is a common use case). I can then later look at a more detailed/complete fix when enabling features directly with -mattr.

That's SGTM.

@kito-cheng To clarify, if someone asks for e.g -march=rv32izvamo0p9, I enable 'v' because that's implied by zvamo, do we also enable zvlsseg, or does asking just for zvamo mean 'v+zvamo' but not 'v+zvamo+zvlsseg'/do I imply these features recursively?

In current GCC implementation, yes, -march=rv32izvamo will result -march=rv32iv_zvamo_zvlsseg, it's buggy work-around to me, but currently seems like no good choice here.

Related issue on v-spec: https://github.com/riscv/riscv-v-spec/issues/546

llvm/lib/Target/RISCV/RISCV.td

 
 def FeatureNoRVCHints
     : SubtargetFeature<"no-rvc-hints", "EnableRVCHintInstrs", "false",
                        "Disable RVC Hint Instructions.">;
 def HasRVCHints : Predicate<"Subtarget->enableRVCHintInstrs()">,
                   AssemblerPredicate<(all_of(not FeatureNoRVCHints)),
                                       "RVC Hint Instructions">;
 
-def FeatureStdExtV
-    : SubtargetFeature<"experimental-v", "HasStdExtV", "true",
-                       "'V' (Vector Instructions)">;
-def HasStdExtV : Predicate<"Subtarget->hasStdExtV()">,
-                           AssemblerPredicate<(all_of FeatureStdExtV),
-                           "'V' (Vector Instructions)">;
-
 def FeatureStdExtZvlsseg
     : SubtargetFeature<"experimental-zvlsseg", "HasStdExtZvlsseg", "true",
-                       "'Zvlsseg' (Vector segment load/store instructions)",
-                       [FeatureStdExtV]>;
+                       "'Zvlsseg' (Vector segment load/store instructions)">;
 def HasStdExtZvlsseg : Predicate<"Subtarget->hasStdExtZvlsseg()">,
                         AssemblerPredicate<(all_of FeatureStdExtZvlsseg),
                         "'Zvlsseg' (Vector segment load/store instructions)">;
-def FeatureExtZvamo
+def FeatureStdExtZvamo
     : SubtargetFeature<"experimental-zvamo", "HasStdExtZvamo", "true",
-                       "'Zvamo'(Vector AMO Operations)",
-                       [FeatureStdExtV]>;
+                       "'Zvamo'(Vector AMO Operations)">;
 def HasStdExtZvamo : Predicate<"Subtarget->hasStdExtZvamo()">,
-                              AssemblerPredicate<(all_of FeatureExtZvamo),
+                              AssemblerPredicate<(all_of FeatureStdExtZvamo),
                               "'Zvamo'(Vector AMO Operations)">;
 
+def FeatureStdExtV
+    : SubtargetFeature<"experimental-v", "HasStdExtV", "true",
+                       "'V' (Vector Instructions)",
+                       [FeatureStdExtZvlsseg,
+                        FeatureStdExtZvamo]>;
+def HasStdExtV : Predicate<"Subtarget->hasStdExtV()">,
+                           AssemblerPredicate<(all_of FeatureStdExtV),
+                           "'V' (Vector Instructions)">;
+
 def Feature64Bit
     : SubtargetFeature<"64bit", "HasRV64", "true", "Implements RV64">;
 def IsRV64 : Predicate<"Subtarget->is64Bit()">,
                        AssemblerPredicate<(all_of Feature64Bit),
                        "RV64I Base Instruction Set">;
 def IsRV32 : Predicate<"!Subtarget->is64Bit()">,
                        AssemblerPredicate<(all_of (not Feature64Bit)),
                        "RV32I Base Instruction Set">;

llvm/test/CodeGen/RISCV/rvv/vlseg-rv32.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -mattr=+d,+experimental-zvlsseg,+experimental-zfh \
+; RUN: llc -mtriple=riscv32 -mattr=+d,+experimental-v,+experimental-zvlsseg,+experimental-zfh \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
 
 declare {<vscale x 16 x i16>,<vscale x 16 x i16>} @llvm.riscv.vlseg2.nxv16i16(i16* , i32)
 declare {<vscale x 16 x i16>,<vscale x 16 x i16>} @llvm.riscv.vlseg2.mask.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, <vscale x 16 x i1>, i32)
 
 define <vscale x 16 x i16> @test_vlseg2_nxv16i16(i16* %base, i32 %vl) {
 ; CHECK-LABEL: test_vlseg2_nxv16i16:
 ; CHECK:       # %bb.0: # %entry

llvm/test/CodeGen/RISCV/rvv/vlseg-rv64.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv64 -mattr=+d,+experimental-zvlsseg,+experimental-zfh \
+; RUN: llc -mtriple=riscv64 -mattr=+d,+experimental-v,+experimental-zvlsseg,+experimental-zfh \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
 
 declare {<vscale x 16 x i16>,<vscale x 16 x i16>} @llvm.riscv.vlseg2.nxv16i16(i16* , i64)
 declare {<vscale x 16 x i16>,<vscale x 16 x i16>} @llvm.riscv.vlseg2.mask.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, <vscale x 16 x i1>, i64)
 
 define <vscale x 16 x i16> @test_vlseg2_nxv16i16(i16* %base, i64 %vl) {
 ; CHECK-LABEL: test_vlseg2_nxv16i16:
 ; CHECK:       # %bb.0: # %entry

llvm/test/CodeGen/RISCV/rvv/vlsseg-rv32.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -mattr=+d,+experimental-zvlsseg,+experimental-zfh \
+; RUN: llc -mtriple=riscv32 -mattr=+d,+experimental-v,+experimental-zvlsseg,+experimental-zfh \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
 
 declare {<vscale x 16 x i16>,<vscale x 16 x i16>} @llvm.riscv.vlsseg2.nxv16i16(i16*, i32, i32)
 declare {<vscale x 16 x i16>,<vscale x 16 x i16>} @llvm.riscv.vlsseg2.mask.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, i32, <vscale x 16 x i1>, i32)
 
 define <vscale x 16 x i16> @test_vlsseg2_nxv16i16(i16* %base, i32 %offset, i32 %vl) {
 ; CHECK-LABEL: test_vlsseg2_nxv16i16:
 ; CHECK:       # %bb.0: # %entry

llvm/test/CodeGen/RISCV/rvv/vlsseg-rv64.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv64 -mattr=+d,+experimental-zvlsseg,+experimental-zfh \
+; RUN: llc -mtriple=riscv64 -mattr=+d,+experimental-v,+experimental-zvlsseg,+experimental-zfh \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
 
 declare {<vscale x 16 x i16>,<vscale x 16 x i16>} @llvm.riscv.vlsseg2.nxv16i16(i16*, i64, i64)
 declare {<vscale x 16 x i16>,<vscale x 16 x i16>} @llvm.riscv.vlsseg2.mask.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, i64, <vscale x 16 x i1>, i64)
 
 define <vscale x 16 x i16> @test_vlsseg2_nxv16i16(i16* %base, i64 %offset, i64 %vl) {
 ; CHECK-LABEL: test_vlsseg2_nxv16i16:
 ; CHECK:       # %bb.0: # %entry

llvm/test/CodeGen/RISCV/rvv/vsseg-rv32.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -mattr=+d,+experimental-zvlsseg,+experimental-zfh \
+; RUN: llc -mtriple=riscv32 -mattr=+d,+experimental-v,+experimental-zvlsseg,+experimental-zfh \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
 
 declare void @llvm.riscv.vsseg2.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16* , i32)
 declare void @llvm.riscv.vsseg2.mask.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, <vscale x 16 x i1>, i32)
 
 define void @test_vsseg2_nxv16i16(<vscale x 16 x i16> %val, i16* %base, i32 %vl) {
 ; CHECK-LABEL: test_vsseg2_nxv16i16:
 ; CHECK:       # %bb.0: # %entry

llvm/test/CodeGen/RISCV/rvv/vsseg-rv64.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv64 -mattr=+d,+experimental-zvlsseg,+experimental-zfh \
+; RUN: llc -mtriple=riscv64 -mattr=+d,+experimental-v,+experimental-zvlsseg,+experimental-zfh \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
 
 declare void @llvm.riscv.vsseg2.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16* , i64)
 declare void @llvm.riscv.vsseg2.mask.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, <vscale x 16 x i1>, i64)
 
 define void @test_vsseg2_nxv16i16(<vscale x 16 x i16> %val, i16* %base, i64 %vl) {
 ; CHECK-LABEL: test_vsseg2_nxv16i16:
 ; CHECK:       # %bb.0: # %entry

llvm/test/CodeGen/RISCV/rvv/vssseg-rv32.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -mattr=+d,+experimental-zvlsseg,+experimental-zfh \
+; RUN: llc -mtriple=riscv32 -mattr=+d,+experimental-v,+experimental-zvlsseg,+experimental-zfh \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
 
 declare void @llvm.riscv.vssseg2.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, i32, i32)
 declare void @llvm.riscv.vssseg2.mask.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, i32, <vscale x 16 x i1>, i32)
 
 define void @test_vssseg2_nxv16i16(<vscale x 16 x i16> %val, i16* %base, i32 %offset, i32 %vl) {
 ; CHECK-LABEL: test_vssseg2_nxv16i16:
 ; CHECK:       # %bb.0: # %entry

llvm/test/CodeGen/RISCV/rvv/vssseg-rv64.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv64 -mattr=+d,+experimental-zvlsseg,+experimental-zfh \
+; RUN: llc -mtriple=riscv64 -mattr=+d,+experimental-v,+experimental-zvlsseg,+experimental-zfh \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
 
 declare void @llvm.riscv.vssseg2.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, i64, i64)
 declare void @llvm.riscv.vssseg2.mask.nxv16i16(<vscale x 16 x i16>,<vscale x 16 x i16>, i16*, i64, <vscale x 16 x i1>, i64)
 
 define void @test_vssseg2_nxv16i16(<vscale x 16 x i16> %val, i16* %base, i64 %offset, i64 %vl) {
 ; CHECK-LABEL: test_vssseg2_nxv16i16:
 ; CHECK:       # %bb.0: # %entry