[RISCV] Accept zicsr and zifencei command line options

[reames]

Summary

This change adds the definition of the two extensions, but does not either a) make any instruction conditional on them or b) enabled the extensions by default. (The *instructions* do remain enabled by default per ISA version 2.0 which is our current default.)

This is meant to be a building block towards something like https://reviews.llvm.org/D141666, and in the meantime, address one of the most surprising of the current user experience warts. The current behavior of rejecting the extensions at the command line despite emitting code which appears to use them is surprising to anyone not deeply versed in the details of this situation.

Between versions 2.0 and 2.1 of the base I specification, a backwards incompatible change was made to remove selected instructions and CSRs from the base ISA. These instructions were grouped into a set of new extensions (these), but were no longer required by the base ISA. This change is described in “Preface to Document Version 20190608-Base-Ratified” from the specification document.

As LLVM currently implements only version 2.0 of the base specification, accepting these extensions at the command line introduces a configuration which doesn't actually match any spec version. It's a pretty harmless variant since the 2.0 extension definitions, to my knowledge, exactly match the text from the 2.0 I text before they were moved into standalone extensions in 2.1 of I. (The version numbering in that sentence is a tad confusing to say the least. Hopefully I got it right.)

It is worth noting that we already have numerous examples of accepting extensions in the march string which didn't exist in version of the spec document corresponding to our current base I version, so this doesn't set any new precedent.

[jrtc27]

I'm fine with this approach but I think they should be filtered out of the ELF attributes, maybe also preprocessor macros? That is, treating them as a redundant no-op in the driver rather than like a true extension.

[reames]

@jrtc27 Not sure if this changes your take, but I realized the variant being introduced is maybe much less interesting than I'd first thought. We generally make no effort to make sure an extension was defined in the spec version corresponding to our base revision. Given that, we have a bunch of cases where we allow I2.0 + some random extension. Given that, this one stops looking all that interesting. It doesn't actually set much precedent - because we already did that, a long time ago.

If you agree with that framing, I'll rework the description.

[jrtc27]

In D143953#4124636, @reames wrote:

@jrtc27 Not sure if this changes your take, but I realized the variant being introduced is maybe much less interesting than I'd first thought. We generally make no effort to make sure an extension was defined in the spec version corresponding to our base revision. Given that, we have a bunch of cases where we allow I2.0 + some random extension. Given that, this one stops looking all that interesting. It doesn't actually set much precedent - because we already did that, a long time ago.

If you agree with that framing, I'll rework the description.

Hm, do we allow M + Zmmul? If so then I guess I can get behind that view.

[reames]

In D143953#4124649, @jrtc27 wrote:

In D143953#4124636, @reames wrote:

@jrtc27 Not sure if this changes your take, but I realized the variant being introduced is maybe much less interesting than I'd first thought. We generally make no effort to make sure an extension was defined in the spec version corresponding to our base revision. Given that, we have a bunch of cases where we allow I2.0 + some random extension. Given that, this one stops looking all that interesting. It doesn't actually set much precedent - because we already did that, a long time ago.

If you agree with that framing, I'll rework the description.

Hm, do we allow M + Zmmul? If so then I guess I can get behind that view.

$ ~/llvm-dev/build/bin/clang -target riscv64 -march=rv64g_zmmul vector_add_i32.c -S
$ cat vector_add_i32.s 
	.text
	.attribute	4, 16
	.attribute	5, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_zmmul1p0"

vector_add_i32.c is a random C file; contents are uninteresting.

[jrtc27]

Personally happy with the concept then, seems consistent and overall helpful, just some nits

llvm/lib/Target/RISCV/RISCVFeatures.td

83–86

s/ifence/fence.i/ (or Instruction Fence if you want words) for both these

llvm/test/CodeGen/RISCV/attributes.ll

105–106

155–156

[asb]

Thanks Philip. I started looking at this too, and agree that something like this is a good next step. I've been looking more closely at the GCC behaviour and noticed a couple of things that might be of interest:

• -march=rv64gc results in`rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0`, but -march=rv64imafdc results in rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0
• -march=rv64i2p0_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0 results in rv64i2p0_m2p0_a2p1_f2p2_d2p2_c2p0 (i.e. the zicsr and zifencei are silently dropped)

The strategy proposed in this patch seems sensible to me, unless someone can see a good reason I'm missing to try to go more down the route gcc took and to rewrite the -march string?

I'm looking at the case now of LLD erroring out when linking a .o specifying I2.1, which I think was unintentional and a regression from 15.0.x to 16.0.x. Similar with llvm-objdump refusing to try disassembling a file with I2.1, which is probably more user hostile than we'd like.

[reames]

Add docs, and rework description.

[reames]

Address @jrtc27's comments.

[reames]

We seem to have rough consensus here, but I'm going to wait roughly 24 hours before landing to give others time to comment.

[asb]

I've posted RFC: Resolving issues related to extension versioning in RISC-V which discusses some related issues (I think it complements this patch, rather than anything being blocking or suggesting a contradictory approach).

[reames]

This was discussed at today's RISCV sync-up call. We have a large conversion pending on the topic of isa compatibility checking, but there was a consensus that this was reasonable and could move forward. I'm going to be landing this change in the near future.

[asb]

LGTM, and the summary of the discussion in the sync-up call matches my understanding.

clang/test/Preprocessor/riscv-target-features.c

 // RUN: -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-ZCD-EXT %s
 // RUN: %clang -target riscv64 -march=rv64izcd1p0 -menable-experimental-extensions \
 // RUN: -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-ZCD-EXT %s
 // CHECK-ZCD-EXT: __riscv_zcd 1000000{{$}}
 
 // RUN: %clang -target riscv32 -march=rv32izcf1p0 -menable-experimental-extensions \
 // RUN: -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-ZCF-EXT %s
 // CHECK-ZCF-EXT: __riscv_zcf 1000000{{$}}
+
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32izicsr2p0 -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-ZICSR-EXT %s
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64izicsr2p0 -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-ZICSR-EXT %s
+// CHECK-ZICSR-EXT: __riscv_zicsr 2000000{{$}}
+
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32izifencei2p0 -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-ZIFENCEI-EXT %s
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64izifencei2p0 -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-ZIFENCEI-EXT %s
+// CHECK-ZIFENCEI-EXT: __riscv_zifencei 2000000{{$}}

llvm/docs/RISCVUsage.rst

 .. _riscv-vlen-32-note:
 
 ``Zve32x``, ``Zve32f``, ``Zvl32b``
   LLVM currently assumes a minimum VLEN (vector register width) of 64 bits during compilation, and as a result ``Zve32x`` and ``Zve32f`` are supported only for VLEN>=64.  Assembly support doesn't have this restriction.
 
 .. _riscv-i2p1-note:
 
 ``zicsr``, ``zifencei``
-  Between versions 2.0 and 2.1 of the base I specification, a backwards incompatible change was made to remove selected instructions and CSRs from the base ISA.  These instructions were grouped into a set of new extensions, but were no longer required by the base ISA.  This change is described in "Preface to Document Version 20190608-Base-Ratified" from the specification document.  LLVM currently implements version 2.0 of the base specification.  Thus, instructions from these extensions are accepted as part of the base ISA, but attempts to explicitly enable the extensions will error.
+  Between versions 2.0 and 2.1 of the base I specification, a backwards incompatible change was made to remove selected instructions and CSRs from the base ISA.  These instructions were grouped into a set of new extensions, but were no longer required by the base ISA.  This change is described in "Preface to Document Version 20190608-Base-Ratified" from the specification document.  LLVM currently implements version 2.0 of the base specification.  Thus, instructions from these extensions are accepted as part of the base ISA.  LLVM also allows the explicit specification of the extensions in an march string.
 
 Experimental Extensions
 =======================
 
 LLVM supports (to various degrees) a number of experimental extensions.  All experimental extensions have ``experimental-`` as a prefix.  There is explicitly no compatibility promised between versions of the toolchain, and regular users are strongly advised *not* to make use of experimental extensions before they reach ratification.
 
 The primary goal of experimental support is to assist in the process of ratification by providing an existence proof of an implementation, and simplifying efforts to validate the value of a proposed extension against large code bases.  Experimental extensions are expected to either transition to ratified status, or be eventually removed.  The decision on whether to accept an experimental extension is currently done on an entirely case by case basis; if you want to propose one, attending the bi-weekly RISC-V sync-up call is strongly advised.
 

llvm/lib/Support/RISCVISAInfo.cpp

static const RISCVSupportedExtension SupportedExtensions[] = {
     {"zve32f", RISCVExtensionVersion{1, 0}},
     {"zve64x", RISCVExtensionVersion{1, 0}},
     {"zve64f", RISCVExtensionVersion{1, 0}},
     {"zve64d", RISCVExtensionVersion{1, 0}},
 
     {"zicbom", RISCVExtensionVersion{1, 0}},
     {"zicboz", RISCVExtensionVersion{1, 0}},
     {"zicbop", RISCVExtensionVersion{1, 0}},
+    {"zicsr", RISCVExtensionVersion{2, 0}},
+    {"zifencei", RISCVExtensionVersion{2, 0}},
 
     {"svnapot", RISCVExtensionVersion{1, 0}},
     {"svpbmt", RISCVExtensionVersion{1, 0}},
     {"svinval", RISCVExtensionVersion{1, 0}},
 
     // vendor-defined ('X') extensions
     {"xtheadba", RISCVExtensionVersion{1, 0}},
     {"xtheadbb", RISCVExtensionVersion{1, 0}},

llvm/lib/Target/RISCV/RISCVFeatures.td

 
 def FeatureStdExtZihintntl
     : SubtargetFeature<"experimental-zihintntl", "HasStdExtZihintntl", "true",
                        "'zihintntl' (Non-Temporal Locality Hints)">;
 def HasStdExtZihintntl : Predicate<"Subtarget->hasStdExtZihintntl()">,
                                     AssemblerPredicate<(all_of FeatureStdExtZihintntl),
                                     "'Zihintntl' (Non-Temporal Locality Hints)">;
 
+def FeatureStdExtZicsr
+    : SubtargetFeature<"zicsr", "HasStdExtZicsr", "true",
+                       "'zicsr' (CSRs)">;
+def HasStdExtZicsr : Predicate<"Subtarget->hasStdExtZicsr()">,
+                                AssemblerPredicate<(all_of FeatureStdExtZicsr),
+                                "'Zicsr' (CSRs)">;
+
+def FeatureStdExtZifencei
+    : SubtargetFeature<"zifencei", "HasStdExtZifencei", "true",
+                       "'zifencei' (fence.i)">;
+def HasStdExtZifencei : Predicate<"Subtarget->hasStdExtZifencei()">,
+                                   AssemblerPredicate<(all_of FeatureStdExtZifencei),
+                                   "'Zifencei' (fence.i)">;
+
 def FeatureStdExtZfhmin
     : SubtargetFeature<"zfhmin", "HasStdExtZfhmin", "true",
                        "'Zfhmin' (Half-Precision Floating-Point Minimal)",
                        [FeatureStdExtF]>;
 def HasStdExtZfhmin : Predicate<"Subtarget->hasStdExtZfhmin()">,
                              AssemblerPredicate<(all_of FeatureStdExtZfhmin),
                              "'Zfhmin' (Half-Precision Floating-Point Minimal)">;
 

llvm/test/CodeGen/RISCV/attributes.ll

 ; RUN: llc -mtriple=riscv32 -mattr=+svnapot %s -o - | FileCheck --check-prefixes=CHECK,RV32SVNAPOT %s
 ; RUN: llc -mtriple=riscv32 -mattr=+svpbmt %s -o - | FileCheck --check-prefixes=CHECK,RV32SVPBMT %s
 ; RUN: llc -mtriple=riscv32 -mattr=+svinval %s -o - | FileCheck --check-prefixes=CHECK,RV32SVINVAL %s
 ; RUN: llc -mtriple=riscv32 -mattr=+xtheadmac %s -o - | FileCheck --check-prefixes=CHECK,RV32XTHEADMAC %s
 ; RUN: llc -mtriple=riscv32 -mattr=+experimental-zca %s -o - | FileCheck --check-prefixes=CHECK,RV32ZCA %s
 ; RUN: llc -mtriple=riscv32 -mattr=+experimental-zcb %s -o - | FileCheck --check-prefixes=CHECK,RV32ZCB %s
 ; RUN: llc -mtriple=riscv32 -mattr=+experimental-zcd %s -o - | FileCheck --check-prefixes=CHECK,RV32ZCD %s
 ; RUN: llc -mtriple=riscv32 -mattr=+experimental-zcf %s -o - | FileCheck --check-prefixes=CHECK,RV32ZCF %s
+; RUN: llc -mtriple=riscv32 -mattr=+zicsr %s -o - | FileCheck --check-prefixes=CHECK,RV32ZICSR %s
+; RUN: llc -mtriple=riscv32 -mattr=+zifencei %s -o - | FileCheck --check-prefixes=CHECK,RV32ZIFENCEI %s
 
 ; RUN: llc -mtriple=riscv64 %s -o - | FileCheck %s
 ; RUN: llc -mtriple=riscv64 -mattr=+m %s -o - | FileCheck --check-prefixes=CHECK,RV64M %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zmmul %s -o - | FileCheck --check-prefixes=CHECK,RV64ZMMUL %s
 ; RUN: llc -mtriple=riscv64 -mattr=+m,+zmmul %s -o - | FileCheck --check-prefixes=CHECK,RV64MZMMUL %s
 ; RUN: llc -mtriple=riscv64 -mattr=+a %s -o - | FileCheck --check-prefixes=CHECK,RV64A %s
 ; RUN: llc -mtriple=riscv64 -mattr=+f %s -o - | FileCheck --check-prefixes=CHECK,RV64F %s
 ; RUN: llc -mtriple=riscv64 -mattr=+d %s -o - | FileCheck --check-prefixes=CHECK,RV64D %s

 ; RUN: llc -mtriple=riscv64 -mattr=+xtheadbs %s -o - | FileCheck --check-prefixes=CHECK,RV64XTHEADBS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+xtheadmac %s -o - | FileCheck --check-prefixes=CHECK,RV64XTHEADMAC %s
 ; RUN: llc -mtriple=riscv64 -mattr=+xtheadvdot %s -o - | FileCheck --check-prefixes=CHECK,RV64XTHEADVDOT %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-zawrs %s -o - | FileCheck --check-prefixes=CHECK,RV64ZAWRS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-ztso %s -o - | FileCheck --check-prefixes=CHECK,RV64ZTSO %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-zca %s -o - | FileCheck --check-prefixes=CHECK,RV64ZCA %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-zcb %s -o - | FileCheck --check-prefixes=CHECK,RV64ZCB %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-zcd %s -o - | FileCheck --check-prefixes=CHECK,RV64ZCD %s
+; RUN: llc -mtriple=riscv64 -mattr=+zicsr %s -o - | FileCheck --check-prefixes=CHECK,RV64ZICSR %s
+; RUN: llc -mtriple=riscv64 -mattr=+zifencei %s -o - | FileCheck --check-prefixes=CHECK,RV64ZIFENCEI %s
 
 ; CHECK: .attribute 4, 16
 
 ; RV32M: .attribute 5, "rv32i2p0_m2p0"
 ; RV32ZMMUL: .attribute 5, "rv32i2p0_zmmul1p0"
 ; RV32MZMMUL: .attribute 5, "rv32i2p0_m2p0_zmmul1p0"
 ; RV32A: .attribute 5, "rv32i2p0_a2p0"
 ; RV32F: .attribute 5, "rv32i2p0_f2p0"
 ; RV32D: .attribute 5, "rv32i2p0_f2p0_d2p0"

 ; RV32SVNAPOT: .attribute 5, "rv32i2p0_svnapot1p0"
 ; RV32SVPBMT: .attribute 5, "rv32i2p0_svpbmt1p0"
 ; RV32SVINVAL: .attribute 5, "rv32i2p0_svinval1p0"
 ; RV32XTHEADMAC: .attribute 5, "rv32i2p0_xtheadmac1p0"
 ; RV32ZCA: .attribute 5, "rv32i2p0_zca1p0"
 ; RV32ZCB: .attribute 5, "rv32i2p0_zca1p0_zcb1p0"
 ; RV32ZCD: .attribute 5, "rv32i2p0_zcd1p0"
 ; RV32ZCF: .attribute 5, "rv32i2p0_zcf1p0"
+; RV32ZICSR: .attribute 5, "rv32i2p0_zicsr2p0"
+; RV32ZIFENCEI: .attribute 5, "rv32i2p0_zifencei2p0"
 
 ; RV64M: .attribute 5, "rv64i2p0_m2p0"
 ; RV64ZMMUL: .attribute 5, "rv64i2p0_zmmul1p0"
 ; RV64MZMMUL: .attribute 5, "rv64i2p0_m2p0_zmmul1p0"
 ; RV64A: .attribute 5, "rv64i2p0_a2p0"
 ; RV64F: .attribute 5, "rv64i2p0_f2p0"
 ; RV64D: .attribute 5, "rv64i2p0_f2p0_d2p0"
 ; RV64C: .attribute 5, "rv64i2p0_c2p0"
 ; RV64ZIHINTPAUSE: .attribute 5, "rv64i2p0_zihintpause2p0"

 ; RV64XTHEADBB: .attribute 5, "rv64i2p0_xtheadbb1p0"
 ; RV64XTHEADBS: .attribute 5, "rv64i2p0_xtheadbs1p0"
 ; RV64XTHEADMAC: .attribute 5, "rv64i2p0_xtheadmac1p0"
 ; RV64XTHEADVDOT: .attribute 5, "rv64i2p0_f2p0_d2p0_v1p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0_xtheadvdot1p0"
 ; RV64ZTSO: .attribute 5, "rv64i2p0_ztso0p1"
 ; RV64ZCA: .attribute 5, "rv64i2p0_zca1p0"
 ; RV64ZCB: .attribute 5, "rv64i2p0_zca1p0_zcb1p0"
 ; RV64ZCD: .attribute 5, "rv64i2p0_zcd1p0"
+; RV64ZICSR: .attribute 5, "rv64i2p0_zicsr2p0"
+; RV64ZIFENCEI: .attribute 5, "rv64i2p0_zifencei2p0"
 
 define i32 @addi(i32 %a) {
   %1 = add i32 %a, 1
   ret i32 %1
 }

llvm/test/MC/RISCV/attribute-arch.s

 .attribute arch, "rv32izcb1p0"
 # CHECK: attribute      5, "rv32i2p0_zca1p0_zcb1p0"
 
 .attribute arch, "rv32izawrs1p0"
 # CHECK: attribute      5, "rv32i2p0_zawrs1p0"
 
 .attribute arch, "rv32iztso0p1"
 # CHECK: attribute      5, "rv32i2p0_ztso0p1"
+
+.attribute arch, "rv32izicsr2p0"
+# CHECK: attribute      5, "rv32i2p0_zicsr2p0"
+
+.attribute arch, "rv32izifencei2p0"
+# CHECK: attribute      5, "rv32i2p0_zifencei2p0"